linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/9] perf auxtrace: Refactor with evsel__is_aux_event()
@ 2024-08-06 20:41 Leo Yan
  2024-08-06 20:41 ` [PATCH v1 1/9] perf auxtrace: Use evsel__is_aux_event() for checking AUX event Leo Yan
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Leo Yan @ 2024-08-06 20:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter,
	James Clark, Suzuki K Poulose, Mike Leach, coresight,
	linux-arm-kernel, linux-kernel, Liang, Kan
  Cc: Leo Yan

As the evsel__is_aux_event() function can be used to detect AUX event,
it is not necessary for every AUX module to maintain a 'pmu_type' field
and compare it for detecting AUX event.

This patch series refactors auxtrace with using evsel__is_aux_event(),
and remove unused the 'pmu_type' field from every AXU module's
structure.

This series has been tested on Arm SPE and Intel PT.


Leo Yan (9):
  perf auxtrace: Use evsel__is_aux_event() for checking AUX event
  perf auxtrace: Remove unused 'pmu' pointer from struct auxtrace_record
  perf auxtrace: Refactor auxtrace__evsel_is_auxtrace()
  perf arm-spe: Remove the 'pmu_type' field
  perf cs-etm: Remove the 'pmu_type' field
  perf hisi-ptt: Remove the unused 'pmu_type' field
  perf intel-bts: Remove the 'pmu_type' field
  perf intel-pt: Remove the 'pmu_type' field
  perf s390-cpumsf: Remove the unused 'pmu_type' field

 tools/perf/arch/arm/util/cs-etm.c     |  1 -
 tools/perf/arch/arm64/util/arm-spe.c  |  1 -
 tools/perf/arch/arm64/util/hisi-ptt.c |  1 -
 tools/perf/arch/x86/util/intel-bts.c  |  1 -
 tools/perf/arch/x86/util/intel-pt.c   |  1 -
 tools/perf/util/arm-spe.c             | 13 +-------
 tools/perf/util/auxtrace.c            |  8 ++---
 tools/perf/util/auxtrace.h            |  3 --
 tools/perf/util/cs-etm.c              | 17 ++--------
 tools/perf/util/hisi-ptt.c            | 11 -------
 tools/perf/util/intel-bts.c           | 14 +-------
 tools/perf/util/intel-pt.c            | 46 ++++++++++-----------------
 tools/perf/util/s390-cpumsf.c         | 11 -------
 13 files changed, 25 insertions(+), 103 deletions(-)

-- 
2.34.1



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v1 1/9] perf auxtrace: Use evsel__is_aux_event() for checking AUX event
  2024-08-06 20:41 [PATCH v1 0/9] perf auxtrace: Refactor with evsel__is_aux_event() Leo Yan
@ 2024-08-06 20:41 ` Leo Yan
  2024-08-09  7:55   ` Adrian Hunter
  2024-08-06 20:41 ` [PATCH v1 2/9] perf auxtrace: Remove unused 'pmu' pointer from struct auxtrace_record Leo Yan
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Leo Yan @ 2024-08-06 20:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter,
	James Clark, Suzuki K Poulose, Mike Leach, coresight,
	linux-arm-kernel, linux-kernel, Liang, Kan
  Cc: Leo Yan

Use evsel__is_aux_event() to decide if an event is a AUX event, this is
a refactoring to replace comparing the PMU type.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/auxtrace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index e2f317063eec..c3f0ef4349fc 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -671,11 +671,11 @@ int auxtrace_record__read_finish(struct auxtrace_record *itr, int idx)
 {
 	struct evsel *evsel;
 
-	if (!itr->evlist || !itr->pmu)
+	if (!itr->evlist)
 		return -EINVAL;
 
 	evlist__for_each_entry(itr->evlist, evsel) {
-		if (evsel->core.attr.type == itr->pmu->type) {
+		if (evsel__is_aux_event(evsel)) {
 			if (evsel->disabled)
 				return 0;
 			return evlist__enable_event_idx(itr->evlist, evsel, idx);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 2/9] perf auxtrace: Remove unused 'pmu' pointer from struct auxtrace_record
  2024-08-06 20:41 [PATCH v1 0/9] perf auxtrace: Refactor with evsel__is_aux_event() Leo Yan
  2024-08-06 20:41 ` [PATCH v1 1/9] perf auxtrace: Use evsel__is_aux_event() for checking AUX event Leo Yan
@ 2024-08-06 20:41 ` Leo Yan
  2024-08-09  8:02   ` Adrian Hunter
  2024-08-06 20:41 ` [PATCH v1 3/9] perf auxtrace: Refactor auxtrace__evsel_is_auxtrace() Leo Yan
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Leo Yan @ 2024-08-06 20:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter,
	James Clark, Suzuki K Poulose, Mike Leach, coresight,
	linux-arm-kernel, linux-kernel, Liang, Kan
  Cc: Leo Yan

The 'pmu' pointer in the auxtrace_record structure is not used after
support multiple AUX events, remove it.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/arch/arm/util/cs-etm.c     | 1 -
 tools/perf/arch/arm64/util/arm-spe.c  | 1 -
 tools/perf/arch/arm64/util/hisi-ptt.c | 1 -
 tools/perf/arch/x86/util/intel-bts.c  | 1 -
 tools/perf/arch/x86/util/intel-pt.c   | 1 -
 tools/perf/util/auxtrace.h            | 1 -
 6 files changed, 6 deletions(-)

diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index da6231367993..96aeb7cdbee1 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -888,7 +888,6 @@ struct auxtrace_record *cs_etm_record_init(int *err)
 	}
 
 	ptr->cs_etm_pmu			= cs_etm_pmu;
-	ptr->itr.pmu			= cs_etm_pmu;
 	ptr->itr.parse_snapshot_options	= cs_etm_parse_snapshot_options;
 	ptr->itr.recording_options	= cs_etm_recording_options;
 	ptr->itr.info_priv_size		= cs_etm_info_priv_size;
diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index d59f6ca499f2..2be99fdf997d 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -514,7 +514,6 @@ struct auxtrace_record *arm_spe_recording_init(int *err,
 	}
 
 	sper->arm_spe_pmu = arm_spe_pmu;
-	sper->itr.pmu = arm_spe_pmu;
 	sper->itr.snapshot_start = arm_spe_snapshot_start;
 	sper->itr.snapshot_finish = arm_spe_snapshot_finish;
 	sper->itr.find_snapshot = arm_spe_find_snapshot;
diff --git a/tools/perf/arch/arm64/util/hisi-ptt.c b/tools/perf/arch/arm64/util/hisi-ptt.c
index ba97c8a562a0..eac9739c87e6 100644
--- a/tools/perf/arch/arm64/util/hisi-ptt.c
+++ b/tools/perf/arch/arm64/util/hisi-ptt.c
@@ -174,7 +174,6 @@ struct auxtrace_record *hisi_ptt_recording_init(int *err,
 	}
 
 	pttr->hisi_ptt_pmu = hisi_ptt_pmu;
-	pttr->itr.pmu = hisi_ptt_pmu;
 	pttr->itr.recording_options = hisi_ptt_recording_options;
 	pttr->itr.info_priv_size = hisi_ptt_info_priv_size;
 	pttr->itr.info_fill = hisi_ptt_info_fill;
diff --git a/tools/perf/arch/x86/util/intel-bts.c b/tools/perf/arch/x86/util/intel-bts.c
index 34696f3d3d5d..85c8186300c8 100644
--- a/tools/perf/arch/x86/util/intel-bts.c
+++ b/tools/perf/arch/x86/util/intel-bts.c
@@ -434,7 +434,6 @@ struct auxtrace_record *intel_bts_recording_init(int *err)
 	}
 
 	btsr->intel_bts_pmu = intel_bts_pmu;
-	btsr->itr.pmu = intel_bts_pmu;
 	btsr->itr.recording_options = intel_bts_recording_options;
 	btsr->itr.info_priv_size = intel_bts_info_priv_size;
 	btsr->itr.info_fill = intel_bts_info_fill;
diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
index 4b710e875953..ea510a7486b1 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -1197,7 +1197,6 @@ struct auxtrace_record *intel_pt_recording_init(int *err)
 	}
 
 	ptr->intel_pt_pmu = intel_pt_pmu;
