Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1584619 > unrolled thread

[PATCH 0/4] perf, pt, coresight: AUX flags and VMX update

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4] perf, pt, coresight: AUX flags and VMX update Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2017-02-20 14:40 +0100
    [PATCH 2/4] perf: Keep AUX flags in the output handle Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2017-02-20 14:50 +0100
      Re: [PATCH 2/4] perf: Keep AUX flags in the output handle Mathieu Poirier <mathieu.poirier@linaro.org> - 2017-02-20 21:50 +0100
      Re: [PATCH 2/4] perf: Keep AUX flags in the output handle Will Deacon <will.deacon@arm.com> - 2017-02-21 11:50 +0100
        Re: [PATCH 2/4] perf: Keep AUX flags in the output handle Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2017-02-21 12:00 +0100
    [PATCH 1/4] perf: Export AUX buffer helpers to modules Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2017-02-20 14:50 +0100
    [PATCH 3/4] perf: Add a flag for partial AUX records Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2017-02-20 14:50 +0100
    [PATCH 4/4] perf/x86/intel/pt: Handle VMX better Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2017-02-20 15:00 +0100
    Re: [PATCH 0/4] perf, pt, coresight: AUX flags and VMX update Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2017-02-20 16:30 +0100
      Re: [PATCH 0/4] perf, pt, coresight: AUX flags and VMX update Adrian Hunter <adrian.hunter@intel.com> - 2017-02-20 16:50 +0100
        Re: [PATCH 0/4] perf, pt, coresight: AUX flags and VMX update Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-02-20 17:20 +0100
          Re: [PATCH 0/4] perf, pt, coresight: AUX flags and VMX update Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2017-02-20 17:40 +0100

#1584619 — [PATCH 0/4] perf, pt, coresight: AUX flags and VMX update

Hi Peter,

With the vmm_exclusive=0, PT seems to be much more usable on BDW now. This
patchset does three things:
 * adds a flag to PERF_RECORD_AUX, signalling that a transaction has gaps
   in it (due to VMX root mode kicking in),
 * changes the AUX API slightly to allow for flags to be set at arbitrary
   points between perf_aux_output_begin() and perf_aux_output_end(),
 * restarts PT after VMXOFF.

I also stole Will's patch from another patchset that adds EXPORT_SYMBOL_GPL
to the AUX calls, which is not strictly relevant, but happens to touch the
same area and is long overdue. The AUX flags patch is also based on Will's
patch from that same context.

Alexander Shishkin (2):
  perf: Add a flag for partial AUX records
  perf/x86/intel/pt: Handle VMX better

Will Deacon (2):
  perf: Export AUX buffer helpers to modules
  perf: Keep AUX flags in the output handle

 arch/x86/events/intel/bts.c                      | 16 +++----
 arch/x86/events/intel/pt.c                       | 55 +++++++++++++-----------
 arch/x86/events/intel/pt.h                       |  1 -
 drivers/hwtracing/coresight/coresight-etb10.c    |  7 ++-
 drivers/hwtracing/coresight/coresight-etm-perf.c |  9 ++--
 drivers/hwtracing/coresight/coresight-priv.h     |  2 -
 drivers/hwtracing/coresight/coresight-tmc-etf.c  |  7 ++-
 include/linux/coresight.h                        |  2 +-
 include/linux/perf_event.h                       |  8 ++--
 include/uapi/linux/perf_event.h                  |  1 +
 kernel/events/ring_buffer.c                      | 38 +++++++++++-----
 11 files changed, 81 insertions(+), 65 deletions(-)

-- 
2.11.0

[toc] | [next] | [standalone]

#1584623 — [PATCH 2/4] perf: Keep AUX flags in the output handle

From: Will Deacon <will.deacon@arm.com>

In preparation for adding more flags to perf AUX records, introduce a
separate API for setting the flags for a session, rather than appending
more bool arguments to perf_aux_output_end. This allows to set each
flag at the time a corresponding condition is detected, instead of
tracking it in each driver's private state.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 arch/x86/events/intel/bts.c                      | 16 +++++------
 arch/x86/events/intel/pt.c                       | 17 ++++++------
 arch/x86/events/intel/pt.h                       |  1 -
 drivers/hwtracing/coresight/coresight-etb10.c    |  7 +++--
 drivers/hwtracing/coresight/coresight-etm-perf.c |  9 +++----
 drivers/hwtracing/coresight/coresight-priv.h     |  2 --
 drivers/hwtracing/coresight/coresight-tmc-etf.c  |  7 +++--
 include/linux/coresight.h                        |  2 +-
 include/linux/perf_event.h                       |  8 +++---
 kernel/events/ring_buffer.c                      | 34 ++++++++++++++++--------
 10 files changed, 55 insertions(+), 48 deletions(-)

diff --git a/arch/x86/events/intel/bts.c b/arch/x86/events/intel/bts.c
index 982c9e31da..8ae8c5ce3a 100644
--- a/arch/x86/events/intel/bts.c
+++ b/arch/x86/events/intel/bts.c
@@ -63,7 +63,6 @@ struct bts_buffer {
 	unsigned int	cur_buf;
 	bool		snapshot;
 	local_t		data_size;
-	local_t		lost;
 	local_t		head;
 	unsigned long	end;
 	void		**data_pages;
@@ -199,7 +198,8 @@ static void bts_update(struct bts_ctx *bts)
 			return;
 
 		if (ds->bts_index >= ds->bts_absolute_maximum)
-			local_inc(&buf->lost);
+			perf_aux_output_flag(&bts->handle,
+			                     PERF_AUX_FLAG_TRUNCATED);
 
 		/*
 		 * old and head are always in the same physical buffer, so we
@@ -276,7 +276,7 @@ static void bts_event_start(struct perf_event *event, int flags)
 	return;
 
 fail_end_stop:
-	perf_aux_output_end(&bts->handle, 0, false);
+	perf_aux_output_end(&bts->handle, 0);
 
 fail_stop:
 	event->hw.state = PERF_HES_STOPPED;
@@ -319,9 +319,8 @@ static void bts_event_stop(struct perf_event *event, int flags)
 				bts->handle.head =
 					local_xchg(&buf->data_size,
 						   buf->nr_pages << PAGE_SHIFT);
-
-			perf_aux_output_end(&bts->handle, local_xchg(&buf->data_size, 0),
-					    !!local_xchg(&buf->lost, 0));
+			perf_aux_output_end(&bts->handle,
+			                    local_xchg(&buf->data_size, 0));
 		}
 
 		cpuc->ds->bts_index = bts->ds_back.bts_buffer_base;
@@ -484,8 +483,7 @@ int intel_bts_interrupt(void)
 	if (old_head == local_read(&buf->head))
 		return handled;
 
-	perf_aux_output_end(&bts->handle, local_xchg(&buf->data_size, 0),
-			    !!local_xchg(&buf->lost, 0));
+	perf_aux_output_end(&bts->handle, local_xchg(&buf->data_size, 0));
 
 	buf = perf_aux_output_begin(&bts->handle, event);
 	if (buf)
@@ -500,7 +498,7 @@ int intel_bts_interrupt(void)
 			 * cleared handle::event
 			 */
 			barrier();
-			perf_aux_output_end(&bts->handle, 0, false);
+			perf_aux_output_end(&bts->handle, 0);
 		}
 	}
 
diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index d92a60ef08..a2d0050fde 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -823,7 +823,8 @@ static void pt_handle_status(struct pt *pt)
 		 */
 		if (!pt_cap_get(PT_CAP_topa_multiple_entries) ||
 		    buf->output_off == sizes(TOPA_ENTRY(buf->cur, buf->cur_idx)->size)) {
-			local_inc(&buf->lost);
+			perf_aux_output_flag(&pt->handle,
+			                     PERF_AUX_FLAG_TRUNCATED);
 			advance++;
 		}
 	}
@@ -916,8 +917,10 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
 
 	/* can't stop in the middle of an output region */
 	if (buf->output_off + handle->size + 1 <
-	    sizes(TOPA_ENTRY(buf->cur, buf->cur_idx)->size))
+	    sizes(TOPA_ENTRY(buf->cur, buf->cur_idx)->size)) {
+		perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
 		return -EINVAL;
+	}
 
 
 	/* single entry ToPA is handled by marking all regions STOP=1 INT=1 */
@@ -1269,8 +1272,7 @@ void intel_pt_interrupt(void)
 
 	pt_update_head(pt);
 
