linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] perf arm-spe: Correct recording configurations
@ 2021-05-19  4:15 Leo Yan
  2021-05-19  4:15 ` [PATCH v2 1/4] perf arm-spe: Correct sample flags for SPE event Leo Yan
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Leo Yan @ 2021-05-19  4:15 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, John Garry, Will Deacon,
	Mathieu Poirier, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, James Clark,
	Al Grant, linux-arm-kernel, linux-perf-users, linux-kernel
  Cc: Leo Yan

This patch series is to correct Arm SPE recording configurations.

As James Clark found, there have several issues of recording
configurations for Arm SPE.  One main issue is the sample flags setting
for Arm SPE event and dummy event, and there have two minor issues for
enabling timestamp and redundant checking for "full_auxtrace".

This series fixes these issues and has been tested on Arm64 Hisilicon
D06 platform.

Changes from v1:
* Added patch 04/04 to remove redundant checking for "full_auxtrace" 
  (James);
* Added James Clark's Tested-by and Reviewed-by tags for patches 01-03.

Leo Yan (4):
  perf arm-spe: Correct sample flags for SPE event
  perf arm-spe: Correct sample flags for dummy event
  perf arm-spe: Enable timestamp for per-cpu mode
  perf arm-spe: Remove redundant checking for "full_auxtrace"

 tools/perf/arch/arm64/util/arm-spe.c | 45 +++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 7 deletions(-)

-- 
2.25.1


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

* [PATCH v2 1/4] perf arm-spe: Correct sample flags for SPE event
  2021-05-19  4:15 [PATCH v2 0/4] perf arm-spe: Correct recording configurations Leo Yan
@ 2021-05-19  4:15 ` Leo Yan
  2021-05-19  4:15 ` [PATCH v2 2/4] perf arm-spe: Correct sample flags for dummy event Leo Yan
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Leo Yan @ 2021-05-19  4:15 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, John Garry, Will Deacon,
	Mathieu Poirier, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, James Clark,
	Al Grant, linux-arm-kernel, linux-perf-users, linux-kernel
  Cc: Leo Yan

Now it's hard code to set sample flags for CPU, TIME and TID for SPE
event, which is pointless.

The CPU is useful for sampling only for per-mmap case, it is used to
indicate the AUX trace is associated to which CPU.

The TIME sample is not needed for AUX event, since the time for AUX
event is not really used and this time is a different thing from the
timestamp in Arm SPE trace, the timestamp tracing which is controlled
by Arm SPE's config bit.

The TID sample is not useful for AUX event.

This patch corrects the sample flags for SPE event, it only set CPU
sample bit for per-cpu mmap case.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: James Clark <james.clark@arm.com>
Tested-by: James Clark <james.clark@arm.com>
---
 tools/perf/arch/arm64/util/arm-spe.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index 414c8a5584b1..902e73a64184 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -68,6 +68,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
 			container_of(itr, struct arm_spe_recording, itr);
 	struct perf_pmu *arm_spe_pmu = sper->arm_spe_pmu;
 	struct evsel *evsel, *arm_spe_evsel = NULL;
+	struct perf_cpu_map *cpus = evlist->core.cpus;
 	bool privileged = perf_event_paranoid_check(-1);
 	struct evsel *tracking_evsel;
 	int err;
@@ -120,9 +121,9 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
 	 */
 	evlist__to_front(evlist, arm_spe_evsel);
 
-	evsel__set_sample_bit(arm_spe_evsel, CPU);
-	evsel__set_sample_bit(arm_spe_evsel, TIME);
-	evsel__set_sample_bit(arm_spe_evsel, TID);
+	/* In the case of per-cpu mmaps, sample CPU for AUX event. */
+	if (!perf_cpu_map__empty(cpus))
+		evsel__set_sample_bit(arm_spe_evsel, CPU);
 
 	/* Add dummy event to keep tracking */
 	err = parse_events(evlist, "dummy:u", NULL);
-- 
2.25.1


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

* [PATCH v2 2/4] perf arm-spe: Correct sample flags for dummy event
  2021-05-19  4:15 [PATCH v2 0/4] perf arm-spe: Correct recording configurations Leo Yan
  2021-05-19  4:15 ` [PATCH v2 1/4] perf arm-spe: Correct sample flags for SPE event Leo Yan
