Re: [PATCH] perf jevents: return potentially empty metrics table

[James Clark <james.clark@linaro.org>]

On 12/07/2024 10:54 pm, Ian Rogers wrote:
> On Wed, Jul 3, 2024 at 3:33 PM Namhyung Kim <namhyung@kernel.org> wrote:
>>
>> Hello,
>>
>> On Mon, Jul 01, 2024 at 07:19:55PM +0200, Lucas Stach wrote:
>>> Hi,
>>>
>>> has this patch fallen through the cracks? It fixes a functional
>>> regression where the DDR controller metrics are completely unavailable
>>> on all i.MX8M* systems and thus would be nice if someone could have a
>>> look.
>>>
>>> Regards,
>>> Lucas
>>>
>>> Am Freitag, dem 31.05.2024 um 21:44 +0200 schrieb Lucas Stach:
>>>> Don't return NULL when a empty (num_pmus = 0) metrics table is encountered,
>>>> as this causes many of the users to bail out, which will skip matching any
>>>> potentially existing sys metrics later on. Instead return the empty table
>>>> which will be handled properly by the iterators and allows matching to
>>>> continue.
>>>>
>>>> This fixes metrics reporting on systems where only the sys, but not the
>>>> core PMUs have metrics defined.
>>>>
>>>> Fixes: f20c15d13f01 ("perf pmu-events: Remember the perf_events_map for a PMU")
>>>> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>>
>> Looks ok to me.  Ian?
> 
> Sorry I missed this, I expect to see patches on LKML and so my email
> filters placed this elsewhere.
> 
> The change doesn't make sense. A search is requested with a PMU and
> you return success (the table) when there is no PMU match. If this
> behavior were expected then NULL should be passed as a PMU - this is
> the behavior in util/pmu.c and ARM is the only architecture to pass a
> PMU. The ARM pmu_metrics_table__find also specifically breaks
> heterogeneous (BIG.little) systems and this change won't impact/fix
> that. Perhaps there is some context I lack.
> 
> Thanks,
> Ian
> 

Yes it looks like the fix should possibly be higher up in the arm64 
pmu_metrics_table__find().

Which set of JSON files are you seeing the issue with exactly Lucas? I 
see that there are multiple versions of i.MX8M metrics.

James

>> Thanks,
>> Namhyung
>>
>>
>>>> ---
>>>>   tools/perf/pmu-events/jevents.py | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
>>>> index e42efc16723e..0a1ed9ee1429 100755
>>>> --- a/tools/perf/pmu-events/jevents.py
>>>> +++ b/tools/perf/pmu-events/jevents.py
>>>> @@ -1081,7 +1081,7 @@ const struct pmu_metrics_table *perf_pmu__find_metrics_table(struct perf_pmu *pm
>>>>           if (!map)
>>>>                   return NULL;
>>>>
>>>> -        if (!pmu)
>>>> +        if (!pmu || !map->metric_table.num_pmus)
>>>>                   return &map->metric_table;
>>>>
>>>>           for (size_t i = 0; i < map->metric_table.num_pmus; i++) {
>>>
>