Re: [PATCH V3 0/7] Clean up perf mem

From: Liang, Kan
Date: Fri Jan 05 2024 - 09:38:25 EST

Next message: Jason Gunthorpe: "Re: [RFC PATCH 3/6] iommu/amd: Introduce Guest-ID struct amd_iommu_vminfo"
Previous message: Jason Gunthorpe: "Re: [RFC PATCH 6/6] iommu/amd: Introduce nested translation support"
In reply to: kajoljain: "Re: [PATCH V3 0/7] Clean up perf mem"
Next in thread: Leo Yan: "Re: [PATCH V3 0/7] Clean up perf mem"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 2024-01-05 1:38 a.m., kajoljain wrote:
>
>
> On 1/3/24 01:38, Liang, Kan wrote:
>> Hi Kajol Jain
>>
>> On 2023-12-19 9:15 a.m., Liang, Kan wrote:
>>>
>>>
>>> On 2023-12-19 4:26 a.m., kajoljain wrote:
>>>> Hi,
>>>> I was trying to test this patchset on powerpc.
>>>>
>>>> After applying it on top of acme's perf-tools-next branch, I am getting
>>>> below error:
>>>>
>>>> INSTALL libsubcmd_headers
>>>> INSTALL libperf_headers
>>>> INSTALL libsymbol_headers
>>>> INSTALL libapi_headers
>>>> INSTALL libbpf_headers
>>>> CC arch/powerpc/util/mem-events.o
>>>> In file included from arch/powerpc/util/mem-events.c:3:
>>>> arch/powerpc/util/mem-events.h:5:52: error: ‘PERF_MEM_EVENTS__MAX’
>>>> undeclared here (not in a function)
>>>> 5 | extern struct perf_mem_event
>>>> perf_mem_events_power[PERF_MEM_EVENTS__MAX];
>>>> |
>>>> ^~~~~~~~~~~~~~~~~~~~
>>>> make[6]: *** [/home/kajol/linux/tools/build/Makefile.build:105:
>>>> arch/powerpc/util/mem-events.o] Error 1
>>>> make[5]: *** [/home/kajol/linux/tools/build/Makefile.build:158: util]
>>>> Error 2
>>>> make[4]: *** [/home/kajol/linux/tools/build/Makefile.build:158: powerpc]
>>>> Error 2
>>>> make[3]: *** [/home/kajol/linux/tools/build/Makefile.build:158: arch]
>>>> Error 2
>>>> make[3]: *** Waiting for unfinished jobs....
>>>> make[2]: *** [Makefile.perf:693: perf-in.o] Error 2
>>>> make[1]: *** [Makefile.perf:251: sub-make] Error 2
>>>> make: *** [Makefile:70: all] Error 2
>>>>
>>>> It seems some headerfiles are missing from arch/powerpc/util/mem-
>>>> events.c
>>>>
>>>
>>> Leo updated the headerfiles for ARM. https://termbin.com/0dkn
>>>
>>> I guess powerpc has to do the same thing. Could you please try the below
>>> patch?
>>
>>
>> Does the patch work on powerpc?
>
> Hi Kan,
> Sorry I went for vacation so couldn't update. Yes this fix works.

Thanks for the update.

> But
> we have another issue, actually this patch set changes uses ldlat
> attribute. But ldlat is not supported in powerpc because of which perf
> mem is failing in powerpc.

For powerpc, the patch 3 introduced a perf_mem_events_power, which
doesn't have ldlat. But it only be assigned to the pmu->is_core. I'm not
sure if it's the problem.
Also, S390 still uses the default perf_mem_events, which includes ldlat.
I'm not sure if S390 supports the ldlat.

