linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] perf/tool: minor cleanups
@ 2022-10-25 11:06 Yicong Yang
  2022-10-25 11:06 ` [PATCH 1/2] perf tool: move unit macros to kernel.h Yicong Yang
  2022-10-25 11:06 ` [PATCH 2/2] perf auxtrace: Make auxtrace->flush_events() optional Yicong Yang
  0 siblings, 2 replies; 3+ messages in thread
From: Yicong Yang @ 2022-10-25 11:06 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, John Garry, linux-perf-users
  Cc: Will Deacon, James Clark, Mike Leach, Leo Yan, Mathieu Poirier,
	liuqi6124, jonathan.cameron, alexander.shishkin, prime.zeng,
	linuxarm

From: Yicong Yang <yangyicong@hisilicon.com>

Some cleanups for perf noticed in the previous review of hisi-ptt:

- Duplicated definition of unit marco MiB/KiB
- auxtrace->flush_events() is optional to some PMU and can be improved

Yicong Yang (2):
  perf tool: move unit macros to kernel.h
  perf auxtrace: Make auxtrace->flush_events() optional

 tools/include/linux/kernel.h          | 5 +++++
 tools/perf/arch/arm64/util/arm-spe.c  | 3 ---
 tools/perf/arch/arm64/util/hisi-ptt.c | 3 ---
 tools/perf/arch/x86/util/intel-bts.c  | 5 -----
 tools/perf/arch/x86/util/intel-pt.c   | 5 -----
 tools/perf/util/auxtrace.c            | 2 +-
 tools/perf/util/auxtrace.h            | 2 +-
 tools/perf/util/cs-etm.h              | 3 ---
 tools/perf/util/hisi-ptt.c            | 7 -------
 tools/perf/util/s390-cpumsf.c         | 7 -------
 10 files changed, 7 insertions(+), 35 deletions(-)

-- 
2.24.0


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

* [PATCH 1/2] perf tool: move unit macros to kernel.h
  2022-10-25 11:06 [PATCH 0/2] perf/tool: minor cleanups Yicong Yang
@ 2022-10-25 11:06 ` Yicong Yang
  2022-10-25 11:06 ` [PATCH 2/2] perf auxtrace: Make auxtrace->flush_events() optional Yicong Yang
  1 sibling, 0 replies; 3+ messages in thread
From: Yicong Yang @ 2022-10-25 11:06 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, John Garry, linux-perf-users
  Cc: Will Deacon, James Clark, Mike Leach, Leo Yan, Mathieu Poirier,
	liuqi6124, jonathan.cameron, alexander.shishkin, prime.zeng,
	linuxarm

From: Yicong Yang <yangyicong@hisilicon.com>

Unit macros like MiB/KiB are defined separately in several places:

$ egrep -rn "#define MiB|#define KiB" ./
./arch/arm64/util/hisi-ptt.c:27:#define KiB(x) ((x) * 1024)
./arch/arm64/util/hisi-ptt.c:28:#define MiB(x) ((x) * 1024 * 1024)
./arch/arm64/util/arm-spe.c:28:#define KiB(x) ((x) * 1024)
./arch/arm64/util/arm-spe.c:29:#define MiB(x) ((x) * 1024 * 1024)
./arch/x86/util/intel-bts.c:28:#define KiB(x) ((x) * 1024)
./arch/x86/util/intel-bts.c:29:#define MiB(x) ((x) * 1024 * 1024)
./arch/x86/util/intel-bts.c:30:#define KiB_MASK(x) (KiB(x) - 1)
./arch/x86/util/intel-bts.c:31:#define MiB_MASK(x) (MiB(x) - 1)
./arch/x86/util/intel-pt.c:36:#define KiB(x) ((x) * 1024)
./arch/x86/util/intel-pt.c:37:#define MiB(x) ((x) * 1024 * 1024)
./arch/x86/util/intel-pt.c:38:#define KiB_MASK(x) (KiB(x) - 1)
./arch/x86/util/intel-pt.c:39:#define MiB_MASK(x) (MiB(x) - 1)
./util/cs-etm.h:188:#define KiB(x) ((x) * 1024)
./util/cs-etm.h:189:#define MiB(x) ((x) * 1024 * 1024)

