Re: [PATCH v8 2/4] libperf: Add evsel mmap support

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Wed, Apr 14, 2021 at 03:02:08PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Apr 15, 2021 at 01:41:35AM +0900, Namhyung Kim escreveu:
> > Hello,
> > 
> > On Thu, Apr 15, 2021 at 1:07 AM Rob Herring <robh@kernel.org> wrote:
> > > +void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread)
> > > +{
> > > +       if (FD(evsel, cpu, thread) < 0 || MMAP(evsel, cpu, thread) == NULL)
> > > +               return NULL;
> > 
> > I think you should check the cpu and the thread is in
> > a valid range.  Currently xyarray__entry() simply accesses
> > the content without checking the boundaries.
> 
> So, since xyarray has the bounds, it should check it, i.e. we need to
> have a __xyarray__entry() that is what xyarray__entry() does, i.e.
> assume the values have been bounds checked, then a new method,
> xyarray__entry() that does bounds check, if it fails, return NULL,
> otherwise calls __xyarray__entry().
> 
> I see this is frustrating and I should've chimed in earlier, but at
> least now this is getting traction, and the end result will be better
> not just for the feature you've been dilligently working on,
> 
> Thank you for your persistence,

Re-reading, yeah, this can be done in a separate patch, Namhyung, can I
have your Reviewed-by? That or an Acked-by?

- Arnaldo
 
> - Arnaldo
>  
> > Thanks,
> > Namhyung
> > 
> > 
> > > +
> > > +       return MMAP(evsel, cpu, thread)->base;
> > > +}
> > > +
> > >  int perf_evsel__read_size(struct perf_evsel *evsel)
> > >  {
> > >         u64 read_format = evsel->attr.read_format;
> > > diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h
> > > index 1ffd083b235e..1c067d088bc6 100644
> > > --- a/tools/lib/perf/include/internal/evsel.h
> > > +++ b/tools/lib/perf/include/internal/evsel.h
> > > @@ -41,6 +41,7 @@ struct perf_evsel {
> > >         struct perf_cpu_map     *own_cpus;
> > >         struct perf_thread_map  *threads;
> > >         struct xyarray          *fd;
> > > +       struct xyarray          *mmap;
> > >         struct xyarray          *sample_id;
> > >         u64                     *id;
> > >         u32                      ids;
> > > diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
> > > index c82ec39a4ad0..60eae25076d3 100644
> > > --- a/tools/lib/perf/include/perf/evsel.h
> > > +++ b/tools/lib/perf/include/perf/evsel.h
> > > @@ -27,6 +27,9 @@ LIBPERF_API int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *
> > >                                  struct perf_thread_map *threads);
> > >  LIBPERF_API void perf_evsel__close(struct perf_evsel *evsel);
> > >  LIBPERF_API void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu);
> > > +LIBPERF_API int perf_evsel__mmap(struct perf_evsel *evsel, int pages);
> > > +LIBPERF_API void perf_evsel__munmap(struct perf_evsel *evsel);
> > > +LIBPERF_API void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread);
> > >  LIBPERF_API int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
> > >                                  struct perf_counts_values *count);
> > >  LIBPERF_API int perf_evsel__enable(struct perf_evsel *evsel);
> > > diff --git a/tools/lib/perf/libperf.map b/tools/lib/perf/libperf.map
> > > index 7be1af8a546c..c0c7ceb11060 100644
> > > --- a/tools/lib/perf/libperf.map
> > > +++ b/tools/lib/perf/libperf.map
> > > @@ -23,6 +23,9 @@ LIBPERF_0.0.1 {
> > >                 perf_evsel__disable;
> > >                 perf_evsel__open;
> > >                 perf_evsel__close;
> > > +               perf_evsel__mmap;
> > > +               perf_evsel__munmap;
> > > +               perf_evsel__mmap_base;
> > >                 perf_evsel__read;
> > >                 perf_evsel__cpus;
> > >                 perf_evsel__threads;
> > > --
> > > 2.27.0
> 
> -- 
> 
> - Arnaldo

-- 

- Arnaldo