-	ptr->itr.pmu = intel_pt_pmu;
 	ptr->itr.recording_options = intel_pt_recording_options;
 	ptr->itr.info_priv_size = intel_pt_info_priv_size;
 	ptr->itr.info_fill = intel_pt_info_fill;
diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
index 8a6ec9565835..95304368103b 100644
--- a/tools/perf/util/auxtrace.h
+++ b/tools/perf/util/auxtrace.h
@@ -411,7 +411,6 @@ struct auxtrace_record {
 	int (*read_finish)(struct auxtrace_record *itr, int idx);
 	unsigned int alignment;
 	unsigned int default_aux_sample_size;
-	struct perf_pmu *pmu;
 	struct evlist *evlist;
 };
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 3/9] perf auxtrace: Refactor auxtrace__evsel_is_auxtrace()
  2024-08-06 20:41 [PATCH v1 0/9] perf auxtrace: Refactor with evsel__is_aux_event() Leo Yan
  2024-08-06 20:41 ` [PATCH v1 1/9] perf auxtrace: Use evsel__is_aux_event() for checking AUX event Leo Yan
  2024-08-06 20:41 ` [PATCH v1 2/9] perf auxtrace: Remove unused 'pmu' pointer from struct auxtrace_record Leo Yan
@ 2024-08-06 20:41 ` Leo Yan
  2024-08-08 12:58   ` Adrian Hunter
  2024-08-06 20:41 ` [PATCH v1 4/9] perf arm-spe: Remove the 'pmu_type' field Leo Yan
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Leo Yan @ 2024-08-06 20:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter,
	James Clark, Suzuki K Poulose, Mike Leach, coresight,
	linux-arm-kernel, linux-kernel, Liang, Kan
  Cc: Leo Yan

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.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/arm-spe.c     |  9 ---------
 tools/perf/util/auxtrace.c    |  4 ++--
 tools/perf/util/auxtrace.h    |  2 --
 tools/perf/util/cs-etm.c      | 13 +------------
 tools/perf/util/hisi-ptt.c    |  9 ---------
 tools/perf/util/intel-bts.c   | 10 ----------
 tools/perf/util/intel-pt.c    | 10 ----------
 tools/perf/util/s390-cpumsf.c |  9 ---------
 8 files changed, 3 insertions(+), 63 deletions(-)

diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
index afbd5869f6bf..27e393a0beec 100644
--- a/tools/perf/util/arm-spe.c
+++ b/tools/perf/util/arm-spe.c
@@ -1053,14 +1053,6 @@ static void arm_spe_free(struct perf_session *session)
 	free(spe);
 }
 
-static bool arm_spe_evsel_is_auxtrace(struct perf_session *session,
-				      struct evsel *evsel)
-{
-	struct arm_spe *spe = container_of(session->auxtrace, struct arm_spe, auxtrace);
-
-	return evsel->core.attr.type == spe->pmu_type;
-}
-
 static const char * const arm_spe_info_fmts[] = {
 	[ARM_SPE_PMU_TYPE]		= "  PMU Type           %"PRId64"\n",
 };
@@ -1344,7 +1336,6 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
 	spe->auxtrace.flush_events = arm_spe_flush;
 	spe->auxtrace.free_events = arm_spe_free_events;
 	spe->auxtrace.free = arm_spe_free;
-	spe->auxtrace.evsel_is_auxtrace = arm_spe_evsel_is_auxtrace;
 	session->auxtrace = &spe->auxtrace;
 
 	arm_spe_print_info(&auxtrace_info->priv[0]);
diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index c3f0ef4349fc..03462ff346e7 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -2874,8 +2874,8 @@ void auxtrace__free(struct perf_session *session)
 bool auxtrace__evsel_is_auxtrace(struct perf_session *session,
 				 struct evsel *evsel)
 {
-	if (!session->auxtrace || !session->auxtrace->evsel_is_auxtrace)
+	if (!session->auxtrace)
 		return false;
 
-	return session->auxtrace->evsel_is_auxtrace(session, evsel);
+	return evsel__is_aux_event(evsel);
 }
diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
index 95304368103b..f13c2de17653 100644
--- a/tools/perf/util/auxtrace.h
+++ b/tools/perf/util/auxtrace.h
@@ -221,8 +221,6 @@ struct auxtrace {
 			    struct perf_tool *tool);
 	void (*free_events)(struct perf_session *session);
 	void (*free)(struct perf_session *session);
-	bool (*evsel_is_auxtrace)(struct perf_session *session,
-				  struct evsel *evsel);
 };
 
 /**
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index d3e9c64d17d4..dac0f7c7e44d 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -899,16 +899,6 @@ static void cs_etm__free(struct perf_session *session)
 	zfree(&aux);
 }
 
-static bool cs_etm__evsel_is_auxtrace(struct perf_session *session,
-				      struct evsel *evsel)
-{
-	struct cs_etm_auxtrace *aux = container_of(session->auxtrace,
-						   struct cs_etm_auxtrace,
-						   auxtrace);
-
-	return evsel->core.attr.type == aux->pmu_type;
-}
-
 static struct machine *cs_etm__get_machine(struct cs_etm_queue *etmq,
 					   ocsd_ex_level el)
 {
@@ -2877,7 +2867,7 @@ static int cs_etm__setup_timeless_decoding(struct cs_etm_auxtrace *etm)
 	 * Find the cs_etm evsel and look at what its timestamp setting was
 	 */
 	evlist__for_each_entry(evlist, evsel)
-		if (cs_etm__evsel_is_auxtrace(etm->session, evsel)) {
+		if (evsel__is_aux_event(evsel)) {
 			etm->timeless_decoding =
 				!(evsel->core.attr.config & BIT(ETM_OPT_TS));
 			return 0;
@@ -3380,7 +3370,6 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
 	etm->auxtrace.flush_events = cs_etm__flush_events;
 	etm->auxtrace.free_events = cs_etm__free_events;
 	etm->auxtrace.free = cs_etm__free;
-	etm->auxtrace.evsel_is_auxtrace = cs_etm__evsel_is_auxtrace;
 	session->auxtrace = &etm->auxtrace;
 
 	err = cs_etm__setup_timeless_decoding(etm);
diff --git a/tools/perf/util/hisi-ptt.c b/tools/perf/util/hisi-ptt.c
index 37ea987017f6..57dd98afb0d2 100644
--- a/tools/perf/util/hisi-ptt.c
+++ b/tools/perf/util/hisi-ptt.c
@@ -141,14 +141,6 @@ static void hisi_ptt_free(struct perf_session *session)
 	free(ptt);
 }
 
-static bool hisi_ptt_evsel_is_auxtrace(struct perf_session *session,
-				       struct evsel *evsel)
-{
-	struct hisi_ptt *ptt = container_of(session->auxtrace, struct hisi_ptt, auxtrace);
-
-	return evsel->core.attr.type == ptt->pmu_type;
-}
-
 static void hisi_ptt_print_info(__u64 type)
 {
 	if (!dump_trace)
@@ -181,7 +173,6 @@ int hisi_ptt_process_auxtrace_info(union perf_event *event,
 	ptt->auxtrace.flush_events = hisi_ptt_flush;
 	ptt->auxtrace.free_events = hisi_ptt_free_events;
 	ptt->auxtrace.free = hisi_ptt_free;
-	ptt->auxtrace.evsel_is_auxtrace = hisi_ptt_evsel_is_auxtrace;
 	session->auxtrace = &ptt->auxtrace;
 
 	hisi_ptt_print_info(auxtrace_info->priv[0]);
diff --git a/tools/perf/util/intel-bts.c b/tools/perf/util/intel-bts.c
index ec1b3bd9f530..2b571e56f9c9 100644
--- a/tools/perf/util/intel-bts.c
+++ b/tools/perf/util/intel-bts.c
@@ -728,15 +728,6 @@ static void intel_bts_free(struct perf_session *session)
 	free(bts);
 }
 
-static bool intel_bts_evsel_is_auxtrace(struct perf_session *session,
-					struct evsel *evsel)
-{
-	struct intel_bts *bts = container_of(session->auxtrace, struct intel_bts,
-					     auxtrace);
-
-	return evsel->core.attr.type == bts->pmu_type;
-}
-
 struct intel_bts_synth {
 	struct perf_tool dummy_tool;
 	struct perf_session *session;
@@ -892,7 +883,6 @@ int intel_bts_process_auxtrace_info(union perf_event *event,
 	bts->auxtrace.flush_events = intel_bts_flush;
 	bts->auxtrace.free_events = intel_bts_free_events;
 	bts->auxtrace.free = intel_bts_free;
-	bts->auxtrace.evsel_is_auxtrace = intel_bts_evsel_is_auxtrace;
 	session->auxtrace = &bts->auxtrace;
 
 	intel_bts_print_info(&auxtrace_info->priv[0], INTEL_BTS_PMU_TYPE,
diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
index d6d7b7512505..1608d0e38679 100644
--- a/tools/perf/util/intel-pt.c
+++ b/tools/perf/util/intel-pt.c
@@ -3589,15 +3589,6 @@ static void intel_pt_free(struct perf_session *session)
 	free(pt);
 }
 
-static bool intel_pt_evsel_is_auxtrace(struct perf_session *session,
-				       struct evsel *evsel)
-{
-	struct intel_pt *pt = container_of(session->auxtrace, struct intel_pt,
-					   auxtrace);
-
-	return evsel->core.attr.type == pt->pmu_type;
-}
-
 static int intel_pt_process_auxtrace_event(struct perf_session *session,
 					   union perf_event *event,
 					   struct perf_tool *tool __maybe_unused)
@@ -4356,7 +4347,6 @@ int intel_pt_process_auxtrace_info(union perf_event *event,
 	pt->auxtrace.flush_events = intel_pt_flush;
 	pt->auxtrace.free_events = intel_pt_free_events;
 	pt->auxtrace.free = intel_pt_free;
-	pt->auxtrace.evsel_is_auxtrace = intel_pt_evsel_is_auxtrace;
 	session->auxtrace = &pt->auxtrace;
 
 	if (dump_trace)
diff --git a/tools/perf/util/s390-cpumsf.c b/tools/perf/util/s390-cpumsf.c
index 6fe478b0b61b..a9b0dea199d5 100644
--- a/tools/perf/util/s390-cpumsf.c
+++ b/tools/perf/util/s390-cpumsf.c
@@ -1048,14 +1048,6 @@ static void s390_cpumsf_free(struct perf_session *session)
 	free(sf);
 }
 
-static bool
-s390_cpumsf_evsel_is_auxtrace(struct perf_session *session __maybe_unused,
-			      struct evsel *evsel)
-{
-	return evsel->core.attr.type == PERF_TYPE_RAW &&
-	       evsel->core.attr.config == PERF_EVENT_CPUM_SF_DIAG;
-}
-
 static int s390_cpumsf_get_type(const char *cpuid)
 {
 	int ret, family = 0;
@@ -1152,7 +1144,6 @@ int s390_cpumsf_process_auxtrace_info(union perf_event *event,
 	sf->auxtrace.flush_events = s390_cpumsf_flush;
 	sf->auxtrace.free_events = s390_cpumsf_free_events;
 	sf->auxtrace.free = s390_cpumsf_free;
-	sf->auxtrace.evsel_is_auxtrace = s390_cpumsf_evsel_is_auxtrace;
 	session->auxtrace = &sf->auxtrace;
 
 	if (dump_trace)
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 4/9] perf arm-spe: Remove the 'pmu_type' field
  2024-08-06 20:41 [PATCH v1 0/9] perf auxtrace: Refactor with evsel__is_aux_event() Leo Yan
                   ` (2 preceding siblings ...)
  2024-08-06 20:41 ` [PATCH v1 3/9] perf auxtrace: Refactor auxtrace__evsel_is_auxtrace() Leo Yan
@ 2024-08-06 20:41 ` Leo Yan
  2024-08-06 20:41 ` [PATCH v1 5/9] perf cs-etm: " Leo Yan
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Leo Yan @ 2024-08-06 20:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter,
	James Clark, Suzuki K Poulose, Mike Leach, coresight,
	linux-arm-kernel, linux-kernel, Liang, Kan
  Cc: Leo Yan

Use the evsel__is_aux_event() function in the Arm SPE layer to detected
the enabled AUX events. It is safe for this change as it is no chance
for mixing AUX events and only the same kind of AUX events are enabled
during the initialization.

After the refactoring, the 'pmu_type' field is not used, so remove it.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/arm-spe.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
index 27e393a0beec..213320cfcea7 100644
--- a/tools/perf/util/arm-spe.c
+++ b/tools/perf/util/arm-spe.c
@@ -45,7 +45,6 @@ struct arm_spe {
 	u32				auxtrace_type;
 	struct perf_session		*session;
 	struct machine			*machine;
-	u32				pmu_type;
 	u64				midr;
 
 	struct perf_tsc_conversion	tc;
@@ -1120,7 +1119,7 @@ arm_spe_synth_events(struct arm_spe *spe, struct perf_session *session)
 	int err;
 
 	evlist__for_each_entry(evlist, evsel) {
-		if (evsel->core.attr.type == spe->pmu_type) {
+		if (evsel__is_aux_event(evsel)) {
 			found = true;
 			break;
 		}
@@ -1305,7 +1304,6 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
 	spe->session = session;
 	spe->machine = &session->machines.host; /* No kvm support */
 	spe->auxtrace_type = auxtrace_info->type;
-	spe->pmu_type = auxtrace_info->priv[ARM_SPE_PMU_TYPE];
 	spe->midr = midr;
 
 	spe->timeless_decoding = arm_spe__is_timeless_decoding(spe);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 5/9] perf cs-etm: Remove the 'pmu_type' field
  2024-08-06 20:41 [PATCH v1 0/9] perf auxtrace: Refactor with evsel__is_aux_event() Leo Yan
                   ` (3 preceding siblings ...)
  2024-08-06 20:41 ` [PATCH v1 4/9] perf arm-spe: Remove the 'pmu_type' field Leo Yan
@ 2024-08-06 20:41 ` Leo Yan
  2024-08-06 20:41 ` [PATCH v1 6/9] perf hisi-ptt: Remove the unused " Leo Yan
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Leo Yan @ 2024-08-06 20:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter,
	James Clark, Suzuki K Poulose, Mike Leach, coresight,
	linux-arm-kernel, linux-kernel, Liang, Kan
  Cc: Leo Yan

Use the evsel__is_aux_event() function in the Arm CoreSight layer to
detect the enabled AUX events. So the 'pmu_type' field is not used,
remove it.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/cs-etm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index dac0f7c7e44d..13050b1919c8 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -77,7 +77,6 @@ struct cs_etm_auxtrace {
 	u64 instructions_sample_period;
 	u64 instructions_id;
 	u64 **metadata;
-	unsigned int pmu_type;
 	enum cs_etm_pid_fmt pid_fmt;
 };
 
@@ -1629,7 +1628,7 @@ static int cs_etm__synth_events(struct cs_etm_auxtrace *etm,
 	int err;
 
 	evlist__for_each_entry(evlist, evsel) {
-		if (evsel->core.attr.type == etm->pmu_type) {
+		if (evsel__is_aux_event(evsel)) {
 			found = true;
 			break;
 		}
@@ -3338,7 +3337,6 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
 	etm->session = session;
 
 	etm->num_cpu = num_cpu;
-	etm->pmu_type = (unsigned int) ((ptr[CS_PMU_TYPE_CPUS] >> 32) & 0xffffffff);
 	etm->snapshot_mode = (ptr[CS_ETM_SNAPSHOT] != 0);
 	etm->metadata = metadata;
 	etm->auxtrace_type = auxtrace_info->type;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 6/9] perf hisi-ptt: Remove the unused 'pmu_type' field
  2024-08-06 20:41 [PATCH v1 0/9] perf auxtrace: Refactor with evsel__is_aux_event() Leo Yan
                   ` (4 preceding siblings ...)
  2024-08-06 20:41 ` [PATCH v1 5/9] perf cs-etm: " Leo Yan
@ 2024-08-06 20:41 ` Leo Yan
  2024-08-06 20:41 ` [PATCH v1 7/9] perf intel-bts: Remove the " Leo Yan
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Leo Yan @ 2024-08-06 20:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter,
	James Clark, Suzuki K Poulose, Mike Leach, coresight,
	linux-arm-kernel, linux-kernel, Liang, Kan
  Cc: Leo Yan

The 'pmu_type' field is not used, remove it.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/hisi-ptt.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tools/perf/util/hisi-ptt.c b/tools/perf/util/hisi-ptt.c
index 57dd98afb0d2..859e1bc38e12 100644
--- a/tools/perf/util/hisi-ptt.c
+++ b/tools/perf/util/hisi-ptt.c
@@ -32,7 +32,6 @@ struct hisi_ptt {
 	u32 auxtrace_type;
 	struct perf_session *session;
 	struct machine *machine;
-	u32 pmu_type;
 };
 
 static enum hisi_ptt_pkt_type hisi_ptt_check_packet_type(unsigned char *buf)
@@ -166,7 +165,6 @@ int hisi_ptt_process_auxtrace_info(union perf_event *event,
 	ptt->session = session;
 	ptt->machine = &session->machines.host; /* No kvm support */
 	ptt->auxtrace_type = auxtrace_info->type;
-	ptt->pmu_type = auxtrace_info->priv[0];
 
 	ptt->auxtrace.process_event = hisi_ptt_process_event;
 	ptt->auxtrace.process_auxtrace_event = hisi_ptt_process_auxtrace_event;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 7/9] perf intel-bts: Remove the 'pmu_type' field
  2024-08-06 20:41 [PATCH v1 0/9] perf auxtrace: Refactor with evsel__is_aux_event() Leo Yan
                   ` (5 preceding siblings ...)
  2024-08-06 20:41 ` [PATCH v1 6/9] perf hisi-ptt: Remove the unused " Leo Yan
@ 2024-08-06 20:41 ` Leo Yan
  2024-08-06 20:41 ` [PATCH v1 8/9] perf intel-pt: " Leo Yan
  2024-08-06 20:41 ` [PATCH v1 9/9] perf s390-cpumsf: Remove the unused " Leo Yan
  8 siblings, 0 replies; 17+ messages in thread