@ 2021-05-19  4:15 ` Leo Yan
  2021-05-19  4:15 ` [PATCH v2 3/4] perf arm-spe: Enable timestamp for per-cpu mode Leo Yan
  2021-05-19  4:15 ` [PATCH v2 4/4] perf arm-spe: Remove redundant checking for "full_auxtrace" Leo Yan
  3 siblings, 0 replies; 7+ messages in thread
From: Leo Yan @ 2021-05-19  4:15 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, John Garry, Will Deacon,
	Mathieu Poirier, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, James Clark,
	Al Grant, linux-arm-kernel, linux-perf-users, linux-kernel
  Cc: Leo Yan

The dummy event is mainly used for mmap, the TIME sample is only needed
for per-cpu case so that the perf tool can rely on the correct timing
for parsing symbols.  And the CPU sample is useless for mmap.

The BRANCH_STACK sample bit will be always reset for the dummy event in
the function evsel__config(), so don't need to repeatedly reset it for
Arm SPE specific.

So this patch only enables TIME sample for per-cpu mmap.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: James Clark <james.clark@arm.com>
Tested-by: James Clark <james.clark@arm.com>
---
 tools/perf/arch/arm64/util/arm-spe.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index 902e73a64184..4c916626c203 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -135,9 +135,10 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
 
 	tracking_evsel->core.attr.freq = 0;
 	tracking_evsel->core.attr.sample_period = 1;
-	evsel__set_sample_bit(tracking_evsel, TIME);
-	evsel__set_sample_bit(tracking_evsel, CPU);
-	evsel__reset_sample_bit(tracking_evsel, BRANCH_STACK);
+
+	/* In per-cpu case, always need the time of mmap events etc */
+	if (!perf_cpu_map__empty(cpus))
+		evsel__set_sample_bit(tracking_evsel, TIME);
 
 	return 0;
 }
-- 
2.25.1


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

* [PATCH v2 3/4] perf arm-spe: Enable timestamp for per-cpu mode
  2021-05-19  4:15 [PATCH v2 0/4] perf arm-spe: Correct recording configurations Leo Yan
  2021-05-19  4:15 ` [PATCH v2 1/4] perf arm-spe: Correct sample flags for SPE event Leo Yan
  2021-05-19  4:15 ` [PATCH v2 2/4] perf arm-spe: Correct sample flags for dummy event Leo Yan
@ 2021-05-19  4:15 ` Leo Yan
  2021-05-19  4:15 ` [PATCH v2 4/4] perf arm-spe: Remove redundant checking for "full_auxtrace" Leo Yan
  3 siblings, 0 replies; 7+ messages in thread
From: Leo Yan @ 2021-05-19  4:15 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, John Garry, Will Deacon,
	Mathieu Poirier, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, James Clark,
	Al Grant, linux-arm-kernel, linux-perf-users, linux-kernel
  Cc: Leo Yan

For per-cpu mmap, it should enable timestamp tracing for Arm SPE; this
is helpful for samples correlation.

To automatically enable the timestamp, a helper arm_spe_set_timestamp()
is introduced for setting "ts_enable" format bit.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: James Clark <james.clark@arm.com>
Tested-by: James Clark <james.clark@arm.com>
---
 tools/perf/arch/arm64/util/arm-spe.c | 33 ++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index 4c916626c203..bacdf366040d 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -14,6 +14,7 @@
 #include "../../../util/cpumap.h"
 #include "../../../util/event.h"
 #include "../../../util/evsel.h"
+#include "../../../util/evsel_config.h"
 #include "../../../util/evlist.h"
 #include "../../../util/session.h"
 #include <internal/lib.h> // page_size
@@ -32,6 +33,29 @@ struct arm_spe_recording {
 	struct evlist		*evlist;
 };
 
+static void arm_spe_set_timestamp(struct auxtrace_record *itr,
+				  struct evsel *evsel)
+{
+	struct arm_spe_recording *ptr;
+	struct perf_pmu *arm_spe_pmu;
+	struct evsel_config_term *term = evsel__get_config_term(evsel, CFG_CHG);
+	u64 user_bits = 0, bit;
+
+	ptr = container_of(itr, struct arm_spe_recording, itr);
+	arm_spe_pmu = ptr->arm_spe_pmu;
+
+	if (term)
+		user_bits = term->val.cfg_chg;
+
+	bit = perf_pmu__format_bits(&arm_spe_pmu->format, "ts_enable");
+
+	/* Skip if user has set it */
+	if (bit & user_bits)
+		return;
+
+	evsel->core.attr.config |= bit;
+}
+
 static size_t
 arm_spe_info_priv_size(struct auxtrace_record *itr __maybe_unused,
 		       struct evlist *evlist __maybe_unused)
@@ -121,9 +145,14 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
 	 */
 	evlist__to_front(evlist, arm_spe_evsel);
 
-	/* In the case of per-cpu mmaps, sample CPU for AUX event. */
-	if (!perf_cpu_map__empty(cpus))
+	/*
+	 * In the case of per-cpu mmaps, sample CPU for AUX event;
+	 * also enable the timestamp tracing for samples correlation.
+	 */
+	if (!perf_cpu_map__empty(cpus)) {
 		evsel__set_sample_bit(arm_spe_evsel, CPU);
+		arm_spe_set_timestamp(itr, arm_spe_evsel);
+	}
 
 	/* Add dummy event to keep tracking */
 	err = parse_events(evlist, "dummy:u", NULL);
-- 
2.25.1


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

* [PATCH v2 4/4] perf arm-spe: Remove redundant checking for "full_auxtrace"
  2021-05-19  4:15 [PATCH v2 0/4] perf arm-spe: Correct recording configurations Leo Yan
                   ` (2 preceding siblings ...)
  2021-05-19  4:15 ` [PATCH v2 3/4] perf arm-spe: Enable timestamp for per-cpu mode Leo Yan
@ 2021-05-19  4:15 ` Leo Yan
  2021-05-25 12:03   ` James Clark
  3 siblings, 1 reply; 7+ messages in thread
From: Leo Yan @ 2021-05-19  4:15 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, John Garry, Will Deacon,
	Mathieu Poirier, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, James Clark,
	Al Grant, linux-arm-kernel, linux-perf-users, linux-kernel
  Cc: Leo Yan

The option "opts->full_auxtrace" is checked at the earlier place, if it
is false the function will directly bail out.  So remove the redundant
checking for "opts->full_auxtrace".

Suggested-by: James Clark <james.clark@arm.com>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/arch/arm64/util/arm-spe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index bacdf366040d..a4420d4df503 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -116,7 +116,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
 		return 0;
 
 	/* We are in full trace mode but '-m,xyz' wasn't specified */
-	if (opts->full_auxtrace && !opts->auxtrace_mmap_pages) {
+	if (!opts->auxtrace_mmap_pages) {
 		if (privileged) {
 			opts->auxtrace_mmap_pages = MiB(4) / page_size;
 		} else {
-- 
2.25.1


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

* Re: [PATCH v2 4/4] perf arm-spe: Remove redundant checking for "full_auxtrace"
  2021-05-19  4:15 ` [PATCH v2 4/4] perf arm-spe: Remove redundant checking for "full_auxtrace" Leo Yan
