Received-SPF: pass (google.com: domain of linux-kernel+bounces-17516-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <057a1c19-3117-1aec-41d6-4950c599b862@linux.ibm.com>
Date: Fri, 5 Jan 2024 12:08:45 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH V3 0/7] Clean up perf mem
To: "Liang, Kan" <kan.liang@linux.intel.com>, acme@kernel.org,
        irogers@google.com, peterz@infradead.org, mingo@redhat.com,
        namhyung@kernel.org, jolsa@kernel.org, adrian.hunter@intel.com,
        john.g.garry@oracle.com, will@kernel.org, james.clark@arm.com,
        mike.leach@linaro.org, leo.yan@linaro.org,
        yuhaixin.yhx@linux.alibaba.com, renyu.zj@linux.alibaba.com,
        tmricht@linux.ibm.com, ravi.bangoria@amd.com,
        atrajeev@linux.vnet.ibm.com, linux-kernel@vger.kernel.org,
        linux-perf-users@vger.kernel.org, linux-arm-kernel@lists.infradead.org
References: <20231213195154.1085945-1-kan.liang@linux.intel.com>
 <a0abfee5-4dcd-3eb5-82fe-1a0dcdade038@linux.ibm.com>
 <befb6acd-86be-4255-af96-38865affc56c@linux.intel.com>
 <8bfadc86-e137-4a9f-a9ce-0bc62464c195@linux.intel.com>
Content-Language: en-US
From: kajoljain <kjain@linux.ibm.com>
In-Reply-To: <8bfadc86-e137-4a9f-a9ce-0bc62464c195@linux.intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0


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. 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.

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@linux.intel.com wrote:
>>>> From: Kan Liang <kan.liang@linux.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
>>>>
>>>
>>