[PATCH 0/4] perf: Allow suppressing useless AUX records

[PATCH 0/4] perf: Allow suppressing useless AUX records

[Alexander Shishkin]

Hi guys,

It's been brought to my attention many times that the AUX records are
not always useful. One instance is the AUX overwrite mode, where we
generate one such record every time the target task schedules out.

This patchset adds a bit to the attribute structure that would enable
suppressing the AUX records when the only the only set flag is OVERWRITE.
In this case, at least for the PT and BTS data we just ignore these
records. But at the same time, AUX overwrite events are likely to be
kept running over longer periods of time, making the buildup of AUX
records an unnecessary annoyance.

Alexander Shishkin (4):
  perf: Allow suppressing AUX records
  tools, perf_event.h: Synchronize
  perf tools: Add 'suppress_aux' attribute bit definition and fallback
  perf intel-pt, intel-bts: Suppress useless AUX records by default

 include/uapi/linux/perf_event.h       |  3 ++-
 kernel/events/core.c                  |  5 +++++
 kernel/events/ring_buffer.c           | 12 ++++++++++--
 tools/include/uapi/linux/perf_event.h |  3 ++-
 tools/perf/arch/x86/util/auxtrace.c   |  2 ++
 tools/perf/util/evsel.c               |  9 +++++++++
 6 files changed, 30 insertions(+), 4 deletions(-)

-- 
2.14.2

[PATCH 1/4] perf: Allow suppressing AUX records

[Alexander Shishkin]

It has been pointed out to me many times that it is useful to be able
to switch off AUX records to save the bandwidth for records that actually
matter, for example, in AUX overwrite mode.

The usefulness of PERF_RECORD_AUX is in some of its flags, like the
TRUNCATED flag that tells the decoder where exactly gaps in the trace are.
The OVERWRITE flag, on the other hand will be set on every single record
in overwrite mode. However, a PERF_RECORD_AUX[flags=OVERWRITE] is
generated on every target task's sched_out, which over time adds up to
a lot of useless information.

This patch adds an attribute bit that enables suppressing such records.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Markus Metzger <markus.t.metzger@intel.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
---
 include/uapi/linux/perf_event.h |  3 ++-
 kernel/events/core.c            |  5 +++++
 kernel/events/ring_buffer.c     | 12 ++++++++++--
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 362493a2f950..fa3821d9dc52 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -370,7 +370,8 @@ struct perf_event_attr {
 				context_switch :  1, /* context switch data */
 				write_backward :  1, /* Write ring buffer from end to beginning */
 				namespaces     :  1, /* include namespaces data */
-				__reserved_1   : 35;
+				suppress_aux   :  1, /* don't generate PERF_RECORD_AUX */
+				__reserved_1   : 34;
 
 	union {
 		__u32		wakeup_events;	  /* wakeup every n events */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 81dd57b9e5e3..483122c73936 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10014,6 +10014,11 @@ SYSCALL_DEFINE5(perf_event_open,
 		goto err_context;
 	}
 
+	if (attr.suppress_aux && !pmu->setup_aux) {
+		err = -EINVAL;
+		goto err_context;
+	}
+
 	/*
 	 * Look up the group leader (we will attach this event to it):
 	 */
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index f684d8e5fa2b..d3f147e99165 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -425,6 +425,12 @@ static bool __always_inline rb_need_aux_wakeup(struct ring_buffer *rb)
 	return false;
 }
 
+/*
+ * These flags won't generate a PERF_RECORD_AUX on their own if
+ * attr::suppress_aux is set.
+ */
+#define SUPPRESSABLE_FLAGS	PERF_AUX_FLAG_OVERWRITE
+
 /*
  * Commit the data written by hardware into the ring buffer by adjusting
  * aux_head and posting a PERF_RECORD_AUX into the perf buffer. It is the
@@ -459,8 +465,10 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
 		 * Only send RECORD_AUX if we have something useful to communicate
 		 */
 
-		perf_event_aux_event(handle->event, aux_head, size,
-		                     handle->aux_flags);
+		if (handle->event->attr.suppress_aux &&
+		    handle->aux_flags & ~SUPPRESSABLE_FLAGS)
+			perf_event_aux_event(handle->event, aux_head, size,
+					     handle->aux_flags);
 	}
 
 	rb->user_page->aux_head = rb->aux_head;
-- 
2.14.2

[PATCH 2/4] tools, perf_event.h: Synchronize

[Alexander Shishkin]

I've just added attr::suppress_aux bit to the kernel header, adding
it to the tools' header too.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 tools/include/uapi/linux/perf_event.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index 362493a2f950..fa3821d9dc52 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -370,7 +370,8 @@ struct perf_event_attr {
 				context_switch :  1, /* context switch data */
 				write_backward :  1, /* Write ring buffer from end to beginning */
 				namespaces     :  1, /* include namespaces data */