Move them to a common place to avoid duplicated definition.

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 tools/include/linux/kernel.h          | 5 +++++
 tools/perf/arch/arm64/util/arm-spe.c  | 3 ---
 tools/perf/arch/arm64/util/hisi-ptt.c | 3 ---
 tools/perf/arch/x86/util/intel-bts.c  | 5 -----
 tools/perf/arch/x86/util/intel-pt.c   | 5 -----
 tools/perf/util/cs-etm.h              | 3 ---
 6 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/tools/include/linux/kernel.h b/tools/include/linux/kernel.h
index 4b0673bf52c2..e03aaa9c8ff1 100644
--- a/tools/include/linux/kernel.h
+++ b/tools/include/linux/kernel.h
@@ -105,4 +105,9 @@ int scnprintf_pad(char * buf, size_t size, const char * fmt, ...);
 #define current_gfp_context(k) 0
 #define synchronize_rcu()
 
+#define KiB(x) ((x) * 1024)
+#define MiB(x) ((x) * 1024 * 1024)
+#define KiB_MASK(x) (KiB(x) - 1)
+#define MiB_MASK(x) (MiB(x) - 1)
+
 #endif
diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index d4c234076541..8f1ff8b99b7a 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -25,9 +25,6 @@
 #include "../../../util/arm-spe.h"
 #include <tools/libc_compat.h> // reallocarray
 