-	perf_aux_output_end(&pt->handle, local_xchg(&buf->data_size, 0),
-			    local_xchg(&buf->lost, 0));
+	perf_aux_output_end(&pt->handle, local_xchg(&buf->data_size, 0));
 
 	if (!event->hw.state) {
 		int ret;
@@ -1285,7 +1287,7 @@ void intel_pt_interrupt(void)
 		/* snapshot counters don't use PMI, so it's safe */
 		ret = pt_buffer_reset_markers(buf, &pt->handle);
 		if (ret) {
-			perf_aux_output_end(&pt->handle, 0, true);
+			perf_aux_output_end(&pt->handle, 0);
 			return;
 		}
 
@@ -1357,7 +1359,7 @@ static void pt_event_start(struct perf_event *event, int mode)
 	return;
 
 fail_end_stop:
-	perf_aux_output_end(&pt->handle, 0, true);
+	perf_aux_output_end(&pt->handle, 0);
 fail_stop:
 	hwc->state = PERF_HES_STOPPED;
 }
@@ -1398,8 +1400,7 @@ static void pt_event_stop(struct perf_event *event, int mode)
 			pt->handle.head =
 				local_xchg(&buf->data_size,
 					   buf->nr_pages << PAGE_SHIFT);
-		perf_aux_output_end(&pt->handle, local_xchg(&buf->data_size, 0),
-				    local_xchg(&buf->lost, 0));
+		perf_aux_output_end(&pt->handle, local_xchg(&buf->data_size, 0));
 	}
 }
 
