| 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
| ||
|
Message-ID: <029c123a-461a-4444-bd92-0317be833334@intel.com>
Date: Fri, 24 Oct 2025 13:18:58 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: tanze <tanze@...inos.cn>, <leo.yan@....com>, <namhyung@...nel.org>,
<mingo@...hat.com>
CC: <graham.woodward@....com>, <irogers@...gle.com>, <james.clark@...aro.org>,
<john.g.garry@...cle.com>, <linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, <linux-perf-users@...r.kernel.org>,
<mike.leach@...aro.org>
Subject: Re: [PATCH v3] perf auxtrace: Add auxtrace_synth_id_range_start()
helper
On 24/10/2025 11:56, tanze wrote:
> To avoid hardcoding the offset value for synthetic event IDs
> in multiple auxtrace modules (arm-spe, cs-etm, intel-pt, etc.),
> and to improve code reusability, this patch unifies
> the handling of the ID offset via a dedicated helper function.
>
> Signed-off-by: tanze <tanze@...inos.cn>
Reviewed-by: Adrian Hunter <adrian.hunter@...el.com>
> ---
>
> Hi Leo Yan, Adrian Hunter,
>
> Thank you for your valuable comments. I have modified my code according to your suggestions.
>
> Best regards,
> Ze Tan
> ---
> Changes in v3:
> - Removed PERF_SYNTH_EVENT_ID_OFFSET from synthetic-events.h
> - Added AUXTRACE_SYNTH_EVENT_ID_OFFSET macro in auxtrace.c
> - Introduced auxtrace_synth_id_range_start() helper function in auxtrace.c
> to unify ID calculation
> - Updated all auxtrace modules to use the new helper function
>
> Changes in v2:
> - Migrated the macro definitions to the synthetic-events.h file
> - Added modifications to other source files that use the offset value
>
> v2:
> -patch: https://lore.kernel.org/all/20251017021540.45930-1-tanze@kylinos.cn/
>
> ---
> tools/perf/util/arm-spe.c | 5 +----
> tools/perf/util/auxtrace.c | 16 ++++++++++++++++
> tools/perf/util/auxtrace.h | 1 +
> tools/perf/util/cs-etm.c | 5 +----
> tools/perf/util/intel-bts.c | 4 +---
> tools/perf/util/intel-pt.c | 4 +---
> tools/perf/util/powerpc-vpadtl.c | 4 +---
> 7 files changed, 22 insertions(+), 17 deletions(-)
>
> diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
> index 71be979f5077..4fd47600d9e4 100644
> --- a/tools/perf/util/arm-spe.c
> +++ b/tools/perf/util/arm-spe.c
> @@ -1732,10 +1732,7 @@ arm_spe_synth_events(struct arm_spe *spe, struct perf_session *session)
> attr.sample_period = spe->synth_opts.period;
>
> /* create new id val to be a fixed offset from evsel id */
> - id = evsel->core.id[0] + 1000000000;
> -
> - if (!id)
> - id = 1;
> + id = auxtrace_synth_id_range_start(evsel);
>
> if (spe->synth_opts.flc) {
> spe->sample_flc = true;
> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index 1539c1dc823c..35f4745f6b2b 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -62,6 +62,22 @@
> #include <internal/lib.h>
> #include "util/sample.h"
>
> +#define AUXTRACE_SYNTH_EVENT_ID_OFFSET 1000000000ULL
> +
> +/*
> + * Event IDs are allocated sequentially, so a big offset from any
> + * existing ID will reach a unused range.
> + */
> +u64 auxtrace_synth_id_range_start(struct evsel *evsel)
> +{
> + u64 id = evsel->core.id[0] + AUXTRACE_SYNTH_EVENT_ID_OFFSET;
> +
> + if (!id)
> + id = 1;
> +
> + return id;
> +}
> +
> /*
> * Make a group from 'leader' to 'last', requiring that the events were not
> * already grouped to a different leader.
> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> index e0a5b39fed12..ed3a1aaaf5d9 100644
> --- a/tools/perf/util/auxtrace.h
> +++ b/tools/perf/util/auxtrace.h
> @@ -648,6 +648,7 @@ void auxtrace__free_events(struct perf_session *session);
> void auxtrace__free(struct perf_session *session);
> bool auxtrace__evsel_is_auxtrace(struct perf_session *session,
> struct evsel *evsel);
> +u64 auxtrace_synth_id_range_start(struct evsel *evsel);
>
> #define ITRACE_HELP \
> " i[period]: synthesize instructions events\n" \
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 30f4bb3e7fa3..62812bef1edf 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1726,10 +1726,7 @@ static int cs_etm__synth_events(struct cs_etm_auxtrace *etm,
> attr.read_format = evsel->core.attr.read_format;
>
> /* create new id val to be a fixed offset from evsel id */
> - id = evsel->core.id[0] + 1000000000;
> -
> - if (!id)
> - id = 1;
> + id = auxtrace_synth_id_range_start(evsel);
>
> if (etm->synth_opts.branches) {
> attr.config = PERF_COUNT_HW_BRANCH_INSTRUCTIONS;
> diff --git a/tools/perf/util/intel-bts.c b/tools/perf/util/intel-bts.c
> index 3625c6224750..382255393fb3 100644
> --- a/tools/perf/util/intel-bts.c
> +++ b/tools/perf/util/intel-bts.c
> @@ -777,9 +777,7 @@ static int intel_bts_synth_events(struct intel_bts *bts,
> attr.sample_id_all = evsel->core.attr.sample_id_all;
> attr.read_format = evsel->core.attr.read_format;
>
> - id = evsel->core.id[0] + 1000000000;
> - if (!id)
> - id = 1;
> + id = auxtrace_synth_id_range_start(evsel);
>
> if (bts->synth_opts.branches) {
> attr.config = PERF_COUNT_HW_BRANCH_INSTRUCTIONS;
> diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
> index 9b1011fe4826..fc9eec8b54b8 100644
> --- a/tools/perf/util/intel-pt.c
> +++ b/tools/perf/util/intel-pt.c
> @@ -3987,9 +3987,7 @@ static int intel_pt_synth_events(struct intel_pt *pt,
> attr.sample_id_all = evsel->core.attr.sample_id_all;
> attr.read_format = evsel->core.attr.read_format;
>
> - id = evsel->core.id[0] + 1000000000;
> - if (!id)
> - id = 1;
> + id = auxtrace_synth_id_range_start(evsel);
>
> if (pt->synth_opts.branches) {
> attr.config = PERF_COUNT_HW_BRANCH_INSTRUCTIONS;
> diff --git a/tools/perf/util/powerpc-vpadtl.c b/tools/perf/util/powerpc-vpadtl.c
> index 39a3fb3f1330..bfa4156d7a97 100644
> --- a/tools/perf/util/powerpc-vpadtl.c
> +++ b/tools/perf/util/powerpc-vpadtl.c
> @@ -656,9 +656,7 @@ powerpc_vpadtl_synth_events(struct powerpc_vpadtl *vpa, struct perf_session *ses
> attr.config = PERF_SYNTH_POWERPC_VPA_DTL;
>
> /* create new id val to be a fixed offset from evsel id */
> - id = evsel->core.id[0] + 1000000000;
> - if (!id)
> - id = 1;
> + id = auxtrace_synth_id_range_start(evsel);
>
> err = perf_session__deliver_synth_attr_event(session, &attr, id);
> if (err)
Powered by blists - more mailing lists