Thanks,
Kan
>
> I am looking into a work around to fix this issue. I will update the fix.
>
> Thanks,
> Kajol Jain
>
>
>>
>>
>> Thanks,
>> Kan
>>>
>>> diff --git a/tools/perf/arch/powerpc/util/mem-events.c
>>> b/tools/perf/arch/powerpc/util/mem-events.c
>>> index 72a6ac2b52f5..765d4a054b0a 100644
>>> --- a/tools/perf/arch/powerpc/util/mem-events.c
>>> +++ b/tools/perf/arch/powerpc/util/mem-events.c
>>> @@ -1,5 +1,6 @@
>>> // SPDX-License-Identifier: GPL-2.0
>>> -#include "map_symbol.h"
>>> +#include "util/map_symbol.h"
>>> +#include "util/mem-events.h"
>>> #include "mem-events.h"
>>>
>>> #define E(t, n, s, l, a) { .tag = t, .name = n, .event_name = s, .ldlat
>>> = l, .aux_event = a }
>>>
>>> Thanks,
>>> Kan
>>>
>>>> Thanks,
>>>> Kajol Jain
>>>>
>>>> On 12/14/23 01:21, kan.liang@xxxxxxxxxxxxxxx wrote:
>>>>> From: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
>>>>>
>>>>> Changes since V2:
>>>>> - Fix the Arm64 building error (Leo)
>>>>> - Add two new patches to clean up perf_mem_events__record_args()
>>>>> and perf_pmus__num_mem_pmus() (Leo)
>>>>>
>>>>> Changes since V1:
>>>>> - Fix strcmp of PMU name checking (Ravi)
>>>>> - Fix "/," typo (Ian)
>>>>> - Rename several functions with perf_pmu__mem_events prefix. (Ian)
>>>>> - Fold the header removal patch into the patch where the cleanups made.
>>>>> (Arnaldo)
>>>>> - Add reviewed-by and tested-by from Ian and Ravi
>>>>>
>>>>> As discussed in the below thread, the patch set is to clean up perf mem.
>>>>> https://lore.kernel.org/lkml/afefab15-cffc-4345-9cf4-c6a4128d4d9c@xxxxxxxxxxxxxxx/
>>>>>
>>>>> Introduce generic functions perf_mem_events__ptr(),
>>>>> perf_mem_events__name() ,and is_mem_loads_aux_event() to replace the
>>>>> ARCH specific ones.
>>>>> Simplify the perf_mem_event__supported().
>>>>>
>>>>> Only keeps the ARCH-specific perf_mem_events array in the corresponding
>>>>> mem-events.c for each ARCH.
>>>>>
>>>>> There is no functional change.
>>>>>
>>>>> The patch set touches almost all the ARCHs, Intel, AMD, ARM, Power and
>>>>> etc. But I can only test it on two Intel platforms.
>>>>> Please give it try, if you have machines with other ARCHs.
>>>>>
>>>>> Here are the test results:
>>>>> Intel hybrid machine:
>>>>>
>>>>> $perf mem record -e list
>>>>> ldlat-loads : available
>>>>> ldlat-stores : available
>>>>>
>>>>> $perf mem record -e ldlat-loads -v --ldlat 50
>>>>> calling: record -e cpu_atom/mem-loads,ldlat=50/P -e cpu_core/mem-loads,ldlat=50/P
>>>>>
>>>>> $perf mem record -v
>>>>> calling: record -e cpu_atom/mem-loads,ldlat=30/P -e cpu_atom/mem-stores/P -e cpu_core/mem-loads,ldlat=30/P -e cpu_core/mem-stores/P
>>>>>
>>>>> $perf mem record -t store -v
>>>>> calling: record -e cpu_atom/mem-stores/P -e cpu_core/mem-stores/P
>>>>>
>>>>>
>>>>> Intel SPR:
>>>>> $perf mem record -e list
>>>>> ldlat-loads : available
>>>>> ldlat-stores : available
>>>>>
>>>>> $perf mem record -e ldlat-loads -v --ldlat 50
>>>>> calling: record -e {cpu/mem-loads-aux/,cpu/mem-loads,ldlat=50/}:P
>>>>>
>>>>> $perf mem record -v
>>>>> calling: record -e {cpu/mem-loads-aux/,cpu/mem-loads,ldlat=30/}:P -e cpu/mem-stores/P
>>>>>
>>>>> $perf mem record -t store -v
>>>>> calling: record -e cpu/mem-stores/P
>>>>>
>>>>> Kan Liang (7):
>>>>> perf mem: Add mem_events into the supported perf_pmu
>>>>> perf mem: Clean up perf_mem_events__ptr()
>>>>> perf mem: Clean up perf_mem_events__name()
>>>>> perf mem: Clean up perf_mem_event__supported()
>>>>> perf mem: Clean up is_mem_loads_aux_event()
>>>>> perf mem: Clean up perf_mem_events__record_args()
>>>>> perf mem: Clean up perf_pmus__num_mem_pmus()
>>>>>
>>>>> tools/perf/arch/arm/util/pmu.c | 3 +
>>>>> tools/perf/arch/arm64/util/mem-events.c | 39 +---
>>>>> tools/perf/arch/arm64/util/mem-events.h | 7 +
>>>>> tools/perf/arch/powerpc/util/mem-events.c | 13 +-
>>>>> tools/perf/arch/powerpc/util/mem-events.h | 7 +
>>>>> tools/perf/arch/powerpc/util/pmu.c | 11 ++
>>>>> tools/perf/arch/s390/util/pmu.c | 3 +
>>>>> tools/perf/arch/x86/util/mem-events.c | 99 ++--------
>>>>> tools/perf/arch/x86/util/mem-events.h | 10 +
>>>>> tools/perf/arch/x86/util/pmu.c | 19 +-
>>>>> tools/perf/builtin-c2c.c | 45 ++---
>>>>> tools/perf/builtin-mem.c | 48 ++---
>>>>> tools/perf/util/mem-events.c | 217 +++++++++++++---------
>>>>> tools/perf/util/mem-events.h | 19 +-
>>>>> tools/perf/util/pmu.c | 4 +-
>>>>> tools/perf/util/pmu.h | 7 +
>>>>> tools/perf/util/pmus.c | 6 -
>>>>> tools/perf/util/pmus.h | 1 -
>>>>> 18 files changed, 278 insertions(+), 280 deletions(-)
>>>>> create mode 100644 tools/perf/arch/arm64/util/mem-events.h
>>>>> create mode 100644 tools/perf/arch/powerpc/util/mem-events.h
>>>>> create mode 100644 tools/perf/arch/powerpc/util/pmu.c
>>>>> create mode 100644 tools/perf/arch/x86/util/mem-events.h
>>>>>
>>>>
>>>
>

Next message: Jason Gunthorpe: "Re: [RFC PATCH 3/6] iommu/amd: Introduce Guest-ID struct amd_iommu_vminfo"
Previous message: Jason Gunthorpe: "Re: [RFC PATCH 6/6] iommu/amd: Introduce nested translation support"
In reply to: kajoljain: "Re: [PATCH V3 0/7] Clean up perf mem"
Next in thread: Leo Yan: "Re: [PATCH V3 0/7] Clean up perf mem"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]