diff --git a/arch/x86/events/intel/pt.h b/arch/x86/events/intel/pt.h
index fa81f8d8ed..25fa9710f4 100644
--- a/arch/x86/events/intel/pt.h
+++ b/arch/x86/events/intel/pt.h
@@ -144,7 +144,6 @@ struct pt_buffer {
 	size_t			output_off;
 	unsigned long		nr_pages;
 	local_t			data_size;
-	local_t			lost;
 	local64_t		head;
 	bool			snapshot;
 	unsigned long		stop_pos, intr_pos;
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
index d7325c6534..82c8ddcf09 100644
--- a/drivers/hwtracing/coresight/coresight-etb10.c
+++ b/drivers/hwtracing/coresight/coresight-etb10.c
@@ -321,7 +321,7 @@ static int etb_set_buffer(struct coresight_device *csdev,
 
 static unsigned long etb_reset_buffer(struct coresight_device *csdev,
 				      struct perf_output_handle *handle,
-				      void *sink_config, bool *lost)
+				      void *sink_config)
 {
 	unsigned long size = 0;
 	struct cs_buffers *buf = sink_config;
@@ -343,7 +343,6 @@ static unsigned long etb_reset_buffer(struct coresight_device *csdev,
 		 * resetting parameters here and squaring off with the ring
 		 * buffer API in the tracer PMU is fine.
 		 */
-		*lost = !!local_xchg(&buf->lost, 0);
 		size = local_xchg(&buf->data_size, 0);
 	}
 
@@ -385,7 +384,7 @@ static void etb_update_buffer(struct coresight_device *csdev,
 			(unsigned long)write_ptr);
 
 		write_ptr &= ~(ETB_FRAME_SIZE_WORDS - 1);
-		local_inc(&buf->lost);
+		perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
 	}
 
 	/*
@@ -396,7 +395,7 @@ static void etb_update_buffer(struct coresight_device *csdev,
 	 */
 	status = readl_relaxed(drvdata->base + ETB_STATUS_REG);
 	if (status & ETB_STATUS_RAM_FULL) {
-		local_inc(&buf->lost);
+		perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
 		to_read = capacity;
 		read_ptr = write_ptr;
 	} else {
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 70eaa74dc2..47ea0eee67 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -301,7 +301,8 @@ static void etm_event_start(struct perf_event *event, int flags)
 	return;
 
 fail_end_stop:
-	perf_aux_output_end(handle, 0, true);
+	perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
+	perf_aux_output_end(handle, 0);
 fail:
 	event->hw.state = PERF_HES_STOPPED;
 	goto out;
@@ -309,7 +310,6 @@ static void etm_event_start(struct perf_event *event, int flags)
 
 static void etm_event_stop(struct perf_event *event, int mode)
 {
-	bool lost;
 	int cpu = smp_processor_id();
 	unsigned long size;
 	struct coresight_device *sink, *csdev = per_cpu(csdev_src, cpu);
@@ -347,10 +347,9 @@ static void etm_event_stop(struct perf_event *event, int mode)
 			return;
 
 		size = sink_ops(sink)->reset_buffer(sink, handle,
-						    event_data->snk_config,
-						    &lost);
+						    event_data->snk_config);
 
-		perf_aux_output_end(handle, size, lost);
+		perf_aux_output_end(handle, size);
 	}
 
 	/* Disabling the path make its elements available to other sessions */
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index ef9d8e93e3..5f662d8205 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -76,7 +76,6 @@ enum cs_mode {
  * @nr_pages:	max number of pages granted to us
  * @offset:	offset within the current buffer
  * @data_size:	how much we collected in this run
- * @lost:	other than zero if we had a HW buffer wrap around
  * @snapshot:	is this run in snapshot mode
  * @data_pages:	a handle the ring buffer
  */
@@ -85,7 +84,6 @@ struct cs_buffers {
 	unsigned int		nr_pages;
 	unsigned long		offset;
 	local_t			data_size;
-	local_t			lost;
 	bool			snapshot;
 	void			**data_pages;
 };
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 1549436e24..aec61a6d5c 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -329,7 +329,7 @@ static int tmc_set_etf_buffer(struct coresight_device *csdev,
 
 static unsigned long tmc_reset_etf_buffer(struct coresight_device *csdev,
 					  struct perf_output_handle *handle,
-					  void *sink_config, bool *lost)
+					  void *sink_config)
 {
 	long size = 0;
 	struct cs_buffers *buf = sink_config;
@@ -350,7 +350,6 @@ static unsigned long tmc_reset_etf_buffer(struct coresight_device *csdev,
 		 * resetting parameters here and squaring off with the ring
 		 * buffer API in the tracer PMU is fine.
 		 */
-		*lost = !!local_xchg(&buf->lost, 0);
 		size = local_xchg(&buf->data_size, 0);
 	}
 
@@ -389,7 +388,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
 	 */
 	status = readl_relaxed(drvdata->base + TMC_STS);
 	if (status & TMC_STS_FULL) {
-		local_inc(&buf->lost);
+		perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
 		to_read = drvdata->size;
 	} else {
 		to_read = CIRC_CNT(write_ptr, read_ptr, drvdata->size);
@@ -434,7 +433,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
 			read_ptr -= drvdata->size;
 		/* Tell the HW */
 		writel_relaxed(read_ptr, drvdata->base + TMC_RRP);
-		local_inc(&buf->lost);
+		perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
 	}
 
 	cur = buf->cur;
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 2a5982c37d..035c16c9a5 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -201,7 +201,7 @@ struct coresight_ops_sink {
 			  void *sink_config);
 	unsigned long (*reset_buffer)(struct coresight_device *csdev,
 				      struct perf_output_handle *handle,
-				      void *sink_config, bool *lost);
+				      void *sink_config);
 	void (*update_buffer)(struct coresight_device *csdev,
 			      struct perf_output_handle *handle,
 			      void *sink_config);
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index a39564314e..cdbaa88dc8 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -812,6 +812,7 @@ struct perf_output_handle {
 	struct ring_buffer		*rb;
 	unsigned long			wakeup;
 	unsigned long			size;
+	u64				aux_flags;
 	union {
 		void			*addr;
 		unsigned long		head;
@@ -860,10 +861,11 @@ perf_cgroup_from_task(struct task_struct *task, struct perf_event_context *ctx)
 extern void *perf_aux_output_begin(struct perf_output_handle *handle,
 				   struct perf_event *event);
 extern void perf_aux_output_end(struct perf_output_handle *handle,
-				unsigned long size, bool truncated);
+				unsigned long size);
 extern int perf_aux_output_skip(struct perf_output_handle *handle,
 				unsigned long size);
 extern void *perf_get_aux(struct perf_output_handle *handle);
+extern void perf_aux_output_flag(struct perf_output_handle *handle, u64 flags);
 
 extern int perf_pmu_register(struct pmu *pmu, const char *name, int type);
 extern void perf_pmu_unregister(struct pmu *pmu);
@@ -1278,8 +1280,8 @@ static inline void *
 perf_aux_output_begin(struct perf_output_handle *handle,
 		      struct perf_event *event)				{ return NULL; }
 static inline void
-perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
-		    bool truncated)					{ }
+perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
+									{ }
 static inline int
 perf_aux_output_skip(struct perf_output_handle *handle,
 		     unsigned long size)				{ return -EINVAL; }
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 6415807169..f3ebafe060 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -297,6 +297,19 @@ ring_buffer_init(struct ring_buffer *rb, long watermark, int flags)
 		rb->paused = 1;
 }
 
+void perf_aux_output_flag(struct perf_output_handle *handle, u64 flags)
+{
+	/*
+	 * OVERWRITE is determined by perf_aux_output_end() and can't
+	 * be passed in directly.
+	 */
+	if (WARN_ON_ONCE(flags & PERF_AUX_FLAG_OVERWRITE))
+		return;
+
+	handle->aux_flags |= flags;
+}
+EXPORT_SYMBOL_GPL(perf_aux_output_flag);
+
 /*
  * This is called before hardware starts writing to the AUX area to
  * obtain an output handle and make sure there's room in the buffer.
@@ -360,6 +373,7 @@ void *perf_aux_output_begin(struct perf_output_handle *handle,
 	handle->event = event;
 	handle->head = aux_head;
 	handle->size = 0;
+	handle->aux_flags = 0;
 
 	/*
 	 * In overwrite mode, AUX data stores do not depend on aux_tail,
@@ -409,34 +423,32 @@ EXPORT_SYMBOL_GPL(perf_aux_output_begin);
  * of the AUX buffer management code is that after pmu::stop(), the AUX
  * transaction must be stopped and therefore drop the AUX reference count.
  */
-void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
-			 bool truncated)
+void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
 {
 	struct ring_buffer *rb = handle->rb;
-	bool wakeup = truncated;
+	bool wakeup = !!handle->aux_flags;
 	unsigned long aux_head;
-	u64 flags = 0;
-
-	if (truncated)
-		flags |= PERF_AUX_FLAG_TRUNCATED;
 
 	/* in overwrite mode, driver provides aux_head via handle */
 	if (rb->aux_overwrite) {
-		flags |= PERF_AUX_FLAG_OVERWRITE;
+		handle->aux_flags |= PERF_AUX_FLAG_OVERWRITE;
 
 		aux_head = handle->head;
 		local_set(&rb->aux_head, aux_head);
 	} else {
+		handle->aux_flags &= ~PERF_AUX_FLAG_OVERWRITE;
+
 		aux_head = local_read(&rb->aux_head);
 		local_add(size, &rb->aux_head);
 	}
 
-	if (size || flags) {
+	if (size || handle->aux_flags) {
 		/*
 		 * Only send RECORD_AUX if we have something useful to communicate
 		 */
 
-		perf_event_aux_event(handle->event, aux_head, size, flags);
+		perf_event_aux_event(handle->event, aux_head, size,
+		                     handle->aux_flags);
 	}
 
 	aux_head = rb->user_page->aux_head = local_read(&rb->aux_head);
@@ -447,7 +459,7 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
 	}
 
 	if (wakeup) {
-		if (truncated)
+		if (handle->aux_flags & PERF_AUX_FLAG_TRUNCATED)
 			handle->event->pending_disable = 1;
 		perf_output_wakeup(handle);
 	}
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1584896 — Re: [PATCH 2/4] perf: Keep AUX flags in the output handle

On 20 February 2017 at 06:33, Alexander Shishkin
<alexander.shishkin@linux.intel.com> wrote:
> From: Will Deacon <will.deacon@arm.com>
>
> In preparation for adding more flags to perf AUX records, introduce a
> separate API for setting the flags for a session, rather than appending
> more bool arguments to perf_aux_output_end. This allows to set each
> flag at the time a corresponding condition is detected, instead of
> tracking it in each driver's private state.
>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> ---
>  arch/x86/events/intel/bts.c                      | 16 +++++------
>  arch/x86/events/intel/pt.c                       | 17 ++++++------
>  arch/x86/events/intel/pt.h                       |  1 -
>  drivers/hwtracing/coresight/coresight-etb10.c    |  7 +++--
>  drivers/hwtracing/coresight/coresight-etm-perf.c |  9 +++----
>  drivers/hwtracing/coresight/coresight-priv.h     |  2 --
>  drivers/hwtracing/coresight/coresight-tmc-etf.c  |  7 +++--

For the CS files:
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>

>  include/linux/coresight.h                        |  2 +-
>  include/linux/perf_event.h                       |  8 +++---
>  kernel/events/ring_buffer.c                      | 34 ++++++++++++++++--------
>  10 files changed, 55 insertions(+), 48 deletions(-)
>
> diff --git a/arch/x86/events/intel/bts.c b/arch/x86/events/intel/bts.c
> index 982c9e31da..8ae8c5ce3a 100644
> --- a/arch/x86/events/intel/bts.c
> +++ b/arch/x86/events/intel/bts.c
> @@ -63,7 +63,6 @@ struct bts_buffer {
>         unsigned int    cur_buf;
>         bool            snapshot;
>         local_t         data_size;
> -       local_t         lost;
>         local_t         head;
>         unsigned long   end;
>         void            **data_pages;
> @@ -199,7 +198,8 @@ static void bts_update(struct bts_ctx *bts)
>                         return;
>
>                 if (ds->bts_index >= ds->bts_absolute_maximum)
> -                       local_inc(&buf->lost);
> +                       perf_aux_output_flag(&bts->handle,
> +                                            PERF_AUX_FLAG_TRUNCATED);
>
>                 /*
>                  * old and head are always in the same physical buffer, so we
> @@ -276,7 +276,7 @@ static void bts_event_start(struct perf_event *event, int flags)
>         return;
>
>  fail_end_stop:
> -       perf_aux_output_end(&bts->handle, 0, false);
> +       perf_aux_output_end(&bts->handle, 0);
>
>  fail_stop:
>         event->hw.state = PERF_HES_STOPPED;
> @@ -319,9 +319,8 @@ static void bts_event_stop(struct perf_event *event, int flags)
>                                 bts->handle.head =
>                                         local_xchg(&buf->data_size,
>                                                    buf->nr_pages << PAGE_SHIFT);
> -
> -                       perf_aux_output_end(&bts->handle, local_xchg(&buf->data_size, 0),
> -                                           !!local_xchg(&buf->lost, 0));
> +                       perf_aux_output_end(&bts->handle,
> +                                           local_xchg(&buf->data_size, 0));
>                 }
>
>                 cpuc->ds->bts_index = bts->ds_back.bts_buffer_base;
> @@ -484,8 +483,7 @@ int intel_bts_interrupt(void)
>         if (old_head == local_read(&buf->head))
>                 return handled;
>
> -       perf_aux_output_end(&bts->handle, local_xchg(&buf->data_size, 0),
> -                           !!local_xchg(&buf->lost, 0));
> +       perf_aux_output_end(&bts->handle, local_xchg(&buf->data_size, 0));
>
>         buf = perf_aux_output_begin(&bts->handle, event);
>         if (buf)
> @@ -500,7 +498,7 @@ int intel_bts_interrupt(void)
>                          * cleared handle::event
>                          */
>                         barrier();
> -                       perf_aux_output_end(&bts->handle, 0, false);
> +                       perf_aux_output_end(&bts->handle, 0);
>                 }
>         }
>
> diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
> index d92a60ef08..a2d0050fde 100644
> --- a/arch/x86/events/intel/pt.c
> +++ b/arch/x86/events/intel/pt.c
> @@ -823,7 +823,8 @@ static void pt_handle_status(struct pt *pt)
>                  */
>                 if (!pt_cap_get(PT_CAP_topa_multiple_entries) ||
>                     buf->output_off == sizes(TOPA_ENTRY(buf->cur, buf->cur_idx)->size)) {
> -                       local_inc(&buf->lost);
> +                       perf_aux_output_flag(&pt->handle,
> +                                            PERF_AUX_FLAG_TRUNCATED);
>                         advance++;
>                 }
>         }
> @@ -916,8 +917,10 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
>
>         /* can't stop in the middle of an output region */
>         if (buf->output_off + handle->size + 1 <
> -           sizes(TOPA_ENTRY(buf->cur, buf->cur_idx)->size))
> +           sizes(TOPA_ENTRY(buf->cur, buf->cur_idx)->size)) {
> +               perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>                 return -EINVAL;
> +       }
>
>
>         /* single entry ToPA is handled by marking all regions STOP=1 INT=1 */
> @@ -1269,8 +1272,7 @@ void intel_pt_interrupt(void)
>
>         pt_update_head(pt);
>
> -       perf_aux_output_end(&pt->handle, local_xchg(&buf->data_size, 0),
> -                           local_xchg(&buf->lost, 0));
> +       perf_aux_output_end(&pt->handle, local_xchg(&buf->data_size, 0));
>
>         if (!event->hw.state) {
>                 int ret;
> @@ -1285,7 +1287,7 @@ void intel_pt_interrupt(void)
>                 /* snapshot counters don't use PMI, so it's safe */
>                 ret = pt_buffer_reset_markers(buf, &pt->handle);
>                 if (ret) {
> -                       perf_aux_output_end(&pt->handle, 0, true);
> +                       perf_aux_output_end(&pt->handle, 0);
>                         return;
>                 }
>
> @@ -1357,7 +1359,7 @@ static void pt_event_start(struct perf_event *event, int mode)
>         return;
>
>  fail_end_stop:
> -       perf_aux_output_end(&pt->handle, 0, true);
> +       perf_aux_output_end(&pt->handle, 0);
>  fail_stop:
>         hwc->state = PERF_HES_STOPPED;
>  }
> @@ -1398,8 +1400,7 @@ static void pt_event_stop(struct perf_event *event, int mode)
>                         pt->handle.head =
>                                 local_xchg(&buf->data_size,
>                                            buf->nr_pages << PAGE_SHIFT);
> -               perf_aux_output_end(&pt->handle, local_xchg(&buf->data_size, 0),
> -                                   local_xchg(&buf->lost, 0));
> +               perf_aux_output_end(&pt->handle, local_xchg(&buf->data_size, 0));
>         }
>  }
>
> diff --git a/arch/x86/events/intel/pt.h b/arch/x86/events/intel/pt.h
> index fa81f8d8ed..25fa9710f4 100644
> --- a/arch/x86/events/intel/pt.h
> +++ b/arch/x86/events/intel/pt.h
> @@ -144,7 +144,6 @@ struct pt_buffer {
>         size_t                  output_off;
>         unsigned long           nr_pages;
>         local_t                 data_size;
> -       local_t                 lost;
>         local64_t               head;
>         bool                    snapshot;
>         unsigned long           stop_pos, intr_pos;
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index d7325c6534..82c8ddcf09 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -321,7 +321,7 @@ static int etb_set_buffer(struct coresight_device *csdev,
>
>  static unsigned long etb_reset_buffer(struct coresight_device *csdev,
>                                       struct perf_output_handle *handle,
> -                                     void *sink_config, bool *lost)
> +                                     void *sink_config)
>  {
>         unsigned long size = 0;
>         struct cs_buffers *buf = sink_config;
> @@ -343,7 +343,6 @@ static unsigned long etb_reset_buffer(struct coresight_device *csdev,
>                  * resetting parameters here and squaring off with the ring
>                  * buffer API in the tracer PMU is fine.
>                  */
> -               *lost = !!local_xchg(&buf->lost, 0);
>                 size = local_xchg(&buf->data_size, 0);
>         }
>
> @@ -385,7 +384,7 @@ static void etb_update_buffer(struct coresight_device *csdev,
>                         (unsigned long)write_ptr);
>
>                 write_ptr &= ~(ETB_FRAME_SIZE_WORDS - 1);
> -               local_inc(&buf->lost);
> +               perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>         }
>
>         /*
> @@ -396,7 +395,7 @@ static void etb_update_buffer(struct coresight_device *csdev,
>          */
>         status = readl_relaxed(drvdata->base + ETB_STATUS_REG);
>         if (status & ETB_STATUS_RAM_FULL) {
> -               local_inc(&buf->lost);
> +               perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>                 to_read = capacity;
>                 read_ptr = write_ptr;
>         } else {
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index 70eaa74dc2..47ea0eee67 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -301,7 +301,8 @@ static void etm_event_start(struct perf_event *event, int flags)
>         return;
>
>  fail_end_stop:
> -       perf_aux_output_end(handle, 0, true);
> +       perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
> +       perf_aux_output_end(handle, 0);
>  fail:
>         event->hw.state = PERF_HES_STOPPED;
>         goto out;
> @@ -309,7 +310,6 @@ static void etm_event_start(struct perf_event *event, int flags)
>
>  static void etm_event_stop(struct perf_event *event, int mode)
>  {
> -       bool lost;
>         int cpu = smp_processor_id();
>         unsigned long size;
>         struct coresight_device *sink, *csdev = per_cpu(csdev_src, cpu);
> @@ -347,10 +347,9 @@ static void etm_event_stop(struct perf_event *event, int mode)
>                         return;
>
>                 size = sink_ops(sink)->reset_buffer(sink, handle,
> -                                                   event_data->snk_config,
> -                                                   &lost);
> +                                                   event_data->snk_config);
>
> -               perf_aux_output_end(handle, size, lost);
> +               perf_aux_output_end(handle, size);
>         }
>
>         /* Disabling the path make its elements available to other sessions */
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index ef9d8e93e3..5f662d8205 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -76,7 +76,6 @@ enum cs_mode {
>   * @nr_pages:  max number of pages granted to us
>   * @offset:    offset within the current buffer
>   * @data_size: how much we collected in this run
> - * @lost:      other than zero if we had a HW buffer wrap around
>   * @snapshot:  is this run in snapshot mode
>   * @data_pages:        a handle the ring buffer
>   */
> @@ -85,7 +84,6 @@ struct cs_buffers {
>         unsigned int            nr_pages;
>         unsigned long           offset;
>         local_t                 data_size;
> -       local_t                 lost;
>         bool                    snapshot;
>         void                    **data_pages;
>  };
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> index 1549436e24..aec61a6d5c 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> @@ -329,7 +329,7 @@ static int tmc_set_etf_buffer(struct coresight_device *csdev,
>
>  static unsigned long tmc_reset_etf_buffer(struct coresight_device *csdev,
>                                           struct perf_output_handle *handle,
> -                                         void *sink_config, bool *lost)
> +                                         void *sink_config)
>  {
>         long size = 0;
>         struct cs_buffers *buf = sink_config;
> @@ -350,7 +350,6 @@ static unsigned long tmc_reset_etf_buffer(struct coresight_device *csdev,
>                  * resetting parameters here and squaring off with the ring
>                  * buffer API in the tracer PMU is fine.
>                  */
> -               *lost = !!local_xchg(&buf->lost, 0);
>                 size = local_xchg(&buf->data_size, 0);
>         }
>
> @@ -389,7 +388,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
>          */
>         status = readl_relaxed(drvdata->base + TMC_STS);
>         if (status & TMC_STS_FULL) {
> -               local_inc(&buf->lost);
> +               perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>                 to_read = drvdata->size;
>         } else {
>                 to_read = CIRC_CNT(write_ptr, read_ptr, drvdata->size);
> @@ -434,7 +433,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
>                         read_ptr -= drvdata->size;
>                 /* Tell the HW */
>                 writel_relaxed(read_ptr, drvdata->base + TMC_RRP);
> -               local_inc(&buf->lost);
> +               perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>         }
>
>         cur = buf->cur;
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 2a5982c37d..035c16c9a5 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -201,7 +201,7 @@ struct coresight_ops_sink {
>                           void *sink_config);
>         unsigned long (*reset_buffer)(struct coresight_device *csdev,
>                                       struct perf_output_handle *handle,
> -                                     void *sink_config, bool *lost);
> +                                     void *sink_config);
>         void (*update_buffer)(struct coresight_device *csdev,
>                               struct perf_output_handle *handle,
>                               void *sink_config);
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index a39564314e..cdbaa88dc8 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -812,6 +812,7 @@ struct perf_output_handle {
>         struct ring_buffer              *rb;
>         unsigned long                   wakeup;
>         unsigned long                   size;
> +       u64                             aux_flags;
>         union {
>                 void                    *addr;
>                 unsigned long           head;
> @@ -860,10 +861,11 @@ perf_cgroup_from_task(struct task_struct *task, struct perf_event_context *ctx)
>  extern void *perf_aux_output_begin(struct perf_output_handle *handle,
>                                    struct perf_event *event);
>  extern void perf_aux_output_end(struct perf_output_handle *handle,
> -                               unsigned long size, bool truncated);
> +                               unsigned long size);
>  extern int perf_aux_output_skip(struct perf_output_handle *handle,
>                                 unsigned long size);
>  extern void *perf_get_aux(struct perf_output_handle *handle);
> +extern void perf_aux_output_flag(struct perf_output_handle *handle, u64 flags);
>
>  extern int perf_pmu_register(struct pmu *pmu, const char *name, int type);
>  extern void perf_pmu_unregister(struct pmu *pmu);
> @@ -1278,8 +1280,8 @@ static inline void *
>  perf_aux_output_begin(struct perf_output_handle *handle,
>                       struct perf_event *event)                         { return NULL; }
>  static inline void
> -perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
> -                   bool truncated)                                     { }
> +perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
> +                                                                       { }
>  static inline int
>  perf_aux_output_skip(struct perf_output_handle *handle,
>                      unsigned long size)                                { return -EINVAL; }
> diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
> index 6415807169..f3ebafe060 100644
> --- a/kernel/events/ring_buffer.c
> +++ b/kernel/events/ring_buffer.c
> @@ -297,6 +297,19 @@ ring_buffer_init(struct ring_buffer *rb, long watermark, int flags)
>                 rb->paused = 1;
>  }
>
> +void perf_aux_output_flag(struct perf_output_handle *handle, u64 flags)
> +{
> +       /*
> +        * OVERWRITE is determined by perf_aux_output_end() and can't
> +        * be passed in directly.
> +        */
> +       if (WARN_ON_ONCE(flags & PERF_AUX_FLAG_OVERWRITE))
> +               return;
> +
> +       handle->aux_flags |= flags;
> +}
> +EXPORT_SYMBOL_GPL(perf_aux_output_flag);
> +
>  /*
>   * This is called before hardware starts writing to the AUX area to
>   * obtain an output handle and make sure there's room in the buffer.
> @@ -360,6 +373,7 @@ void *perf_aux_output_begin(struct perf_output_handle *handle,
>         handle->event = event;
>         handle->head = aux_head;
>         handle->size = 0;
> +       handle->aux_flags = 0;
>
>         /*
>          * In overwrite mode, AUX data stores do not depend on aux_tail,
> @@ -409,34 +423,32 @@ EXPORT_SYMBOL_GPL(perf_aux_output_begin);
>   * of the AUX buffer management code is that after pmu::stop(), the AUX
>   * transaction must be stopped and therefore drop the AUX reference count.
>   */
> -void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
> -                        bool truncated)
> +void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
>  {
>         struct ring_buffer *rb = handle->rb;
> -       bool wakeup = truncated;
> +       bool wakeup = !!handle->aux_flags;
>         unsigned long aux_head;
> -       u64 flags = 0;
> -
> -       if (truncated)
> -               flags |= PERF_AUX_FLAG_TRUNCATED;
>
>         /* in overwrite mode, driver provides aux_head via handle */
>         if (rb->aux_overwrite) {
> -               flags |= PERF_AUX_FLAG_OVERWRITE;
> +               handle->aux_flags |= PERF_AUX_FLAG_OVERWRITE;
>
>                 aux_head = handle->head;
>                 local_set(&rb->aux_head, aux_head);
>         } else {
> +               handle->aux_flags &= ~PERF_AUX_FLAG_OVERWRITE;
> +
>                 aux_head = local_read(&rb->aux_head);
>                 local_add(size, &rb->aux_head);
>         }
>
> -       if (size || flags) {
> +       if (size || handle->aux_flags) {
>                 /*
>                  * Only send RECORD_AUX if we have something useful to communicate
>                  */
>
> -               perf_event_aux_event(handle->event, aux_head, size, flags);
> +               perf_event_aux_event(handle->event, aux_head, size,
> +                                    handle->aux_flags);
>         }
>
>         aux_head = rb->user_page->aux_head = local_read(&rb->aux_head);
> @@ -447,7 +459,7 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
>         }
>
>         if (wakeup) {
> -               if (truncated)
> +               if (handle->aux_flags & PERF_AUX_FLAG_TRUNCATED)
>                         handle->event->pending_disable = 1;
>                 perf_output_wakeup(handle);
>         }
> --
> 2.11.0
>

[toc] | [prev] | [next] | [standalone]

#1585200 — Re: [PATCH 2/4] perf: Keep AUX flags in the output handle

Hi Alexander,

Thanks for picking this up/adapting it. Just one comment below

On Mon, Feb 20, 2017 at 03:33:50PM +0200, Alexander Shishkin wrote:
> From: Will Deacon <will.deacon@arm.com>
> 
> In preparation for adding more flags to perf AUX records, introduce a
> separate API for setting the flags for a session, rather than appending
> more bool arguments to perf_aux_output_end. This allows to set each
> flag at the time a corresponding condition is detected, instead of
> tracking it in each driver's private state.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> ---

--->8

> +void perf_aux_output_flag(struct perf_output_handle *handle, u64 flags)
> +{
> +	/*
> +	 * OVERWRITE is determined by perf_aux_output_end() and can't
> +	 * be passed in directly.
> +	 */
> +	if (WARN_ON_ONCE(flags & PERF_AUX_FLAG_OVERWRITE))
> +		return;

Now that you've added this check...

> +	handle->aux_flags |= flags;
> +}
> +EXPORT_SYMBOL_GPL(perf_aux_output_flag);
> +
>  /*
>   * This is called before hardware starts writing to the AUX area to
>   * obtain an output handle and make sure there's room in the buffer.
> @@ -360,6 +373,7 @@ void *perf_aux_output_begin(struct perf_output_handle *handle,
>  	handle->event = event;
>  	handle->head = aux_head;
>  	handle->size = 0;
> +	handle->aux_flags = 0;
>  
>  	/*
>  	 * In overwrite mode, AUX data stores do not depend on aux_tail,
> @@ -409,34 +423,32 @@ EXPORT_SYMBOL_GPL(perf_aux_output_begin);
>   * of the AUX buffer management code is that after pmu::stop(), the AUX
>   * transaction must be stopped and therefore drop the AUX reference count.
>   */
> -void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
> -			 bool truncated)
> +void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
>  {
>  	struct ring_buffer *rb = handle->rb;
> -	bool wakeup = truncated;
> +	bool wakeup = !!handle->aux_flags;
>  	unsigned long aux_head;
> -	u64 flags = 0;
> -
> -	if (truncated)
> -		flags |= PERF_AUX_FLAG_TRUNCATED;
>  
>  	/* in overwrite mode, driver provides aux_head via handle */
>  	if (rb->aux_overwrite) {
> -		flags |= PERF_AUX_FLAG_OVERWRITE;
> +		handle->aux_flags |= PERF_AUX_FLAG_OVERWRITE;
>  
>  		aux_head = handle->head;
>  		local_set(&rb->aux_head, aux_head);
>  	} else {
> +		handle->aux_flags &= ~PERF_AUX_FLAG_OVERWRITE;
> +

... I don't think you need this addition anymore. It's harmless, though.

Will

[toc] | [prev] | [next] | [standalone]

#1585210 — Re: [PATCH 2/4] perf: Keep AUX flags in the output handle

Will Deacon <will.deacon@arm.com> writes:

> Hi Alexander,

Hi Will,

>> +		handle->aux_flags &= ~PERF_AUX_FLAG_OVERWRITE;
>> +
>
> ... I don't think you need this addition anymore. It's harmless, though.

Right, assuming the PMU driver isn't doing anything fishy to
handle->aux_flags directly.

Regards,
--
Alex

[toc] | [prev] | [next] | [standalone]

#1584625 — [PATCH 1/4] perf: Export AUX buffer helpers to modules

From: Will Deacon <will.deacon@arm.com>

Perf PMU drivers using AUX buffers cannot be built as modules unless
the AUX helpers are exported.

This patch exports perf_aux_output_{begin,end,skip} and perf_get_aux to
modules.

Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 kernel/events/ring_buffer.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 257fa460b8..6415807169 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -397,6 +397,7 @@ void *perf_aux_output_begin(struct perf_output_handle *handle,
 
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(perf_aux_output_begin);
 
 /*
  * Commit the data written by hardware into the ring buffer by adjusting
@@ -458,6 +459,7 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
 	rb_free_aux(rb);
 	ring_buffer_put(rb);
 }
+EXPORT_SYMBOL_GPL(perf_aux_output_end);
 
 /*
  * Skip over a given number of bytes in the AUX buffer, due to, for example,
@@ -486,6 +488,7 @@ int perf_aux_output_skip(struct perf_output_handle *handle, unsigned long size)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(perf_aux_output_skip);
 
 void *perf_get_aux(struct perf_output_handle *handle)
 {
@@ -495,6 +498,7 @@ void *perf_get_aux(struct perf_output_handle *handle)
 
 	return handle->rb->aux_priv;
 }
+EXPORT_SYMBOL_GPL(perf_get_aux);
 
 #define PERF_AUX_GFP	(GFP_KERNEL | __GFP_ZERO | __GFP_NOWARN | __GFP_NORETRY)
 
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1584626 — [PATCH 3/4] perf: Add a flag for partial AUX records

Intel PT driver needs to be able to communicate partial AUX transactions,
that is, transactions with gaps in data for reasons other than no room
left in the buffer (i.e. truncated transactions). Therefore, this condition
does not imply a wakeup for the consumer.

To this end, add a new "partial" AUX flag.

Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 include/uapi/linux/perf_event.h | 1 +
 kernel/events/ring_buffer.c     | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index c66a485a24..8306415207 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -885,6 +885,7 @@ enum perf_callchain_context {
  */
 #define PERF_AUX_FLAG_TRUNCATED		0x01	/* record was truncated to fit */
 #define PERF_AUX_FLAG_OVERWRITE		0x02	/* snapshot from overwrite mode */
+#define PERF_AUX_FLAG_PARTIAL		0x04	/* record contains gaps */
 
 #define PERF_FLAG_FD_NO_GROUP		(1UL << 0)
 #define PERF_FLAG_FD_OUTPUT		(1UL << 1)
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index f3ebafe060..cd5e902a27 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -425,8 +425,8 @@ EXPORT_SYMBOL_GPL(perf_aux_output_begin);
  */
 void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
 {
+	bool wakeup = !!(handle->aux_flags & PERF_AUX_FLAG_TRUNCATED);
 	struct ring_buffer *rb = handle->rb;
-	bool wakeup = !!handle->aux_flags;
 	unsigned long aux_head;
 
 	/* in overwrite mode, driver provides aux_head via handle */
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1584633 — [PATCH 4/4] perf/x86/intel/pt: Handle VMX better

Since commit 1c5ac21a0e ("perf/x86/intel/pt: Don't die on VMXON") PT
events depend on re-scheduling to get enabled after a VMX session has
taken place. This is, in particular, a problem for CPU context events,
which don't normally get re-scheduled, unless there is a reason.

This patch changes the VMX handling so that PT event gets re-enabled
when VMX root mode exits.

Also, notify the user when there's a gap in PT data due to VMX root
mode by flagging AUX records as partial.

In combination with vmm_exclusive=0 parameter of the kvm_intel driver,
this will result in trace gaps only for the duration of the guest's
timeslices.

Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 arch/x86/events/intel/pt.c | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index a2d0050fde..a0fd10f11f 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -468,6 +468,7 @@ static u64 pt_config_filters(struct perf_event *event)
 
 static void pt_config(struct perf_event *event)
 {
+	struct pt *pt = this_cpu_ptr(&pt_ctx);
 	u64 reg;
 
 	if (!event->hw.itrace_started) {
@@ -499,11 +500,15 @@ static void pt_config(struct perf_event *event)
 	reg |= (event->attr.config & PT_CONFIG_MASK);
 
 	event->hw.config = reg;
-	wrmsrl(MSR_IA32_RTIT_CTL, reg);
+	if (READ_ONCE(pt->vmx_on))
+		perf_aux_output_flag(&pt->handle, PERF_AUX_FLAG_PARTIAL);
+	else
+		wrmsrl(MSR_IA32_RTIT_CTL, reg);
 }
 
 static void pt_config_stop(struct perf_event *event)
 {
+	struct pt *pt = this_cpu_ptr(&pt_ctx);
 	u64 ctl = READ_ONCE(event->hw.config);
 
 	/* may be already stopped by a PMI */
@@ -511,7 +516,8 @@ static void pt_config_stop(struct perf_event *event)
 		return;
 
 	ctl &= ~RTIT_CTL_TRACEEN;
-	wrmsrl(MSR_IA32_RTIT_CTL, ctl);
+	if (!READ_ONCE(pt->vmx_on))
+		wrmsrl(MSR_IA32_RTIT_CTL, ctl);
 
 	WRITE_ONCE(event->hw.config, ctl);
 
@@ -1251,12 +1257,6 @@ void intel_pt_interrupt(void)
 	if (!READ_ONCE(pt->handle_nmi))
 		return;
 
-	/*
-	 * If VMX is on and PT does not support it, don't touch anything.
-	 */
-	if (READ_ONCE(pt->vmx_on))
-		return;
-
 	if (!event)
 		return;
 
@@ -1316,12 +1316,19 @@ void intel_pt_handle_vmx(int on)
 	local_irq_save(flags);
 	WRITE_ONCE(pt->vmx_on, on);
 
-	if (on) {
-		/* prevent pt_config_stop() from writing RTIT_CTL */
-		event = pt->handle.event;
-		if (event)
-			event->hw.config = 0;
-	}
+	/*
+	 * If an AUX transaction is in progress, it will contain
+	 * gap(s), so flag it PARTIAL to inform the user.
+	 */
+	event = pt->handle.event;
+	if (event)
+		perf_aux_output_flag(&pt->handle,
+		                     PERF_AUX_FLAG_PARTIAL);
+
+	/* Turn PTs back on */
+	if (!on && event)
+		wrmsrl(MSR_IA32_RTIT_CTL, event->hw.config);
+
 	local_irq_restore(flags);
 }
 EXPORT_SYMBOL_GPL(intel_pt_handle_vmx);
@@ -1336,9 +1343,6 @@ static void pt_event_start(struct perf_event *event, int mode)
 	struct pt *pt = this_cpu_ptr(&pt_ctx);
 	struct pt_buffer *buf;
 
-	if (READ_ONCE(pt->vmx_on))
-		return;
-
 	buf = perf_aux_output_begin(&pt->handle, event);
 	if (!buf)
 		goto fail_stop;
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1584667

Alexander Shishkin <alexander.shishkin@linux.intel.com> writes:

> With the vmm_exclusive=0, PT seems to be much more usable on BDW now. This
> patchset does three things:
>  * adds a flag to PERF_RECORD_AUX, signalling that a transaction has gaps
>    in it (due to VMX root mode kicking in),

Hi Arnaldo & Adrian,

In the above context, will something like this be fine?

Regards,
--
Alex

From 5aba03e79c1119408b44435af8c4cee2480b0775 Mon Sep 17 00:00:00 2001
From: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Date: Mon, 20 Feb 2017 17:08:53 +0200
Subject: [PATCH] perf tools: Handle partial AUX records and print a warning

This patch decodes the 'partial' flag in AUX records and prints
a warning to the user, so that they don't have to guess why their
PT traces contain gaps (or missing altogether):

> Warning:
> AUX data had gaps in it 6 times out of 8!
>
> Are you running a KVM guest in the background?

Currently this is the only reason for partial records.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 tools/include/uapi/linux/perf_event.h |  1 +
 tools/perf/util/event.c               |  5 +++--
 tools/perf/util/event.h               |  1 +
 tools/perf/util/session.c             | 17 ++++++++++++++---
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index c66a485a24..8306415207 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -885,6 +885,7 @@ enum perf_callchain_context {
  */
 #define PERF_AUX_FLAG_TRUNCATED		0x01	/* record was truncated to fit */
 #define PERF_AUX_FLAG_OVERWRITE		0x02	/* snapshot from overwrite mode */
+#define PERF_AUX_FLAG_PARTIAL		0x04	/* record contains gaps */
 
 #define PERF_FLAG_FD_NO_GROUP		(1UL << 0)
 #define PERF_FLAG_FD_OUTPUT		(1UL << 1)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 4ea7ce72ed..ba193cd019 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -1153,11 +1153,12 @@ int perf_event__process_exit(struct perf_tool *tool __maybe_unused,
 
 size_t perf_event__fprintf_aux(union perf_event *event, FILE *fp)
 {
-	return fprintf(fp, " offset: %#"PRIx64" size: %#"PRIx64" flags: %#"PRIx64" [%s%s]\n",
+	return fprintf(fp, " offset: %#"PRIx64" size: %#"PRIx64" flags: %#"PRIx64" [%s%s%s]\n",
 		       event->aux.aux_offset, event->aux.aux_size,
 		       event->aux.flags,
 		       event->aux.flags & PERF_AUX_FLAG_TRUNCATED ? "T" : "",
-		       event->aux.flags & PERF_AUX_FLAG_OVERWRITE ? "O" : "");
+		       event->aux.flags & PERF_AUX_FLAG_OVERWRITE ? "O" : "",
+		       event->aux.flags & PERF_AUX_FLAG_PARTIAL   ? "P" : "");
 }
 
 size_t perf_event__fprintf_itrace_start(union perf_event *event, FILE *fp)
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index c735c53a26..d7e53fe176 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -269,6 +269,7 @@ struct events_stats {
 	u64 total_lost;
 	u64 total_lost_samples;
 	u64 total_aux_lost;
+	u64 total_aux_partial;
 	u64 total_invalid_chains;
 	u32 nr_events[PERF_RECORD_HEADER_MAX];
 	u32 nr_non_filtered_samples;
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 4cdbc8f5f1..abdb797fa4 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1258,9 +1258,12 @@ static int machines__deliver_event(struct machines *machines,
 	case PERF_RECORD_UNTHROTTLE:
 		return tool->unthrottle(tool, event, sample, machine);
 	case PERF_RECORD_AUX:
-		if (tool->aux == perf_event__process_aux &&
-		    (event->aux.flags & PERF_AUX_FLAG_TRUNCATED))
-			evlist->stats.total_aux_lost += 1;
+		if (tool->aux == perf_event__process_aux) {
+			if (event->aux.flags & PERF_AUX_FLAG_TRUNCATED)
+				evlist->stats.total_aux_lost += 1;
+			if (event->aux.flags & PERF_AUX_FLAG_PARTIAL)
+				evlist->stats.total_aux_partial += 1;
+		}
 		return tool->aux(tool, event, sample, machine);
 	case PERF_RECORD_ITRACE_START:
 		return tool->itrace_start(tool, event, sample, machine);
@@ -1548,6 +1551,14 @@ static void perf_session__warn_about_errors(const struct perf_session *session)
 			    stats->nr_events[PERF_RECORD_AUX]);
 	}
 
+	if (session->tool->aux == perf_event__process_aux &&
+	    stats->total_aux_partial != 0) {
+		ui__warning("AUX data had gaps in it %" PRIu64 " times out of %u!\n\n"
+		            "Are you running a KVM guest in the background?\n\n",
+			    stats->total_aux_partial,
+			    stats->nr_events[PERF_RECORD_AUX]);
+	}
+
 	if (stats->nr_unknown_events != 0) {
 		ui__warning("Found %u unknown events!\n\n"
 			    "Is this an older tool processing a perf.data "
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1584689

On 20/02/17 17:18, Alexander Shishkin wrote:
> Alexander Shishkin <alexander.shishkin@linux.intel.com> writes:
> 
>> With the vmm_exclusive=0, PT seems to be much more usable on BDW now. This
>> patchset does three things:
>>  * adds a flag to PERF_RECORD_AUX, signalling that a transaction has gaps
>>    in it (due to VMX root mode kicking in),
> 
> Hi Arnaldo & Adrian,
> 
> In the above context, will something like this be fine?

Looks fine to me.

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


> 
> Regards,
> --
> Alex
> 
>>From 5aba03e79c1119408b44435af8c4cee2480b0775 Mon Sep 17 00:00:00 2001
> From: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Date: Mon, 20 Feb 2017 17:08:53 +0200
> Subject: [PATCH] perf tools: Handle partial AUX records and print a warning
> 
> This patch decodes the 'partial' flag in AUX records and prints
> a warning to the user, so that they don't have to guess why their
> PT traces contain gaps (or missing altogether):
> 
>> Warning:
>> AUX data had gaps in it 6 times out of 8!
>>
>> Are you running a KVM guest in the background?
> 
> Currently this is the only reason for partial records.
> 
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> ---
>  tools/include/uapi/linux/perf_event.h |  1 +
>  tools/perf/util/event.c               |  5 +++--
>  tools/perf/util/event.h               |  1 +
>  tools/perf/util/session.c             | 17 ++++++++++++++---
>  4 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
> index c66a485a24..8306415207 100644
> --- a/tools/include/uapi/linux/perf_event.h
> +++ b/tools/include/uapi/linux/perf_event.h
> @@ -885,6 +885,7 @@ enum perf_callchain_context {
>   */
>  #define PERF_AUX_FLAG_TRUNCATED		0x01	/* record was truncated to fit */
>  #define PERF_AUX_FLAG_OVERWRITE		0x02	/* snapshot from overwrite mode */
> +#define PERF_AUX_FLAG_PARTIAL		0x04	/* record contains gaps */
>  
>  #define PERF_FLAG_FD_NO_GROUP		(1UL << 0)
>  #define PERF_FLAG_FD_OUTPUT		(1UL << 1)
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 4ea7ce72ed..ba193cd019 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -1153,11 +1153,12 @@ int perf_event__process_exit(struct perf_tool *tool __maybe_unused,
>  
>  size_t perf_event__fprintf_aux(union perf_event *event, FILE *fp)
>  {
> -	return fprintf(fp, " offset: %#"PRIx64" size: %#"PRIx64" flags: %#"PRIx64" [%s%s]\n",
> +	return fprintf(fp, " offset: %#"PRIx64" size: %#"PRIx64" flags: %#"PRIx64" [%s%s%s]\n",
>  		       event->aux.aux_offset, event->aux.aux_size,
>  		       event->aux.flags,
>  		       event->aux.flags & PERF_AUX_FLAG_TRUNCATED ? "T" : "",
> -		       event->aux.flags & PERF_AUX_FLAG_OVERWRITE ? "O" : "");
> +		       event->aux.flags & PERF_AUX_FLAG_OVERWRITE ? "O" : "",
> +		       event->aux.flags & PERF_AUX_FLAG_PARTIAL   ? "P" : "");
>  }
>  
>  size_t perf_event__fprintf_itrace_start(union perf_event *event, FILE *fp)
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index c735c53a26..d7e53fe176 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -269,6 +269,7 @@ struct events_stats {
>  	u64 total_lost;
>  	u64 total_lost_samples;
>  	u64 total_aux_lost;
> +	u64 total_aux_partial;
>  	u64 total_invalid_chains;
>  	u32 nr_events[PERF_RECORD_HEADER_MAX];
>  	u32 nr_non_filtered_samples;
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 4cdbc8f5f1..abdb797fa4 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1258,9 +1258,12 @@ static int machines__deliver_event(struct machines *machines,
>  	case PERF_RECORD_UNTHROTTLE:
>  		return tool->unthrottle(tool, event, sample, machine);
>  	case PERF_RECORD_AUX:
> -		if (tool->aux == perf_event__process_aux &&
> -		    (event->aux.flags & PERF_AUX_FLAG_TRUNCATED))
> -			evlist->stats.total_aux_lost += 1;
> +		if (tool->aux == perf_event__process_aux) {
> +			if (event->aux.flags & PERF_AUX_FLAG_TRUNCATED)
> +				evlist->stats.total_aux_lost += 1;
> +			if (event->aux.flags & PERF_AUX_FLAG_PARTIAL)
> +				evlist->stats.total_aux_partial += 1;
> +		}
>  		return tool->aux(tool, event, sample, machine);
>  	case PERF_RECORD_ITRACE_START:
>  		return tool->itrace_start(tool, event, sample, machine);
> @@ -1548,6 +1551,14 @@ static void perf_session__warn_about_errors(const struct perf_session *session)
>  			    stats->nr_events[PERF_RECORD_AUX]);
>  	}
>  
> +	if (session->tool->aux == perf_event__process_aux &&
> +	    stats->total_aux_partial != 0) {
> +		ui__warning("AUX data had gaps in it %" PRIu64 " times out of %u!\n\n"
> +		            "Are you running a KVM guest in the background?\n\n",
> +			    stats->total_aux_partial,
> +			    stats->nr_events[PERF_RECORD_AUX]);
> +	}
> +
>  	if (stats->nr_unknown_events != 0) {
>  		ui__warning("Found %u unknown events!\n\n"
>  			    "Is this an older tool processing a perf.data "
> 

[toc] | [prev] | [next] | [standalone]

#1584708

Em Mon, Feb 20, 2017 at 05:39:43PM +0200, Adrian Hunter escreveu:
> On 20/02/17 17:18, Alexander Shishkin wrote:
> > Alexander Shishkin <alexander.shishkin@linux.intel.com> writes:
> > 
> >> With the vmm_exclusive=0, PT seems to be much more usable on BDW now. This
> >> patchset does three things:
> >>  * adds a flag to PERF_RECORD_AUX, signalling that a transaction has gaps
> >>    in it (due to VMX root mode kicking in),
> > 
> > In the above context, will something like this be fine?
 
> Looks fine to me.
 
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> > From: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Subject: [PATCH] perf tools: Handle partial AUX records and print a warning

> > This patch decodes the 'partial' flag in AUX records and prints
> > a warning to the user, so that they don't have to guess why their
> > PT traces contain gaps (or missing altogether):

> >> Warning:
> >> AUX data had gaps in it 6 times out of 8!

The above should be left for a more verbose mode?

> >> Are you running a KVM guest in the background?

The warning should be a bit more precise, as you said, tuning
vmm_exclusive is key here, i.e.:

"Are you running a KVM guest in the background with
kvm_intel.vmm_exclusive=1?"

And that we can even figure out, its just a matter of reading:

[root@jouet ~]# cat /sys/module/kvm_intel/parameters/vmm_exclusive
Y

I have tested after setting that using:

 modprobe kvm_intel vmm_exclusive=n

And I was able to get Intel PT records from a workload.

So perhaps we can get this patch in, which improves the situation, and
then, on top of it do these extra checks and give proper hints, ok?

- Arnaldo
 
> > Currently this is the only reason for partial records.
> > 
> > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > ---
> >  tools/include/uapi/linux/perf_event.h |  1 +
> >  tools/perf/util/event.c               |  5 +++--
> >  tools/perf/util/event.h               |  1 +
> >  tools/perf/util/session.c             | 17 ++++++++++++++---
> >  4 files changed, 19 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
> > index c66a485a24..8306415207 100644
> > --- a/tools/include/uapi/linux/perf_event.h
> > +++ b/tools/include/uapi/linux/perf_event.h
> > @@ -885,6 +885,7 @@ enum perf_callchain_context {
> >   */
> >  #define PERF_AUX_FLAG_TRUNCATED		0x01	/* record was truncated to fit */
> >  #define PERF_AUX_FLAG_OVERWRITE		0x02	/* snapshot from overwrite mode */
> > +#define PERF_AUX_FLAG_PARTIAL		0x04	/* record contains gaps */
> >  
> >  #define PERF_FLAG_FD_NO_GROUP		(1UL << 0)
> >  #define PERF_FLAG_FD_OUTPUT		(1UL << 1)
> > diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> > index 4ea7ce72ed..ba193cd019 100644
> > --- a/tools/perf/util/event.c
> > +++ b/tools/perf/util/event.c
> > @@ -1153,11 +1153,12 @@ int perf_event__process_exit(struct perf_tool *tool __maybe_unused,
> >  
> >  size_t perf_event__fprintf_aux(union perf_event *event, FILE *fp)
> >  {
> > -	return fprintf(fp, " offset: %#"PRIx64" size: %#"PRIx64" flags: %#"PRIx64" [%s%s]\n",
> > +	return fprintf(fp, " offset: %#"PRIx64" size: %#"PRIx64" flags: %#"PRIx64" [%s%s%s]\n",
> >  		       event->aux.aux_offset, event->aux.aux_size,
> >  		       event->aux.flags,
> >  		       event->aux.flags & PERF_AUX_FLAG_TRUNCATED ? "T" : "",
> > -		       event->aux.flags & PERF_AUX_FLAG_OVERWRITE ? "O" : "");
> > +		       event->aux.flags & PERF_AUX_FLAG_OVERWRITE ? "O" : "",
> > +		       event->aux.flags & PERF_AUX_FLAG_PARTIAL   ? "P" : "");
> >  }
> >  
> >  size_t perf_event__fprintf_itrace_start(union perf_event *event, FILE *fp)
> > diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> > index c735c53a26..d7e53fe176 100644
> > --- a/tools/perf/util/event.h
> > +++ b/tools/perf/util/event.h
> > @@ -269,6 +269,7 @@ struct events_stats {
> >  	u64 total_lost;
> >  	u64 total_lost_samples;
> >  	u64 total_aux_lost;
> > +	u64 total_aux_partial;
> >  	u64 total_invalid_chains;
> >  	u32 nr_events[PERF_RECORD_HEADER_MAX];
> >  	u32 nr_non_filtered_samples;
> > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> > index 4cdbc8f5f1..abdb797fa4 100644
> > --- a/tools/perf/util/session.c
> > +++ b/tools/perf/util/session.c
> > @@ -1258,9 +1258,12 @@ static int machines__deliver_event(struct machines *machines,
> >  	case PERF_RECORD_UNTHROTTLE:
> >  		return tool->unthrottle(tool, event, sample, machine);
> >  	case PERF_RECORD_AUX:
> > -		if (tool->aux == perf_event__process_aux &&
> > -		    (event->aux.flags & PERF_AUX_FLAG_TRUNCATED))
> > -			evlist->stats.total_aux_lost += 1;
> > +		if (tool->aux == perf_event__process_aux) {
> > +			if (event->aux.flags & PERF_AUX_FLAG_TRUNCATED)
> > +				evlist->stats.total_aux_lost += 1;
> > +			if (event->aux.flags & PERF_AUX_FLAG_PARTIAL)
> > +				evlist->stats.total_aux_partial += 1;
> > +		}
> >  		return tool->aux(tool, event, sample, machine);
> >  	case PERF_RECORD_ITRACE_START:
> >  		return tool->itrace_start(tool, event, sample, machine);
> > @@ -1548,6 +1551,14 @@ static void perf_session__warn_about_errors(const struct perf_session *session)
> >  			    stats->nr_events[PERF_RECORD_AUX]);
> >  	}
> >  
> > +	if (session->tool->aux == perf_event__process_aux &&
> > +	    stats->total_aux_partial != 0) {
> > +		ui__warning("AUX data had gaps in it %" PRIu64 " times out of %u!\n\n"
> > +		            "Are you running a KVM guest in the background?\n\n",
> > +			    stats->total_aux_partial,
> > +			    stats->nr_events[PERF_RECORD_AUX]);
> > +	}
> > +
> >  	if (stats->nr_unknown_events != 0) {
> >  		ui__warning("Found %u unknown events!\n\n"
> >  			    "Is this an older tool processing a perf.data "
> > 

[toc] | [prev] | [next] | [standalone]

#1584727

Arnaldo Carvalho de Melo <acme@kernel.org> writes:

> Em Mon, Feb 20, 2017 at 05:39:43PM +0200, Adrian Hunter escreveu:
>> On 20/02/17 17:18, Alexander Shishkin wrote:
>> > Alexander Shishkin <alexander.shishkin@linux.intel.com> writes:
>> > 
>> >> With the vmm_exclusive=0, PT seems to be much more usable on BDW now. This
>> >> patchset does three things:
>> >>  * adds a flag to PERF_RECORD_AUX, signalling that a transaction has gaps
>> >>    in it (due to VMX root mode kicking in),
>> > 
>> > In the above context, will something like this be fine?
>  
>> Looks fine to me.
>  
>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>> 
>> > From: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> > Subject: [PATCH] perf tools: Handle partial AUX records and print a warning
>
>> > This patch decodes the 'partial' flag in AUX records and prints
>> > a warning to the user, so that they don't have to guess why their
>> > PT traces contain gaps (or missing altogether):
>
>> >> Warning:
>> >> AUX data had gaps in it 6 times out of 8!
>
> The above should be left for a more verbose mode?
>
>> >> Are you running a KVM guest in the background?
>
> The warning should be a bit more precise, as you said, tuning
> vmm_exclusive is key here, i.e.:
>
> "Are you running a KVM guest in the background with
> kvm_intel.vmm_exclusive=1?"

You'll still get gaps with vmm_exclusive=0 if you run perf record -a or
if you try to trace the actual kvm.

> And that we can even figure out, its just a matter of reading:
>
> [root@jouet ~]# cat /sys/module/kvm_intel/parameters/vmm_exclusive
> Y
>
> I have tested after setting that using:
>
>  modprobe kvm_intel vmm_exclusive=n
>
> And I was able to get Intel PT records from a workload.
>
> So perhaps we can get this patch in, which improves the situation, and
> then, on top of it do these extra checks and give proper hints, ok?

Sure.

Thanks,
--
Alex

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web