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

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

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,

- 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