@ 2021-05-25 12:03   ` James Clark
  2021-05-25 12:50     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 7+ messages in thread
From: James Clark @ 2021-05-25 12:03 UTC (permalink / raw)
  To: Leo Yan, Arnaldo Carvalho de Melo, John Garry, Will Deacon,
	Mathieu Poirier, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, Al Grant,
	linux-arm-kernel, linux-perf-users, linux-kernel



On 19/05/2021 07:15, Leo Yan wrote:
> The option "opts->full_auxtrace" is checked at the earlier place, if it
> is false the function will directly bail out.  So remove the redundant
> checking for "opts->full_auxtrace".
> 
> Suggested-by: James Clark <james.clark@arm.com>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  tools/perf/arch/arm64/util/arm-spe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
> index bacdf366040d..a4420d4df503 100644
> --- a/tools/perf/arch/arm64/util/arm-spe.c
> +++ b/tools/perf/arch/arm64/util/arm-spe.c
> @@ -116,7 +116,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
>  		return 0;
>  
>  	/* We are in full trace mode but '-m,xyz' wasn't specified */
> -	if (opts->full_auxtrace && !opts->auxtrace_mmap_pages) {
> +	if (!opts->auxtrace_mmap_pages) {
>  		if (privileged) {
>  			opts->auxtrace_mmap_pages = MiB(4) / page_size;
>  		} else {
> 

Reviewed-by: James Clark <james.clark@arm.com>

Thanks
James

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

* Re: [PATCH v2 4/4] perf arm-spe: Remove redundant checking for "full_auxtrace"
  2021-05-25 12:03   ` James Clark