-				__reserved_1   : 35;
+				suppress_aux   :  1, /* don't generate PERF_RECORD_AUX */
+				__reserved_1   : 34;
 
 	union {
 		__u32		wakeup_events;	  /* wakeup every n events */
-- 
2.14.2

[PATCH 3/4] perf tools: Add 'suppress_aux' attribute bit definition and fallback

[Alexander Shishkin]

This adds support for suppress_aux, the switch that enables suppressing
not-so-useful PERF_RECORD_AUX records. Also handle kernels where it's
not supported.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 tools/perf/util/evsel.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index f894893c203d..794e56b42c20 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -50,6 +50,7 @@ static struct {
 	bool lbr_flags;
 	bool write_backward;
 	bool group_read;
+	bool suppress_aux;
 } perf_missing_features;
 
 static clockid_t clockid;
@@ -1570,6 +1571,7 @@ int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
 	PRINT_ATTRf(use_clockid, p_unsigned);
 	PRINT_ATTRf(context_switch, p_unsigned);
 	PRINT_ATTRf(write_backward, p_unsigned);
+	PRINT_ATTRf(suppress_aux, p_unsigned);
 
 	PRINT_ATTRn("{ wakeup_events, wakeup_watermark }", wakeup_events, p_unsigned);
 	PRINT_ATTRf(bp_type, p_unsigned);
@@ -1686,6 +1688,8 @@ int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
 				     PERF_SAMPLE_BRANCH_NO_CYCLES);
 	if (perf_missing_features.group_read && evsel->attr.inherit)
 		evsel->attr.read_format &= ~(PERF_FORMAT_GROUP|PERF_FORMAT_ID);
+	if (perf_missing_features.suppress_aux && evsel->attr.suppress_aux)
+		evsel->attr.suppress_aux = 0;
 retry_sample_id:
 	if (perf_missing_features.sample_id_all)
 		evsel->attr.sample_id_all = 0;
@@ -1847,6 +1851,11 @@ int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
 		perf_missing_features.group_read = true;
 		pr_debug2("switching off group read\n");
 		goto fallback_missing_features;
+	} else if (!perf_missing_features.suppress_aux &&
+		   evsel->attr.suppress_aux) {
+		perf_missing_features.suppress_aux = true;
+		pr_debug2("switching off suppress_aux\n");
+		goto fallback_missing_features;
 	}
 out_close:
 	do {
-- 
2.14.2

[PATCH 4/4] perf intel-pt, intel-bts: Suppress useless AUX records by default

[Alexander Shishkin]

This makes use of the shiny new attr::suppress_aux that suppresses the
AUX records that don't carry any 'interesting' information for the
decoders, that is PERF_RECORD_AUX[flag==OVERWRITE], which just stack up
in the DATA buffer for no good reason.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 tools/perf/arch/x86/util/auxtrace.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/arch/x86/util/auxtrace.c b/tools/perf/arch/x86/util/auxtrace.c
index 6aa3f2a38321..5700e6099608 100644
--- a/tools/perf/arch/x86/util/auxtrace.c
+++ b/tools/perf/arch/x86/util/auxtrace.c
@@ -45,6 +45,8 @@ struct auxtrace_record *auxtrace_record__init_intel(struct perf_evlist *evlist,
 			if (intel_bts_pmu &&
 			    evsel->attr.type == intel_bts_pmu->type)
 				found_bts = true;
+			if (found_pt || found_bts)
+				evsel->attr.suppress_aux = 1;
 		}
 	}
 
-- 
2.14.2

Re: [PATCH 1/4] perf: Allow suppressing AUX records

[Adrian Hunter]

On 09/11/17 19:05, Alexander Shishkin wrote:
> diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
> index f684d8e5fa2b..d3f147e99165 100644
> --- a/kernel/events/ring_buffer.c
> +++ b/kernel/events/ring_buffer.c
> @@ -425,6 +425,12 @@ static bool __always_inline rb_need_aux_wakeup(struct ring_buffer *rb)
>  	return false;
>  }
>  
> +/*
> + * These flags won't generate a PERF_RECORD_AUX on their own if
> + * attr::suppress_aux is set.
> + */
> +#define SUPPRESSABLE_FLAGS	PERF_AUX_FLAG_OVERWRITE
> +
>  /*
>   * Commit the data written by hardware into the ring buffer by adjusting
>   * aux_head and posting a PERF_RECORD_AUX into the perf buffer. It is the
> @@ -459,8 +465,10 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
>  		 * Only send RECORD_AUX if we have something useful to communicate
>  		 */
>  
> -		perf_event_aux_event(handle->event, aux_head, size,
> -		                     handle->aux_flags);
> +		if (handle->event->attr.suppress_aux &&
> +		    handle->aux_flags & ~SUPPRESSABLE_FLAGS)

Is that right?  I would have expected:

		if (!handle->event->attr.suppress_aux ||
		    handle->aux_flags & ~SUPPRESSABLE_FLAGS)

Also aux_flags is u64 but ~SUPPRESSABLE_FLAGS is int by default I think.

> +			perf_event_aux_event(handle->event, aux_head, size,
> +					     handle->aux_flags);
>  	}
>  
>  	rb->user_page->aux_head = rb->aux_head;
>