* Re: [PATCH v2] perf arm_spe: Add a macro definition to handle offset value
2025-10-17 2:15 ` [PATCH v2] " tanze
@ 2025-10-17 10:09 ` Leo Yan
2025-10-19 3:05 ` Namhyung Kim
2025-10-24 6:27 ` Adrian Hunter
2 siblings, 0 replies; 9+ messages in thread
From: Leo Yan @ 2025-10-17 10:09 UTC (permalink / raw)
To: tanze
Cc: james.clark, leo.yan, irogers, john.g.garry, linux-arm-kernel,
linux-perf-users, linux-kernel, graham.woodward, mike.leach
On Fri, Oct 17, 2025 at 10:15:40AM +0800, tanze wrote:
> Add a macro definition SPE_SYNTH_ID_OFFSET to handle the offset value
> and improve readability.
>
> Signed-off-by: tanze <tanze@kylinos.cn>
Reviewed-by: Leo Yan <leo.yan@arm.com>
> ---
> Hi, Leo Yan
>
> Thank you for your guidance and suggestions. I have made revisions according to your requirements.
> Do you have any further suggestions?
> ---
> Changes in v2:
> - Migrate the macro definitions to the synthetic-events.h file
> - Add modifications to other source files that use the offset value
> ---
> tools/perf/util/arm-spe.c | 2 +-
> tools/perf/util/cs-etm.c | 2 +-
> tools/perf/util/intel-bts.c | 2 +-
> tools/perf/util/intel-pt.c | 2 +-
> tools/perf/util/powerpc-vpadtl.c | 3 ++-
> tools/perf/util/synthetic-events.h | 2 ++
> 6 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
> index 71be979f5077..b082cb1666a6 100644
> --- a/tools/perf/util/arm-spe.c
> +++ b/tools/perf/util/arm-spe.c
> @@ -1732,7 +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;
> + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
>
> if (!id)
> id = 1;
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 30f4bb3e7fa3..46902f7b69f1 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1726,7 +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;
> + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
>
> if (!id)
> id = 1;
> diff --git a/tools/perf/util/intel-bts.c b/tools/perf/util/intel-bts.c
> index 3625c6224750..98b928eca724 100644
> --- a/tools/perf/util/intel-bts.c
> +++ b/tools/perf/util/intel-bts.c
> @@ -777,7 +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;
> + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
> if (!id)
> id = 1;
>
> diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
> index 9b1011fe4826..4fb9600a7369 100644
> --- a/tools/perf/util/intel-pt.c
> +++ b/tools/perf/util/intel-pt.c
> @@ -3987,7 +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;
> + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
> if (!id)
> id = 1;
>
> diff --git a/tools/perf/util/powerpc-vpadtl.c b/tools/perf/util/powerpc-vpadtl.c
> index 39a3fb3f1330..5ce5d2bf4c6c 100644
> --- a/tools/perf/util/powerpc-vpadtl.c
> +++ b/tools/perf/util/powerpc-vpadtl.c
> @@ -15,6 +15,7 @@
> #include "powerpc-vpadtl.h"
> #include "sample.h"
> #include "tool.h"
> +#include "util/synthetic-events.h"
>
> /*
> * Structure to save the auxtrace queue
> @@ -656,7 +657,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;
> + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
> if (!id)
> id = 1;
>
> diff --git a/tools/perf/util/synthetic-events.h b/tools/perf/util/synthetic-events.h
> index ee29615d68e5..c8a1fe7a3fa4 100644
> --- a/tools/perf/util/synthetic-events.h
> +++ b/tools/perf/util/synthetic-events.h
> @@ -39,6 +39,8 @@ enum perf_record_synth {
> };
> #define PERF_SYNTH_ALL (PERF_SYNTH_MAX - 1)
>
> +#define PERF_SYNTH_EVENT_ID_OFFSET (1000000000ULL)
> +
> int parse_synth_opt(char *str);
>
> typedef int (*perf_event__handler_t)(const struct perf_tool *tool, union perf_event *event,
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] perf arm_spe: Add a macro definition to handle offset value
2025-10-17 2:15 ` [PATCH v2] " tanze
2025-10-17 10:09 ` Leo Yan
@ 2025-10-19 3:05 ` Namhyung Kim
2025-10-24 6:27 ` Adrian Hunter
2 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2025-10-19 3:05 UTC (permalink / raw)
To: tanze
Cc: james.clark, leo.yan, irogers, john.g.garry, linux-arm-kernel,
linux-perf-users, linux-kernel, graham.woodward, mike.leach,
Adrian Hunter, Athira Rajeev
Hello,
On Fri, Oct 17, 2025 at 10:15:40AM +0800, tanze wrote:
> Add a macro definition SPE_SYNTH_ID_OFFSET to handle the offset value
> and improve readability.
>
> Signed-off-by: tanze <tanze@kylinos.cn>
I'm adding relevant folks to CC.
Thanks,
Namhyung
> ---
> Hi, Leo Yan
>
> Thank you for your guidance and suggestions. I have made revisions according to your requirements.
> Do you have any further suggestions?
> ---
> Changes in v2:
> - Migrate the macro definitions to the synthetic-events.h file
> - Add modifications to other source files that use the offset value
> ---
> tools/perf/util/arm-spe.c | 2 +-
> tools/perf/util/cs-etm.c | 2 +-
> tools/perf/util/intel-bts.c | 2 +-
> tools/perf/util/intel-pt.c | 2 +-
> tools/perf/util/powerpc-vpadtl.c | 3 ++-
> tools/perf/util/synthetic-events.h | 2 ++
> 6 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
> index 71be979f5077..b082cb1666a6 100644
> --- a/tools/perf/util/arm-spe.c
> +++ b/tools/perf/util/arm-spe.c
> @@ -1732,7 +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;
> + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
>
> if (!id)
> id = 1;
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 30f4bb3e7fa3..46902f7b69f1 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1726,7 +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;
> + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
>
> if (!id)
> id = 1;
> diff --git a/tools/perf/util/intel-bts.c b/tools/perf/util/intel-bts.c
> index 3625c6224750..98b928eca724 100644
> --- a/tools/perf/util/intel-bts.c
> +++ b/tools/perf/util/intel-bts.c
> @@ -777,7 +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;
> + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
> if (!id)
> id = 1;
>
> diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
> index 9b1011fe4826..4fb9600a7369 100644
> --- a/tools/perf/util/intel-pt.c
> +++ b/tools/perf/util/intel-pt.c
> @@ -3987,7 +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;
> + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
> if (!id)
> id = 1;
>
> diff --git a/tools/perf/util/powerpc-vpadtl.c b/tools/perf/util/powerpc-vpadtl.c
> index 39a3fb3f1330..5ce5d2bf4c6c 100644
> --- a/tools/perf/util/powerpc-vpadtl.c
> +++ b/tools/perf/util/powerpc-vpadtl.c
> @@ -15,6 +15,7 @@
> #include "powerpc-vpadtl.h"
> #include "sample.h"
> #include "tool.h"
> +#include "util/synthetic-events.h"
>
> /*
> * Structure to save the auxtrace queue
> @@ -656,7 +657,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;
> + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
> if (!id)
> id = 1;
>
> diff --git a/tools/perf/util/synthetic-events.h b/tools/perf/util/synthetic-events.h
> index ee29615d68e5..c8a1fe7a3fa4 100644
> --- a/tools/perf/util/synthetic-events.h
> +++ b/tools/perf/util/synthetic-events.h
> @@ -39,6 +39,8 @@ enum perf_record_synth {
> };
> #define PERF_SYNTH_ALL (PERF_SYNTH_MAX - 1)
>
> +#define PERF_SYNTH_EVENT_ID_OFFSET (1000000000ULL)
> +
> int parse_synth_opt(char *str);
>
> typedef int (*perf_event__handler_t)(const struct perf_tool *tool, union perf_event *event,
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] perf arm_spe: Add a macro definition to handle offset value
2025-10-17 2:15 ` [PATCH v2] " tanze
2025-10-17 10:09 ` Leo Yan
2025-10-19 3:05 ` Namhyung Kim
@ 2025-10-24 6:27 ` Adrian Hunter
2025-10-24 8:10 ` Leo Yan
2 siblings, 1 reply; 9+ messages in thread
From: Adrian Hunter @ 2025-10-24 6:27 UTC (permalink / raw)
To: tanze, james.clark, leo.yan, irogers, john.g.garry
Cc: linux-arm-kernel, linux-perf-users, linux-kernel, graham.woodward,
mike.leach
On 17/10/2025 05:15, tanze wrote:
> Add a macro definition SPE_SYNTH_ID_OFFSET to handle the offset value
> and improve readability.
>
> Signed-off-by: tanze <tanze@kylinos.cn>
> ---
> Hi, Leo Yan
>
> Thank you for your guidance and suggestions. I have made revisions according to your requirements.
> Do you have any further suggestions?
> ---
> Changes in v2:
> - Migrate the macro definitions to the synthetic-events.h file
> - Add modifications to other source files that use the offset value
> ---
> tools/perf/util/arm-spe.c | 2 +-
> tools/perf/util/cs-etm.c | 2 +-
> tools/perf/util/intel-bts.c | 2 +-
> tools/perf/util/intel-pt.c | 2 +-
> tools/perf/util/powerpc-vpadtl.c | 3 ++-
> tools/perf/util/synthetic-events.h | 2 ++
> 6 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
> index 71be979f5077..b082cb1666a6 100644
> --- a/tools/perf/util/arm-spe.c
> +++ b/tools/perf/util/arm-spe.c
> @@ -1732,7 +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;
> + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
>
> if (!id)
> id = 1;
Functionally this is to find a free range of IDs for synthesized events,
but also the range can only have one user e.g. in this case auxtrace
So add to auxtrace.c
#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;
}
And then use that:
- /* create new id val to be a fixed offset from evsel id */
- id = evsel->core.id[0] + 1000000000;
+ id = auxtrace_synth_id_range_start(evsel);
-
- if (!id)
- id = 1;
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 30f4bb3e7fa3..46902f7b69f1 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1726,7 +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;
> + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
>
> if (!id)
> id = 1;
> diff --git a/tools/perf/util/intel-bts.c b/tools/perf/util/intel-bts.c
> index 3625c6224750..98b928eca724 100644
> --- a/tools/perf/util/intel-bts.c
> +++ b/tools/perf/util/intel-bts.c
> @@ -777,7 +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;
> + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
> if (!id)
> id = 1;
>
> diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
> index 9b1011fe4826..4fb9600a7369 100644
> --- a/tools/perf/util/intel-pt.c
> +++ b/tools/perf/util/intel-pt.c
> @@ -3987,7 +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;
> + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
> if (!id)
> id = 1;
>
> diff --git a/tools/perf/util/powerpc-vpadtl.c b/tools/perf/util/powerpc-vpadtl.c
> index 39a3fb3f1330..5ce5d2bf4c6c 100644
> --- a/tools/perf/util/powerpc-vpadtl.c
> +++ b/tools/perf/util/powerpc-vpadtl.c
> @@ -15,6 +15,7 @@
> #include "powerpc-vpadtl.h"
> #include "sample.h"
> #include "tool.h"
> +#include "util/synthetic-events.h"
>
> /*
> * Structure to save the auxtrace queue
> @@ -656,7 +657,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;
> + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
> if (!id)
> id = 1;
>
> diff --git a/tools/perf/util/synthetic-events.h b/tools/perf/util/synthetic-events.h
> index ee29615d68e5..c8a1fe7a3fa4 100644
> --- a/tools/perf/util/synthetic-events.h
> +++ b/tools/perf/util/synthetic-events.h
> @@ -39,6 +39,8 @@ enum perf_record_synth {
> };
> #define PERF_SYNTH_ALL (PERF_SYNTH_MAX - 1)
>
> +#define PERF_SYNTH_EVENT_ID_OFFSET (1000000000ULL)
> +
> int parse_synth_opt(char *str);
>
> typedef int (*perf_event__handler_t)(const struct perf_tool *tool, union perf_event *event,
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] perf arm_spe: Add a macro definition to handle offset value
2025-10-24 6:27 ` Adrian Hunter
@ 2025-10-24 8:10 ` Leo Yan
0 siblings, 0 replies; 9+ messages in thread
From: Leo Yan @ 2025-10-24 8:10 UTC (permalink / raw)
To: Adrian Hunter
Cc: tanze, james.clark, leo.yan, irogers, john.g.garry,
linux-arm-kernel, linux-perf-users, linux-kernel, graham.woodward,
mike.leach
On Fri, Oct 24, 2025 at 09:27:25AM +0300, Adrian Hunter wrote:
[...]
> > --- a/tools/perf/util/arm-spe.c
> > +++ b/tools/perf/util/arm-spe.c
> > @@ -1732,7 +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;
> > + id = evsel->core.id[0] + PERF_SYNTH_EVENT_ID_OFFSET;
> >
> > if (!id)
> > id = 1;
>
> Functionally this is to find a free range of IDs for synthesized events,
> but also the range can only have one user e.g. in this case auxtrace
>
> So add to auxtrace.c
>
> #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;
> }
>
> And then use that:
>
> - /* create new id val to be a fixed offset from evsel id */
> - id = evsel->core.id[0] + 1000000000;
> + id = auxtrace_synth_id_range_start(evsel);
> -
> - if (!id)
> - id = 1;
Agreed, much better than I suggested!
Just remind updating subject as well, like:
perf auxtrace: Refactor AUX event ID calculation
perf auxtrace: Add auxtrace_synth_id_range_start() helper
Thanks,
Leo
^ permalink raw reply [flat|nested] 9+ messages in thread