Re: [PATCH 1/2] powerpc/vpa_pmu: Add interface to expose vpa counters via perf

[Christophe Leroy]

Le 13/09/2024 à 10:35, kajoljain a écrit :
> On 9/13/24 12:00, Christophe Leroy wrote:
> > Le 28/08/2024 à 12:21, Kajol Jain a écrit :
> > > The pseries Shared Processor Logical Partition(SPLPAR) machines
> > > can retrieve a log of dispatch and preempt events from the
> > > hypervisor using data from Disptach Trace Log(DTL) buffer.
> > > With this information, user can retrieve when and why each dispatch &
> > > preempt has occurred. Added an interface to expose the Virtual Processor
> > > Area(VPA) DTL counters via perf.
> > >
> > > The following events are available and exposed in sysfs:
> > >
> > >    vpa_dtl/dtl_cede/ - Trace voluntary (OS initiated) virtual processor
> > > waits
> > >    vpa_dtl/dtl_preempt/ - Trace time slice preempts
> > >    vpa_dtl/dtl_fault/ - Trace virtual partition memory page faults.
> > >    vpa_dtl/dtl_all/ - Trace all (dtl_cede/dtl_preempt/dtl_fault)
> > >
> > > Added interface defines supported event list, config fields for the
> > > event attributes and their corresponding bit values which are exported
> > > via sysfs. User could use the standard perf tool to access perf events
> > > exposed via vpa-dtl pmu.
> > >
> > > The VPA DTL PMU counters do not interrupt on overflow or generate any
> > > PMI interrupts. Therefore, the kernel needs to poll the counters, added
> > > hrtimer code to do that. The timer interval can be provided by user via
> > > sample_period field in nano seconds.
> > >
> > > Result on power10 SPLPAR system with 656 cpu threads.
> > > In the below perf record command with vpa_dtl pmu, -c option is used
> > > to provide sample_period whch corresponding to 1000000000ns i.e; 1sec
> > > and the workload time is also 1 second, hence we are getting 656 samples:
> > >
> > > [command] perf record -a -R -e vpa_dtl/dtl_all/ -c 1000000000 sleep 1
> > > [ perf record: Woken up 1 times to write data ]
> > > [ perf record: Captured and wrote 0.828 MB perf.data (656 samples) ]
> > >
> > > There is one hrtimer added per vpa-dtl pmu thread. Code added to handle
> > > addition of dtl buffer data in the raw sample. Since DTL does not provide
> > > IP address for a sample and it just have traces on reason of
> > > dispatch/preempt, we directly saving DTL buffer data to perf.data file as
> > > raw sample. For each hrtimer restart call, interface will dump all the
> > > new dtl entries added to dtl buffer as a raw sample.
> > >
> > > To ensure there are no other conflicting dtl users (example: debugfs dtl
> > > or /proc/powerpc/vcpudispatch_stats), interface added code to use
> > > "down_write_trylock" call to take the dtl_access_lock. The
> > > dtl_access_lock
> > > is defined in dtl.h file. Also added global reference count variable
> > > called
> > > "dtl_global_refc", to ensure dtl data can be captured per-cpu. Code also
> > > added global lock called "dtl_global_lock" to avoid race condition.
> > >
> > > Signed-off-by: Kajol Jain <kjain@xxxxxxxxxxxxx>
> > > ---
> > > Notes:
> > >
> > > - Made code changes on top of recent fix sent by Michael Ellerman.
> > >     Link to the patch:
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.ozlabs.org%2Fproject%2Flinuxppc-dev%2Fpatch%2F20240819122401.513203-1-mpe%40ellerman.id.au%2F&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C95cfb2842b2a44907c9108dcd3cf0b12%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638618133431151306%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=IqFjcvT9G0HYGIbuUWhCrnXkFr9yMtSC1mGFFKZ66MI%3D&reserved=0
> > >
> > >    arch/powerpc/perf/Makefile  |   2 +-
> > >    arch/powerpc/perf/vpa-pmu.c | 469 ++++++++++++++++++++++++++++++++++++
> > >    include/linux/cpuhotplug.h  |   1 +
> > >    3 files changed, 471 insertions(+), 1 deletion(-)
> > >    create mode 100644 arch/powerpc/perf/vpa-pmu.c
> >
> >
> > Seems like it doesn't build on PPC64:
> >
> > arch/powerpc/perf/vpa-pmu.c#L212
> > passing argument 1 of 'up_write' from incompatible pointer type
> > [-Wincompatible-pointer-types]
> >
> > arch/powerpc/perf/vpa-pmu.c#L261
> > passing argument 1 of 'down_write_trylock' from incompatible pointer
> > type [-Wincompatible-pointer-types]
> >
> > arch/powerpc/perf/vpa-pmu.c#L402
> > passing argument 1 of 'up_write' from incompatible pointer type
> > [-Wincompatible-pointer-types]
>
> Hi Christophe,
>     Thanks for checking the patch. These changes are on top of fix patch
> sent by Michael Ellerman
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.ozlabs.org%2Fproject%2Flinuxppc-dev%2Fpatch%2F20240819122401.513203-1-mpe%40ellerman.id.au%2F&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C95cfb2842b2a44907c9108dcd3cf0b12%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638618133431160525%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=r1vQxb%2F8IXBRffDtFMwiqbKnFZ4iiqer6cLykT00%2Bew%3D&reserved=0
>
> Since he changed the dtl_access_lock to be a rw_semaphore.
>
> Are you trying with Michael patch changes?

No, I only saw CI test failure here : 
https://github.com/linuxppc/linux-snowpatch/actions/runs/10594868105

Sorry I didn't see you mentioned it in a note.

Christophe