-#define KiB(x) ((x) * 1024)
-#define MiB(x) ((x) * 1024 * 1024)
-
 struct arm_spe_recording {
 	struct auxtrace_record		itr;
 	struct perf_pmu			*arm_spe_pmu;
diff --git a/tools/perf/arch/arm64/util/hisi-ptt.c b/tools/perf/arch/arm64/util/hisi-ptt.c
index ba97c8a562a0..64e9bb06487a 100644
--- a/tools/perf/arch/arm64/util/hisi-ptt.c
+++ b/tools/perf/arch/arm64/util/hisi-ptt.c
@@ -24,9 +24,6 @@
 #include "../../../util/session.h"
 #include "../../../util/tsc.h"
 
-#define KiB(x) ((x) * 1024)
-#define MiB(x) ((x) * 1024 * 1024)
-
 struct hisi_ptt_recording {
 	struct auxtrace_record	itr;
 	struct perf_pmu *hisi_ptt_pmu;
diff --git a/tools/perf/arch/x86/util/intel-bts.c b/tools/perf/arch/x86/util/intel-bts.c
index 439c2956f3e7..1dded1645287 100644
--- a/tools/perf/arch/x86/util/intel-bts.c
+++ b/tools/perf/arch/x86/util/intel-bts.c
@@ -25,11 +25,6 @@
 #include "../../../util/intel-bts.h"
 #include <internal/lib.h> // page_size
 
-#define KiB(x) ((x) * 1024)
-#define MiB(x) ((x) * 1024 * 1024)
-#define KiB_MASK(x) (KiB(x) - 1)
-#define MiB_MASK(x) (MiB(x) - 1)
-
 struct intel_bts_snapshot_ref {
 	void	*ref_buf;
 	size_t	ref_offset;
diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
index af102f471e9f..963d70b3a008 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -33,11 +33,6 @@
 #include <internal/lib.h> // page_size
 #include "../../../util/intel-pt.h"
 
-#define KiB(x) ((x) * 1024)
-#define MiB(x) ((x) * 1024 * 1024)
-#define KiB_MASK(x) (KiB(x) - 1)
-#define MiB_MASK(x) (MiB(x) - 1)
-
 #define INTEL_PT_PSB_PERIOD_NEAR	256
 
 struct intel_pt_snapshot_ref {
diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h
index 90c83f932d9a..d36a0c18ca59 100644
--- a/tools/perf/util/cs-etm.h
+++ b/tools/perf/util/cs-etm.h
@@ -185,9 +185,6 @@ struct cs_etm_packet_queue {
 	struct cs_etm_packet packet_buffer[CS_ETM_PACKET_MAX_BUFFER];
 };
 
-#define KiB(x) ((x) * 1024)
-#define MiB(x) ((x) * 1024 * 1024)
-
 #define CS_ETM_INVAL_ADDR 0xdeadbeefdeadbeefUL
 
 #define BMVAL(val, lsb, msb)	((val & GENMASK(msb, lsb)) >> lsb)
-- 
2.24.0


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

* [PATCH 2/2] perf auxtrace: Make auxtrace->flush_events() optional
  2022-10-25 11:06 [PATCH 0/2] perf/tool: minor cleanups Yicong Yang
  2022-10-25 11:06 ` [PATCH 1/2] perf tool: move unit macros to kernel.h Yicong Yang
@ 2022-10-25 11:06 ` Yicong Yang
  1 sibling, 0 replies; 3+ messages in thread
From: Yicong Yang @ 2022-10-25 11:06 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, John Garry, linux-perf-users
  Cc: Will Deacon, James Clark, Mike Leach, Leo Yan, Mathieu Poirier,
	liuqi6124, jonathan.cameron, alexander.shishkin, prime.zeng,
	linuxarm

From: Yicong Yang <yangyicong@hisilicon.com>

The auxtrace->flush_events() is optional to some PMUs like hisi-ptt and
s390-cpumsf and they only define an empty callback. Make it optional and
handled by auxtrace framework.

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 tools/perf/util/auxtrace.c    | 2 +-
 tools/perf/util/auxtrace.h    | 2 +-
 tools/perf/util/hisi-ptt.c    | 7 -------
 tools/perf/util/s390-cpumsf.c | 7 -------
 4 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index 60d8beb662aa..c90c733e3d56 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -2819,7 +2819,7 @@ void auxtrace__dump_auxtrace_sample(struct perf_session *session,
 
 int auxtrace__flush_events(struct perf_session *session, struct perf_tool *tool)
 {
-	if (!session->auxtrace)
+	if (!session->auxtrace || !session->auxtrace->flush_events)
 		return 0;
 
 	return session->auxtrace->flush_events(session, tool);
diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
index 6a0f9b98f059..1cb409892633 100644
--- a/tools/perf/util/auxtrace.h
+++ b/tools/perf/util/auxtrace.h
@@ -194,7 +194,7 @@ struct auxtrace_index {
  * @queue_data: queue an AUX sample or PERF_RECORD_AUXTRACE event for later
  *              processing
  * @dump_auxtrace_sample: dump AUX area sample data
- * @flush_events: process any remaining data
+ * @flush_events: process any remaining data (optional)
  * @free_events: free resources associated with event processing
  * @free: free resources associated with the session
  */
diff --git a/tools/perf/util/hisi-ptt.c b/tools/perf/util/hisi-ptt.c
index 45b614bb73bf..f69b120a8e52 100644
--- a/tools/perf/util/hisi-ptt.c
+++ b/tools/perf/util/hisi-ptt.c
@@ -124,12 +124,6 @@ static int hisi_ptt_process_auxtrace_event(struct perf_session *session,
 	return 0;
 }
 
-static int hisi_ptt_flush(struct perf_session *session __maybe_unused,
-			  struct perf_tool *tool __maybe_unused)
-{
-	return 0;
-}
-
 static void hisi_ptt_free_events(struct perf_session *session __maybe_unused)
 {
 }
@@ -180,7 +174,6 @@ int hisi_ptt_process_auxtrace_info(union perf_event *event,
 
 	ptt->auxtrace.process_event = hisi_ptt_process_event;
 	ptt->auxtrace.process_auxtrace_event = hisi_ptt_process_auxtrace_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;
diff --git a/tools/perf/util/s390-cpumsf.c b/tools/perf/util/s390-cpumsf.c
index f3fdad28a852..d961033942c6 100644
--- a/tools/perf/util/s390-cpumsf.c
+++ b/tools/perf/util/s390-cpumsf.c
@@ -1001,12 +1001,6 @@ static void s390_cpumsf_free_events(struct perf_session *session __maybe_unused)
 {
 }
 
-static int s390_cpumsf_flush(struct perf_session *session __maybe_unused,
-			     struct perf_tool *tool __maybe_unused)
-{
-	return 0;
-}
-
 static void s390_cpumsf_free_queues(struct perf_session *session)
 {
 	struct s390_cpumsf *sf = container_of(session->auxtrace,
@@ -1148,7 +1142,6 @@ int s390_cpumsf_process_auxtrace_info(union perf_event *event,
 
 	sf->auxtrace.process_event = s390_cpumsf_process_event;
 	sf->auxtrace.process_auxtrace_event = s390_cpumsf_process_auxtrace_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;
-- 
2.24.0


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

end of thread, other threads:[~2022-10-25 11:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-25 11:06 [PATCH 0/2] perf/tool: minor cleanups Yicong Yang
2022-10-25 11:06 ` [PATCH 1/2] perf tool: move unit macros to kernel.h Yicong Yang
2022-10-25 11:06 ` [PATCH 2/2] perf auxtrace: Make auxtrace->flush_events() optional Yicong Yang

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).