Message-ID: <87bc0aba-2984-459e-b0d6-d9bf7b9497b4@csgroup.eu>
Date: Fri, 13 Sep 2024 11:01:35 +0200
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: kajoljain <kjain@...ux.ibm.com>, mpe@...erman.id.au
Cc: atrajeev@...ux.vnet.ibm.com, linux-kernel@...r.kernel.org,
 linux-perf-users@...r.kernel.org, maddy@...ux.ibm.com,
 disgoel@...ux.ibm.com, linuxppc-dev@...ts.ozlabs.org,
 hbathini@...ux.ibm.com, adubey@...ux.ibm.com
Subject: Re: [PATCH 1/2] powerpc/vpa_pmu: Add interface to expose vpa counters
 via perf



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@...ux.ibm.com>
>>> ---
>>> 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

Powered by blists - more mailing lists