linux-kernel - Re: [PATCH v1 3/9] perf auxtrace: Refactor auxtrace__evsel_is

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3e3b38d1-5170-44ce-b1ba-e9599cceee95@arm.com>
Date: Fri, 9 Aug 2024 11:57:26 +0100
From: Leo Yan <leo.yan@....com>
To: James Clark <james.clark@...aro.org>,
 Adrian Hunter <adrian.hunter@...el.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Ian Rogers <irogers@...gle.com>,
 Suzuki K Poulose <suzuki.poulose@....com>, Mike Leach
 <mike.leach@...aro.org>, coresight@...ts.linaro.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 "Liang, Kan" <kan.liang@...ux.intel.com>
Subject: Re: [PATCH v1 3/9] perf auxtrace: Refactor
 auxtrace__evsel_is_auxtrace()

On 8/9/24 11:08, James Clark wrote:

[...]

> On 08/08/2024 1:58 pm, Adrian Hunter wrote:
>> On 6/08/24 23:41, Leo Yan wrote:
>>> The auxtrace__evsel_is_auxtrace() function invokes the callback
>>> .evsel_is_auxtrace() to check if an event is an AUX trace. In the
>>> low-level code, every AUX trace module provides its callback to
>>> compare the PMU type.
>>>
>>> This commit refactors auxtrace__evsel_is_auxtrace() by simply
>>> calling evsel__is_aux_event() rather than using the callback function.
>>> As a result, the callback .evsel_is_auxtrace() is no longer needed, so
>>> the definition and implementations are removed.
>>
>> evsel__is_aux_event() assumes it is on the target machine e.g.
>> being called from perf record.  It indirectly reads from sysfs
>> to find PMUs, which will not necessarily be the same a different
>> machine.
>>
>> For example, what happens if a perf data file from one arch is
>> being processed on a machine from another arch.
> 
> I think this does go a bit wrong. If I open an SPE file on x86 it finds
> the intel_pt PMU which both have the same type number. But because
> that's also an auxtrace one it appears to work.

Yes, anyway, I missed the cross report case and this patch will break it.

A event attribute should contain info to indicate the event is an AUX event.
This would be better than inquiry every AUX module. I checked the attribute,
there have no attribute field can be use to indicate a event is AUX event.
A relevant field 'aux_output' is used to generate AUX trace rather than
events, which is not set for AUX events.

Can we add a new field 'auxtrace' into the structure perf_event_attr? By using
this way, the event will track the state rather than stored in PMU.

For next step, I will pick the first two patches in this series and merge them
into the multiple AUX events support patch set. This can allow us to firstly
resolve the multiple AUX event issue.

If we agree the refactoring for 'auxtrace' attribute, then I will use a separate
patch set to address it.

Thanks,
Leo