@ 2021-05-25 12:50     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-05-25 12:50 UTC (permalink / raw)
  To: James Clark
  Cc: Leo Yan, John Garry, Will Deacon, Mathieu Poirier,
	Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, Al Grant, linux-arm-kernel,
	linux-perf-users, linux-kernel

Em Tue, May 25, 2021 at 03:03:53PM +0300, James Clark escreveu:
> 
> 
> On 19/05/2021 07:15, Leo Yan wrote:
> > The option "opts->full_auxtrace" is checked at the earlier place, if it
> > is false the function will directly bail out.  So remove the redundant
> > checking for "opts->full_auxtrace".
> > 
> > Suggested-by: James Clark <james.clark@arm.com>
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >  tools/perf/arch/arm64/util/arm-spe.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
> > index bacdf366040d..a4420d4df503 100644
> > --- a/tools/perf/arch/arm64/util/arm-spe.c
> > +++ b/tools/perf/arch/arm64/util/arm-spe.c
> > @@ -116,7 +116,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
> >  		return 0;
> >  
> >  	/* We are in full trace mode but '-m,xyz' wasn't specified */
> > -	if (opts->full_auxtrace && !opts->auxtrace_mmap_pages) {
> > +	if (!opts->auxtrace_mmap_pages) {
> >  		if (privileged) {
> >  			opts->auxtrace_mmap_pages = MiB(4) / page_size;
> >  		} else {
> > 
> 
> Reviewed-by: James Clark <james.clark@arm.com>

Thanks, applied the series,

- Arnaldo


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

end of thread, other threads:[~2021-05-25 12:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-19  4:15 [PATCH v2 0/4] perf arm-spe: Correct recording configurations Leo Yan
2021-05-19  4:15 ` [PATCH v2 1/4] perf arm-spe: Correct sample flags for SPE event Leo Yan
2021-05-19  4:15 ` [PATCH v2 2/4] perf arm-spe: Correct sample flags for dummy event Leo Yan
2021-05-19  4:15 ` [PATCH v2 3/4] perf arm-spe: Enable timestamp for per-cpu mode Leo Yan
2021-05-19  4:15 ` [PATCH v2 4/4] perf arm-spe: Remove redundant checking for "full_auxtrace" Leo Yan
2021-05-25 12:03   ` James Clark
2021-05-25 12:50     ` Arnaldo Carvalho de Melo

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