From: Leo Yan @ 2024-08-06 20:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter,
	James Clark, Suzuki K Poulose, Mike Leach, coresight,
	linux-arm-kernel, linux-kernel, Liang, Kan
  Cc: Leo Yan

Use the evsel__is_aux_event() function to detect the enabled AUX events.
The 'pmu_type' field is not used, remove it.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/intel-bts.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/perf/util/intel-bts.c b/tools/perf/util/intel-bts.c
index 2b571e56f9c9..93f3452f8220 100644
--- a/tools/perf/util/intel-bts.c
+++ b/tools/perf/util/intel-bts.c
@@ -51,7 +51,6 @@ struct intel_bts {
 	bool				sampling_mode;
 	bool				snapshot_mode;
 	bool				data_queued;
-	u32				pmu_type;
 	struct perf_tsc_conversion	tc;
 	bool				cap_user_time_zero;
 	struct itrace_synth_opts	synth_opts;
@@ -768,7 +767,7 @@ static int intel_bts_synth_events(struct intel_bts *bts,
 	int err;
 
 	evlist__for_each_entry(evlist, evsel) {
-		if (evsel->core.attr.type == bts->pmu_type && evsel->core.ids) {
+		if (evsel__is_aux_event(evsel) && evsel->core.ids) {
 			found = true;
 			break;
 		}
@@ -868,7 +867,6 @@ int intel_bts_process_auxtrace_info(union perf_event *event,
 	bts->session = session;
 	bts->machine = &session->machines.host; /* No kvm support */
 	bts->auxtrace_type = auxtrace_info->type;
-	bts->pmu_type = auxtrace_info->priv[INTEL_BTS_PMU_TYPE];
 	bts->tc.time_shift = auxtrace_info->priv[INTEL_BTS_TIME_SHIFT];
 	bts->tc.time_mult = auxtrace_info->priv[INTEL_BTS_TIME_MULT];
 	bts->tc.time_zero = auxtrace_info->priv[INTEL_BTS_TIME_ZERO];
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 8/9] perf intel-pt: Remove the 'pmu_type' field
  2024-08-06 20:41 [PATCH v1 0/9] perf auxtrace: Refactor with evsel__is_aux_event() Leo Yan
                   ` (6 preceding siblings ...)
  2024-08-06 20:41 ` [PATCH v1 7/9] perf intel-bts: Remove the " Leo Yan
@ 2024-08-06 20:41 ` Leo Yan
  2024-08-06 20:41 ` [PATCH v1 9/9] perf s390-cpumsf: Remove the unused " Leo Yan
  8 siblings, 0 replies; 17+ messages in thread
From: Leo Yan @ 2024-08-06 20:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter,
	James Clark, Suzuki K Poulose, Mike Leach, coresight,
	linux-arm-kernel, linux-kernel, Liang, Kan
  Cc: Leo Yan

Use the evsel__is_aux_event() function to detect the enabled AUX events.
The 'pmu_type' field is not used, remove it.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/intel-pt.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
index 1608d0e38679..69b664059c2d 100644
--- a/tools/perf/util/intel-pt.c
+++ b/tools/perf/util/intel-pt.c
@@ -84,7 +84,6 @@ struct intel_pt {
 	unsigned int br_stack_sz;
 	unsigned int br_stack_sz_plus;
 	int have_sched_switch;
-	u32 pmu_type;
 	u64 kernel_start;
 	u64 switch_ip;
 	u64 ptss_ip;
@@ -1020,10 +1019,11 @@ static bool intel_pt_pgd_ip(uint64_t ip, void *data)
 	return __intel_pt_pgd_ip(ip, data) > 0;
 }
 
-static bool intel_pt_get_config(struct intel_pt *pt,
-				struct perf_event_attr *attr, u64 *config)
+static bool intel_pt_get_config(struct evsel *evsel, u64 *config)
 {
-	if (attr->type == pt->pmu_type) {
+	struct perf_event_attr *attr = &evsel->core.attr;
+
+	if (evsel__is_aux_event(evsel)) {
 		if (config)
 			*config = attr->config;
 		return true;
@@ -1037,7 +1037,7 @@ static bool intel_pt_exclude_kernel(struct intel_pt *pt)
 	struct evsel *evsel;
 
 	evlist__for_each_entry(pt->session->evlist, evsel) {
-		if (intel_pt_get_config(pt, &evsel->core.attr, NULL) &&
+		if (intel_pt_get_config(evsel, NULL) &&
 		    !evsel->core.attr.exclude_kernel)
 			return false;
 	}
@@ -1053,7 +1053,7 @@ static bool intel_pt_return_compression(struct intel_pt *pt)
 		return true;
 
 	evlist__for_each_entry(pt->session->evlist, evsel) {
-		if (intel_pt_get_config(pt, &evsel->core.attr, &config) &&
+		if (intel_pt_get_config(evsel, &config) &&
 		    (config & pt->noretcomp_bit))
 			return false;
 	}
@@ -1066,7 +1066,7 @@ static bool intel_pt_branch_enable(struct intel_pt *pt)
 	u64 config;
 
 	evlist__for_each_entry(pt->session->evlist, evsel) {
-		if (intel_pt_get_config(pt, &evsel->core.attr, &config) &&
+		if (intel_pt_get_config(evsel, &config) &&
 		    (config & INTEL_PT_CFG_PASS_THRU) &&
 		    !(config & INTEL_PT_CFG_BRANCH_EN))
 			return false;
@@ -1080,7 +1080,7 @@ static bool intel_pt_disabled_tnt(struct intel_pt *pt)
 	u64 config;
 
 	evlist__for_each_entry(pt->session->evlist, evsel) {
-		if (intel_pt_get_config(pt, &evsel->core.attr, &config) &&
+		if (intel_pt_get_config(evsel, &config) &&
 		    config & INTEL_PT_CFG_TNT_DIS)
 			return true;
 	}
@@ -1100,7 +1100,7 @@ static unsigned int intel_pt_mtc_period(struct intel_pt *pt)
 		config >>= 1;
 
 	evlist__for_each_entry(pt->session->evlist, evsel) {
-		if (intel_pt_get_config(pt, &evsel->core.attr, &config))
+		if (intel_pt_get_config(evsel, &config))
 			return (config & pt->mtc_freq_bits) >> shift;
 	}
 	return 0;
@@ -1118,7 +1118,7 @@ static bool intel_pt_timeless_decoding(struct intel_pt *pt)
 	evlist__for_each_entry(pt->session->evlist, evsel) {
 		if (!(evsel->core.attr.sample_type & PERF_SAMPLE_TIME))
 			return true;
-		if (intel_pt_get_config(pt, &evsel->core.attr, &config)) {
+		if (intel_pt_get_config(evsel, &config)) {
 			if (config & pt->tsc_bit)
 				timeless_decoding = false;
 			else
@@ -1133,7 +1133,7 @@ static bool intel_pt_tracing_kernel(struct intel_pt *pt)
 	struct evsel *evsel;
 
 	evlist__for_each_entry(pt->session->evlist, evsel) {
-		if (intel_pt_get_config(pt, &evsel->core.attr, NULL) &&
+		if (intel_pt_get_config(evsel, NULL) &&
 		    !evsel->core.attr.exclude_kernel)
 			return true;
 	}
@@ -1150,7 +1150,7 @@ static bool intel_pt_have_tsc(struct intel_pt *pt)
 		return false;
 
 	evlist__for_each_entry(pt->session->evlist, evsel) {
-		if (intel_pt_get_config(pt, &evsel->core.attr, &config)) {
+		if (intel_pt_get_config(evsel, &config)) {
 			if (config & pt->tsc_bit)
 				have_tsc = true;
 			else
@@ -1166,7 +1166,7 @@ static bool intel_pt_have_mtc(struct intel_pt *pt)
 	u64 config;
 
 	evlist__for_each_entry(pt->session->evlist, evsel) {
-		if (intel_pt_get_config(pt, &evsel->core.attr, &config) &&
+		if (intel_pt_get_config(evsel, &config) &&
 		    (config & pt->mtc_bit))
 			return true;
 	}
@@ -1191,7 +1191,7 @@ static u64 intel_pt_ctl(struct intel_pt *pt)
 	u64 config;
 
 	evlist__for_each_entry(pt->session->evlist, evsel) {
-		if (intel_pt_get_config(pt, &evsel->core.attr, &config))
+		if (intel_pt_get_config(evsel, &config))
 			return config;
 	}
 	return 0;
@@ -3703,13 +3703,12 @@ static void intel_pt_set_event_name(struct evlist *evlist, u64 id,
 	}
 }
 
-static struct evsel *intel_pt_evsel(struct intel_pt *pt,
-					 struct evlist *evlist)
+static struct evsel *intel_pt_evsel(struct evlist *evlist)
 {
 	struct evsel *evsel;
 
 	evlist__for_each_entry(evlist, evsel) {
-		if (evsel->core.attr.type == pt->pmu_type && evsel->core.ids)
+		if (evsel__is_aux_event(evsel) && evsel->core.ids)
 			return evsel;
 	}
 
@@ -3720,7 +3719,7 @@ static int intel_pt_synth_events(struct intel_pt *pt,
 				 struct perf_session *session)
 {
 	struct evlist *evlist = session->evlist;
-	struct evsel *evsel = intel_pt_evsel(pt, evlist);
+	struct evsel *evsel = intel_pt_evsel(evlist);
 	struct perf_event_attr attr;
 	u64 id;
 	int err;
@@ -4219,7 +4218,6 @@ int intel_pt_process_auxtrace_info(union perf_event *event,
 	pt->session = session;
 	pt->machine = &session->machines.host; /* No kvm support */
 	pt->auxtrace_type = auxtrace_info->type;
-	pt->pmu_type = auxtrace_info->priv[INTEL_PT_PMU_TYPE];
 	pt->tc.time_shift = auxtrace_info->priv[INTEL_PT_TIME_SHIFT];
 	pt->tc.time_mult = auxtrace_info->priv[INTEL_PT_TIME_MULT];
 	pt->tc.time_zero = auxtrace_info->priv[INTEL_PT_TIME_ZERO];
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 9/9] perf s390-cpumsf: Remove the unused 'pmu_type' field
  2024-08-06 20:41 [PATCH v1 0/9] perf auxtrace: Refactor with evsel__is_aux_event() Leo Yan
                   ` (7 preceding siblings ...)
  2024-08-06 20:41 ` [PATCH v1 8/9] perf intel-pt: " Leo Yan
@ 2024-08-06 20:41 ` Leo Yan
  8 siblings, 0 replies; 17+ messages in thread
From: Leo Yan @ 2024-08-06 20:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter,
	James Clark, Suzuki K Poulose, Mike Leach, coresight,
	linux-arm-kernel, linux-kernel, Liang, Kan
  Cc: Leo Yan

The 'pmu_type' field is not used, remove it.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/s390-cpumsf.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tools/perf/util/s390-cpumsf.c b/tools/perf/util/s390-cpumsf.c
index a9b0dea199d5..3d6253a4c5cf 100644
--- a/tools/perf/util/s390-cpumsf.c
+++ b/tools/perf/util/s390-cpumsf.c
@@ -172,7 +172,6 @@ struct s390_cpumsf {
 	struct perf_session	*session;
 	struct machine		*machine;
 	u32			auxtrace_type;
-	u32			pmu_type;
 	u16			machine_type;
 	bool			data_queued;
 	bool			use_logfile;
@@ -1136,7 +1135,6 @@ int s390_cpumsf_process_auxtrace_info(union perf_event *event,
 	sf->session = session;
 	sf->machine = &session->machines.host; /* No kvm support */
 	sf->auxtrace_type = auxtrace_info->type;
-	sf->pmu_type = PERF_TYPE_RAW;
 	sf->machine_type = s390_cpumsf_get_type(session->evlist->env->cpuid);
 
 	sf->auxtrace.process_event = s390_cpumsf_process_event;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 3/9] perf auxtrace: Refactor auxtrace__evsel_is_auxtrace()
  2024-08-06 20:41 ` [PATCH v1 3/9] perf auxtrace: Refactor auxtrace__evsel_is_auxtrace() Leo Yan
@ 2024-08-08 12:58   ` Adrian Hunter
  2024-08-09 10:08     ` James Clark
  0 siblings, 1 reply; 17+ messages in thread
From: Adrian Hunter @ 2024-08-08 12:58 UTC (permalink / raw)
  To: Leo Yan, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers,
	James Clark, Suzuki K Poulose, Mike Leach, coresight,
	linux-arm-kernel, linux-kernel, Liang, Kan

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.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/9] perf auxtrace: Use evsel__is_aux_event() for checking AUX event
  2024-08-06 20:41 ` [PATCH v1 1/9] perf auxtrace: Use evsel__is_aux_event() for checking AUX event Leo Yan
@ 2024-08-09  7:55   ` Adrian Hunter
  0 siblings, 0 replies; 17+ messages in thread
From: Adrian Hunter @ 2024-08-09  7:55 UTC (permalink / raw)
  To: Leo Yan, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers,
	James Clark, Suzuki K Poulose, Mike Leach, coresight,
	linux-arm-kernel, linux-kernel, Liang, Kan

On 6/08/24 23:41, Leo Yan wrote:
> Use evsel__is_aux_event() to decide if an event is a AUX event, this is
> a refactoring to replace comparing the PMU type.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  tools/perf/util/auxtrace.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index e2f317063eec..c3f0ef4349fc 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -671,11 +671,11 @@ int auxtrace_record__read_finish(struct auxtrace_record *itr, int idx)
>  {
>  	struct evsel *evsel;
>  
> -	if (!itr->evlist || !itr->pmu)
> +	if (!itr->evlist)
>  		return -EINVAL;
>  
>  	evlist__for_each_entry(itr->evlist, evsel) {
> -		if (evsel->core.attr.type == itr->pmu->type) {
> +		if (evsel__is_aux_event(evsel)) {
>  			if (evsel->disabled)
>  				return 0;
>  			return evlist__enable_event_idx(itr->evlist, evsel, idx);



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/9] perf auxtrace: Remove unused 'pmu' pointer from struct auxtrace_record
  2024-08-06 20:41 ` [PATCH v1 2/9] perf auxtrace: Remove unused 'pmu' pointer from struct auxtrace_record Leo Yan
@ 2024-08-09  8:02   ` Adrian Hunter
  2024-08-28 21:16     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 17+ messages in thread
From: Adrian Hunter @ 2024-08-09  8:02 UTC (permalink / raw)
  To: Leo Yan, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers,
	James Clark, Suzuki K Poulose, Mike Leach, coresight,
	linux-arm-kernel, linux-kernel, Liang, Kan

On 6/08/24 23:41, Leo Yan wrote:
> The 'pmu' pointer in the auxtrace_record structure is not used after
> support multiple AUX events, remove it.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  tools/perf/arch/arm/util/cs-etm.c     | 1 -
>  tools/perf/arch/arm64/util/arm-spe.c  | 1 -
>  tools/perf/arch/arm64/util/hisi-ptt.c | 1 -
>  tools/perf/arch/x86/util/intel-bts.c  | 1 -
>  tools/perf/arch/x86/util/intel-pt.c   | 1 -
>  tools/perf/util/auxtrace.h            | 1 -
>  6 files changed, 6 deletions(-)
> 
> diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
> index da6231367993..96aeb7cdbee1 100644
> --- a/tools/perf/arch/arm/util/cs-etm.c
> +++ b/tools/perf/arch/arm/util/cs-etm.c
> @@ -888,7 +888,6 @@ struct auxtrace_record *cs_etm_record_init(int *err)
>  	}
>  
>  	ptr->cs_etm_pmu			= cs_etm_pmu;
> -	ptr->itr.pmu			= cs_etm_pmu;
>  	ptr->itr.parse_snapshot_options	= cs_etm_parse_snapshot_options;
>  	ptr->itr.recording_options	= cs_etm_recording_options;
>  	ptr->itr.info_priv_size		= cs_etm_info_priv_size;
> diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
> index d59f6ca499f2..2be99fdf997d 100644
> --- a/tools/perf/arch/arm64/util/arm-spe.c
> +++ b/tools/perf/arch/arm64/util/arm-spe.c
> @@ -514,7 +514,6 @@ struct auxtrace_record *arm_spe_recording_init(int *err,
>  	}
>  
>  	sper->arm_spe_pmu = arm_spe_pmu;
> -	sper->itr.pmu = arm_spe_pmu;
>  	sper->itr.snapshot_start = arm_spe_snapshot_start;
>  	sper->itr.snapshot_finish = arm_spe_snapshot_finish;
>  	sper->itr.find_snapshot = arm_spe_find_snapshot;
> diff --git a/tools/perf/arch/arm64/util/hisi-ptt.c b/tools/perf/arch/arm64/util/hisi-ptt.c
> index ba97c8a562a0..eac9739c87e6 100644
> --- a/tools/perf/arch/arm64/util/hisi-ptt.c
> +++ b/tools/perf/arch/arm64/util/hisi-ptt.c
> @@ -174,7 +174,6 @@ struct auxtrace_record *hisi_ptt_recording_init(int *err,
>  	}
>  
>  	pttr->hisi_ptt_pmu = hisi_ptt_pmu;
> -	pttr->itr.pmu = hisi_ptt_pmu;
>  	pttr->itr.recording_options = hisi_ptt_recording_options;
>  	pttr->itr.info_priv_size = hisi_ptt_info_priv_size;
>  	pttr->itr.info_fill = hisi_ptt_info_fill;
> diff --git a/tools/perf/arch/x86/util/intel-bts.c b/tools/perf/arch/x86/util/intel-bts.c
> index 34696f3d3d5d..85c8186300c8 100644
> --- a/tools/perf/arch/x86/util/intel-bts.c
> +++ b/tools/perf/arch/x86/util/intel-bts.c
> @@ -434,7 +434,6 @@ struct auxtrace_record *intel_bts_recording_init(int *err)
>  	}
>  
>  	btsr->intel_bts_pmu = intel_bts_pmu;
> -	btsr->itr.pmu = intel_bts_pmu;
>  	btsr->itr.recording_options = intel_bts_recording_options;
>  	btsr->itr.info_priv_size = intel_bts_info_priv_size;
>  	btsr->itr.info_fill = intel_bts_info_fill;
> diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
> index 4b710e875953..ea510a7486b1 100644
> --- a/tools/perf/arch/x86/util/intel-pt.c
> +++ b/tools/perf/arch/x86/util/intel-pt.c
> @@ -1197,7 +1197,6 @@ struct auxtrace_record *intel_pt_recording_init(int *err)
>  	}
>  
>  	ptr->intel_pt_pmu = intel_pt_pmu;
> -	ptr->itr.pmu = intel_pt_pmu;
>  	ptr->itr.recording_options = intel_pt_recording_options;
>  	ptr->itr.info_priv_size = intel_pt_info_priv_size;
>  	ptr->itr.info_fill = intel_pt_info_fill;
> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> index 8a6ec9565835..95304368103b 100644
> --- a/tools/perf/util/auxtrace.h
> +++ b/tools/perf/util/auxtrace.h
> @@ -411,7 +411,6 @@ struct auxtrace_record {
>  	int (*read_finish)(struct auxtrace_record *itr, int idx);
>  	unsigned int alignment;
>  	unsigned int default_aux_sample_size;
> -	struct perf_pmu *pmu;
>  	struct evlist *evlist;
>  };
>  



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 3/9] perf auxtrace: Refactor auxtrace__evsel_is_auxtrace()
  2024-08-08 12:58   ` Adrian Hunter
@ 2024-08-09 10:08     ` James Clark
  2024-08-09 10:57       ` Leo Yan
  0 siblings, 1 reply; 17+ messages in thread
From: James Clark @ 2024-08-09 10:08 UTC (permalink / raw)
  To: Adrian Hunter, Leo Yan
  Cc: Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers,
	Suzuki K Poulose, Mike Leach, coresight, linux-arm-kernel,
	linux-kernel, Liang, Kan



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.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 3/9] perf auxtrace: Refactor auxtrace__evsel_is_auxtrace()
  2024-08-09 10:08     ` James Clark
@ 2024-08-09 10:57       ` Leo Yan
  0 siblings, 0 replies; 17+ messages in thread
From: Leo Yan @ 2024-08-09 10:57 UTC (permalink / raw)
  To: James Clark, Adrian Hunter
  Cc: Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers,
	Suzuki K Poulose, Mike Leach, coresight, linux-arm-kernel,
	linux-kernel, Liang, Kan

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
  


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/9] perf auxtrace: Remove unused 'pmu' pointer from struct auxtrace_record
  2024-08-09  8:02   ` Adrian Hunter
@ 2024-08-28 21:16     ` Arnaldo Carvalho de Melo
  2024-08-30  7:32       ` Leo Yan
  0 siblings, 1 reply; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-08-28 21:16 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Leo Yan, Namhyung Kim, Ian Rogers, James Clark, Suzuki K Poulose,
	Mike Leach, coresight, linux-arm-kernel, linux-kernel, Liang, Kan

On Fri, Aug 09, 2024 at 11:02:16AM +0300, Adrian Hunter wrote:
> On 6/08/24 23:41, Leo Yan wrote:
> > The 'pmu' pointer in the auxtrace_record structure is not used after
> > support multiple AUX events, remove it.
> > 
> > Signed-off-by: Leo Yan <leo.yan@arm.com>
> 
> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

Applied the first reviewed two patches.

- Arnaldo

> > ---
> >  tools/perf/arch/arm/util/cs-etm.c     | 1 -
> >  tools/perf/arch/arm64/util/arm-spe.c  | 1 -
> >  tools/perf/arch/arm64/util/hisi-ptt.c | 1 -
> >  tools/perf/arch/x86/util/intel-bts.c  | 1 -
> >  tools/perf/arch/x86/util/intel-pt.c   | 1 -
> >  tools/perf/util/auxtrace.h            | 1 -
> >  6 files changed, 6 deletions(-)
> > 
> > diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
> > index da6231367993..96aeb7cdbee1 100644
> > --- a/tools/perf/arch/arm/util/cs-etm.c
> > +++ b/tools/perf/arch/arm/util/cs-etm.c
> > @@ -888,7 +888,6 @@ struct auxtrace_record *cs_etm_record_init(int *err)
> >  	}
> >  
> >  	ptr->cs_etm_pmu			= cs_etm_pmu;
> > -	ptr->itr.pmu			= cs_etm_pmu;
> >  	ptr->itr.parse_snapshot_options	= cs_etm_parse_snapshot_options;
> >  	ptr->itr.recording_options	= cs_etm_recording_options;
> >  	ptr->itr.info_priv_size		= cs_etm_info_priv_size;
> > diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
> > index d59f6ca499f2..2be99fdf997d 100644
> > --- a/tools/perf/arch/arm64/util/arm-spe.c
> > +++ b/tools/perf/arch/arm64/util/arm-spe.c
> > @@ -514,7 +514,6 @@ struct auxtrace_record *arm_spe_recording_init(int *err,
> >  	}
> >  
> >  	sper->arm_spe_pmu = arm_spe_pmu;
> > -	sper->itr.pmu = arm_spe_pmu;
> >  	sper->itr.snapshot_start = arm_spe_snapshot_start;
> >  	sper->itr.snapshot_finish = arm_spe_snapshot_finish;
> >  	sper->itr.find_snapshot = arm_spe_find_snapshot;
> > diff --git a/tools/perf/arch/arm64/util/hisi-ptt.c b/tools/perf/arch/arm64/util/hisi-ptt.c
> > index ba97c8a562a0..eac9739c87e6 100644
> > --- a/tools/perf/arch/arm64/util/hisi-ptt.c
> > +++ b/tools/perf/arch/arm64/util/hisi-ptt.c
> > @@ -174,7 +174,6 @@ struct auxtrace_record *hisi_ptt_recording_init(int *err,
> >  	}
> >  
> >  	pttr->hisi_ptt_pmu = hisi_ptt_pmu;
> > -	pttr->itr.pmu = hisi_ptt_pmu;
> >  	pttr->itr.recording_options = hisi_ptt_recording_options;
> >  	pttr->itr.info_priv_size = hisi_ptt_info_priv_size;
> >  	pttr->itr.info_fill = hisi_ptt_info_fill;
> > diff --git a/tools/perf/arch/x86/util/intel-bts.c b/tools/perf/arch/x86/util/intel-bts.c
> > index 34696f3d3d5d..85c8186300c8 100644
> > --- a/tools/perf/arch/x86/util/intel-bts.c
> > +++ b/tools/perf/arch/x86/util/intel-bts.c
> > @@ -434,7 +434,6 @@ struct auxtrace_record *intel_bts_recording_init(int *err)
> >  	}
> >  
> >  	btsr->intel_bts_pmu = intel_bts_pmu;
> > -	btsr->itr.pmu = intel_bts_pmu;
> >  	btsr->itr.recording_options = intel_bts_recording_options;
> >  	btsr->itr.info_priv_size = intel_bts_info_priv_size;
> >  	btsr->itr.info_fill = intel_bts_info_fill;
> > diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
> > index 4b710e875953..ea510a7486b1 100644
> > --- a/tools/perf/arch/x86/util/intel-pt.c
> > +++ b/tools/perf/arch/x86/util/intel-pt.c
> > @@ -1197,7 +1197,6 @@ struct auxtrace_record *intel_pt_recording_init(int *err)
> >  	}
> >  
> >  	ptr->intel_pt_pmu = intel_pt_pmu;
> > -	ptr->itr.pmu = intel_pt_pmu;
> >  	ptr->itr.recording_options = intel_pt_recording_options;
> >  	ptr->itr.info_priv_size = intel_pt_info_priv_size;
> >  	ptr->itr.info_fill = intel_pt_info_fill;
> > diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> > index 8a6ec9565835..95304368103b 100644
> > --- a/tools/perf/util/auxtrace.h
> > +++ b/tools/perf/util/auxtrace.h
> > @@ -411,7 +411,6 @@ struct auxtrace_record {
> >  	int (*read_finish)(struct auxtrace_record *itr, int idx);
> >  	unsigned int alignment;
> >  	unsigned int default_aux_sample_size;
> > -	struct perf_pmu *pmu;
> >  	struct evlist *evlist;
> >  };
> >  


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/9] perf auxtrace: Remove unused 'pmu' pointer from struct auxtrace_record
  2024-08-28 21:16     ` Arnaldo Carvalho de Melo
@ 2024-08-30  7:32       ` Leo Yan
  0 siblings, 0 replies; 17+ messages in thread
From: Leo Yan @ 2024-08-30  7:32 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Adrian Hunter
  Cc: Namhyung Kim, Ian Rogers, James Clark, Suzuki K Poulose,
	Mike Leach, coresight, linux-arm-kernel, linux-kernel, Liang, Kan

Hi Arnaldo,

On 8/28/24 22:16, Arnaldo Carvalho de Melo wrote:
> On Fri, Aug 09, 2024 at 11:02:16AM +0300, Adrian Hunter wrote:
>> On 6/08/24 23:41, Leo Yan wrote:
>>> The 'pmu' pointer in the auxtrace_record structure is not used after
>>> support multiple AUX events, remove it.
>>>
>>> Signed-off-by: Leo Yan <leo.yan@arm.com>
>>
>> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> Applied the first reviewed two patches.

I confirmed it is fine to picking up these two patches.

Just note, I combined these two patches into the multiple AUX event series [1].
Once I receive response for that series, I will drop the picked two patches and
send new series.

Thanks,
Leo

[1] https://lore.kernel.org/linux-perf-users/20240823113306.2310957-2-leo.yan@arm.com/


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2024-08-30  7:35 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-06 20:41 [PATCH v1 0/9] perf auxtrace: Refactor with evsel__is_aux_event() Leo Yan
2024-08-06 20:41 ` [PATCH v1 1/9] perf auxtrace: Use evsel__is_aux_event() for checking AUX event Leo Yan
2024-08-09  7:55   ` Adrian Hunter
2024-08-06 20:41 ` [PATCH v1 2/9] perf auxtrace: Remove unused 'pmu' pointer from struct auxtrace_record Leo Yan
2024-08-09  8:02   ` Adrian Hunter
2024-08-28 21:16     ` Arnaldo Carvalho de Melo
2024-08-30  7:32       ` Leo Yan
2024-08-06 20:41 ` [PATCH v1 3/9] perf auxtrace: Refactor auxtrace__evsel_is_auxtrace() Leo Yan
2024-08-08 12:58   ` Adrian Hunter
2024-08-09 10:08     ` James Clark
2024-08-09 10:57       ` Leo Yan
2024-08-06 20:41 ` [PATCH v1 4/9] perf arm-spe: Remove the 'pmu_type' field Leo Yan
2024-08-06 20:41 ` [PATCH v1 5/9] perf cs-etm: " Leo Yan
2024-08-06 20:41 ` [PATCH v1 6/9] perf hisi-ptt: Remove the unused " Leo Yan
2024-08-06 20:41 ` [PATCH v1 7/9] perf intel-bts: Remove the " Leo Yan
2024-08-06 20:41 ` [PATCH v1 8/9] perf intel-pt: " Leo Yan
2024-08-06 20:41 ` [PATCH v1 9/9] perf s390-cpumsf: Remove the unused " Leo Yan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).