[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <692e16f9-062c-4b3c-bd66-a16bac68216c@linux.intel.com>
Date: Fri, 5 Jan 2024 09:38:08 -0500
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: kajoljain <kjain@...ux.ibm.com>, acme@...nel.org, irogers@...gle.com,
peterz@...radead.org, mingo@...hat.com, namhyung@...nel.org,
jolsa@...nel.org, adrian.hunter@...el.com, john.g.garry@...cle.com,
will@...nel.org, james.clark@....com, mike.leach@...aro.org,
leo.yan@...aro.org, yuhaixin.yhx@...ux.alibaba.com,
renyu.zj@...ux.alibaba.com, tmricht@...ux.ibm.com, ravi.bangoria@....com,
atrajeev@...ux.vnet.ibm.com, linux-kernel@...r.kernel.org,
linux-perf-users@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH V3 0/7] Clean up perf mem
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@...ux.intel.com wrote:
>>>>> From: Kan Liang <kan.liang@...ux.intel.com>
>>>>>
>>>>> 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@linux.intel.com/
>>>>>
>>>>> 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
>>>>>
>>>>
>>>
>
Powered by blists - more mailing lists