linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] perf/x86/intel/pt: Driver fixes and cleanups
@ 2015-04-21 13:16 Alexander Shishkin
  2015-04-21 13:16 ` [PATCH 1/5] perf/x86/intel/pt: Kill an unused variable Alexander Shishkin
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Alexander Shishkin @ 2015-04-21 13:16 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar
  Cc: linux-kernel, Paul Mackerras, adrian.hunter, x86, hpa, acme,
	Alexander Shishkin

Hi Peter and Ingo,

Here is a small series of cleanups I came up with while looking at issues
raised by Dan Carpenter and Ingo. They are mostly trivial, except for 2/5,
which fixes an issue with multientry ToPA PT implementations that causes
bigger data loss than it actually has to be.

These are against tip/perf/urgent.

Alexander Shishkin (5):
  perf/x86/intel/pt: Kill an unused variable
  perf/x86/intel/pt: Untangle pt_buffer_reset_markers()
  perf/x86/intel/pt: Document pt_buffer_reset_markers()
  perf/x86/intel/pt: Document pt_buffer_reset_offsets()
  perf/x86/intel/pt: Kill pt_is_running()

 arch/x86/kernel/cpu/perf_event_intel_pt.c | 71 ++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 30 deletions(-)

-- 
2.1.4


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

* [PATCH 1/5] perf/x86/intel/pt: Kill an unused variable
  2015-04-21 13:16 [PATCH 0/5] perf/x86/intel/pt: Driver fixes and cleanups Alexander Shishkin
@ 2015-04-21 13:16 ` Alexander Shishkin
  2015-05-27 10:07   ` [tip:perf/core] " tip-bot for Alexander Shishkin
  2015-04-21 13:16 ` [PATCH 2/5] perf/x86/intel/pt: Untangle pt_buffer_reset_markers() Alexander Shishkin
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Alexander Shishkin @ 2015-04-21 13:16 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar
  Cc: linux-kernel, Paul Mackerras, adrian.hunter, x86, hpa, acme,
	Alexander Shishkin

Currently, there's a set-but-not-used variable in setup_topa_index();
this patch gets rid of it. And while at it, fixes a style issue with
brackets around a one-line block.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 arch/x86/kernel/cpu/perf_event_intel_pt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_pt.c b/arch/x86/kernel/cpu/perf_event_intel_pt.c
index ffe666c2c6..ac08b67c07 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_pt.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_pt.c
@@ -664,7 +664,7 @@ static void pt_buffer_setup_topa_index(struct pt_buffer *buf)
 	struct topa *cur = buf->first, *prev = buf->last;
 	struct topa_entry *te_cur = TOPA_ENTRY(cur, 0),
 		*te_prev = TOPA_ENTRY(prev, prev->last - 1);
-	int pg = 0, idx = 0, ntopa = 0;
+	int pg = 0, idx = 0;
 
 	while (pg < buf->nr_pages) {
 		int tidx;
@@ -679,9 +679,9 @@ static void pt_buffer_setup_topa_index(struct pt_buffer *buf)
 			/* advance to next topa table */
 			idx = 0;
 			cur = list_entry(cur->list.next, struct topa, list);
-			ntopa++;
-		} else
+		} else {
 			idx++;
+		}
 		te_cur = TOPA_ENTRY(cur, idx);
 	}
 
-- 
2.1.4


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

* [PATCH 2/5] perf/x86/intel/pt: Untangle pt_buffer_reset_markers()
  2015-04-21 13:16 [PATCH 0/5] perf/x86/intel/pt: Driver fixes and cleanups Alexander Shishkin
  2015-04-21 13:16 ` [PATCH 1/5] perf/x86/intel/pt: Kill an unused variable Alexander Shishkin
@ 2015-04-21 13:16 ` Alexander Shishkin
  2015-04-21 13:16 ` [PATCH 3/5] perf/x86/intel/pt: Document pt_buffer_reset_markers() Alexander Shishkin
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Alexander Shishkin @ 2015-04-21 13:16 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar
  Cc: linux-kernel, Paul Mackerras, adrian.hunter, x86, hpa, acme,
	Alexander Shishkin

Currently, pt_buffer_reset_markers() is a difficult to read knot of
arithmetics with a redundant check for multiple-entry topa capability,
a commented out wakeup marker placement and a logical error wrt to
stop marker placement. The latter happens when write head is not page
aligned and results in stop marker being placed one page earlier than
it actually should.

All these problems only affect PT implementations that support
multiple-entry topa tables (read: proper sg). For single-entry topa
implementations, there is no functional impact.

This patch deals with all of the above. Tested on both single-entry
and multiple-entry topa PT implementations.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 arch/x86/kernel/cpu/perf_event_intel_pt.c | 34 ++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_pt.c b/arch/x86/kernel/cpu/perf_event_intel_pt.c
index ac08b67c07..8a4595650d 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_pt.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_pt.c
@@ -615,7 +615,8 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
 				   struct perf_output_handle *handle)
 
 {
-	unsigned long idx, npages, end;
+	unsigned long head = local64_read(&buf->head);
+	unsigned long idx, npages, wakeup;
 
 	if (buf->snapshot)
 		return 0;
@@ -634,17 +635,26 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
 	buf->topa_index[buf->stop_pos]->stop = 0;
 	buf->topa_index[buf->intr_pos]->intr = 0;
 
-	if (pt_cap_get(PT_CAP_topa_multiple_entries)) {
-		npages = (handle->size + 1) >> PAGE_SHIFT;
-		end = (local64_read(&buf->head) >> PAGE_SHIFT) + npages;
-		/*if (end > handle->wakeup >> PAGE_SHIFT)
-		  end = handle->wakeup >> PAGE_SHIFT;*/
-		idx = end & (buf->nr_pages - 1);
-		buf->stop_pos = idx;
-		idx = (local64_read(&buf->head) >> PAGE_SHIFT) + npages - 1;
-		idx &= buf->nr_pages - 1;
-		buf->intr_pos = idx;
-	}
+	/* how many pages till the STOP marker */
+	npages = handle->size >> PAGE_SHIFT;
+
+	/* if it's on a page boundary, fill up one more page */
+	if (!offset_in_page(head + handle->size + 1))
+		npages++;
+
+	idx = (head >> PAGE_SHIFT) + npages;
+	idx &= buf->nr_pages - 1;
+	buf->stop_pos = idx;
+
+	wakeup = handle->wakeup >> PAGE_SHIFT;
+
+	/* in the worst case, wake up the consumer one page before hard stop */
+	idx = (head >> PAGE_SHIFT) + npages - 1;
+	if (idx > wakeup)
+		idx = wakeup;
+
+	idx &= buf->nr_pages - 1;
+	buf->intr_pos = idx;
 
 	buf->topa_index[buf->stop_pos]->stop = 1;
 	buf->topa_index[buf->intr_pos]->intr = 1;
-- 
2.1.4


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

* [PATCH 3/5] perf/x86/intel/pt: Document pt_buffer_reset_markers()
  2015-04-21 13:16 [PATCH 0/5] perf/x86/intel/pt: Driver fixes and cleanups Alexander Shishkin
  2015-04-21 13:16 ` [PATCH 1/5] perf/x86/intel/pt: Kill an unused variable Alexander Shishkin
  2015-04-21 13:16 ` [PATCH 2/5] perf/x86/intel/pt: Untangle pt_buffer_reset_markers() Alexander Shishkin
@ 2015-04-21 13:16 ` Alexander Shishkin
  2015-05-27 10:07   ` [tip:perf/core] " tip-bot for Alexander Shishkin
  2015-04-21 13:16 ` [PATCH 4/5] perf/x86/intel/pt: Document pt_buffer_reset_offsets() Alexander Shishkin
  2015-04-21 13:16 ` [PATCH 5/5] perf/x86/intel/pt: Kill pt_is_running() Alexander Shishkin
  4 siblings, 1 reply; 10+ messages in thread
From: Alexander Shishkin @ 2015-04-21 13:16 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar
  Cc: linux-kernel, Paul Mackerras, adrian.hunter, x86, hpa, acme,
	Alexander Shishkin

The comments in the driver don't make it absolutely clear as to what
exactly is the calling order and other possible constraints of buffer
management functions.

Document constraints and calling order for the buffer configuration
functions. While at it, replace a redundant check in
pt_buffer_reset_markers() with an explanation why it is not needed.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 arch/x86/kernel/cpu/perf_event_intel_pt.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_pt.c b/arch/x86/kernel/cpu/perf_event_intel_pt.c
index 8a4595650d..b2746eafa0 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_pt.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_pt.c
@@ -609,7 +609,12 @@ static unsigned int pt_topa_next_entry(struct pt_buffer *buf, unsigned int pg)
  * @handle:	Current output handle.
  *
  * Place INT and STOP marks to prevent overwriting old data that the consumer
- * hasn't yet collected.
+ * hasn't yet collected and waking up the consumer after a certain fraction of
+ * the buffer has filled up. Only needed and sensible for non-snapshot counters.
+ *
+ * This obviously relies on buf::head to figure out buffer markers, so it has
+ * to be called after pt_buffer_reset_offsets() and before the hardware tracing
+ * is enabled.
  */
 static int pt_buffer_reset_markers(struct pt_buffer *buf,
 				   struct perf_output_handle *handle)
@@ -618,9 +623,6 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
 	unsigned long head = local64_read(&buf->head);
 	unsigned long idx, npages, wakeup;
 
-	if (buf->snapshot)
-		return 0;
-
 	/* 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))
@@ -901,6 +903,7 @@ void intel_pt_interrupt(void)
 		}
 
 		pt_buffer_reset_offsets(buf, pt->handle.head);
+		/* 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);
-- 
2.1.4


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

* [PATCH 4/5] perf/x86/intel/pt: Document pt_buffer_reset_offsets()
  2015-04-21 13:16 [PATCH 0/5] perf/x86/intel/pt: Driver fixes and cleanups Alexander Shishkin
                   ` (2 preceding siblings ...)
  2015-04-21 13:16 ` [PATCH 3/5] perf/x86/intel/pt: Document pt_buffer_reset_markers() Alexander Shishkin
@ 2015-04-21 13:16 ` Alexander Shishkin
  2015-05-27 10:08   ` [tip:perf/core] " tip-bot for Alexander Shishkin
  2015-04-21 13:16 ` [PATCH 5/5] perf/x86/intel/pt: Kill pt_is_running() Alexander Shishkin
  4 siblings, 1 reply; 10+ messages in thread
From: Alexander Shishkin @ 2015-04-21 13:16 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar
  Cc: linux-kernel, Paul Mackerras, adrian.hunter, x86, hpa, acme,
	Alexander Shishkin

Currently, the description of pt_buffer_reset_offsets() lacks information
about its calling constraints and ordering with regards to other buffer
management functions.

Add a clarification about when this function has to be called.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 arch/x86/kernel/cpu/perf_event_intel_pt.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_pt.c b/arch/x86/kernel/cpu/perf_event_intel_pt.c
index b2746eafa0..40ba5e4312 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_pt.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_pt.c
@@ -705,7 +705,14 @@ static void pt_buffer_setup_topa_index(struct pt_buffer *buf)
  * @head:	Write pointer (aux_head) from AUX buffer.
  *
  * Find the ToPA table and entry corresponding to given @head and set buffer's
- * "current" pointers accordingly.
+ * "current" pointers accordingly. This is done after we have obtained the
+ * current aux_head position from a successful call to perf_aux_output_begin()
+ * to make sure the hardware is writing to the right place.
+ *
+ * This function modifies buf::{cur,cur_idx,output_off} that will be programmed
+ * into PT msrs when the tracing is enabled and buf::head and buf::data_size,
+ * which are used to determine INT and STOP markers' locations by a subsequent
+ * call to pt_buffer_reset_markers().
  */
 static void pt_buffer_reset_offsets(struct pt_buffer *buf, unsigned long head)
 {
-- 
2.1.4


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

* [PATCH 5/5] perf/x86/intel/pt: Kill pt_is_running()
  2015-04-21 13:16 [PATCH 0/5] perf/x86/intel/pt: Driver fixes and cleanups Alexander Shishkin
                   ` (3 preceding siblings ...)
  2015-04-21 13:16 ` [PATCH 4/5] perf/x86/intel/pt: Document pt_buffer_reset_offsets() Alexander Shishkin
@ 2015-04-21 13:16 ` Alexander Shishkin
  2015-05-27 10:08   ` [tip:perf/core] " tip-bot for Alexander Shishkin
  4 siblings, 1 reply; 10+ messages in thread
From: Alexander Shishkin @ 2015-04-21 13:16 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar
  Cc: linux-kernel, Paul Mackerras, adrian.hunter, x86, hpa, acme,
	Alexander Shishkin

Initially, we were trying to guard against scenarios where somebody
attaches to the system with a hardware debugger while PT is enabled
from software and pt_is_running() tries to make sure we handle this
better, but the truth is, there is still a race window no matter what
and people with hardware debuggers should really know what they are
doing anyway. In other words, there is no point in keeping this one
around, and it's one rdmsr fewer in the fast path.

One case when PT is enabled by the bios at the boot time is handled
in driver initialization path and doesn't use pt_is_running().

This patch gets rid of it.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 arch/x86/kernel/cpu/perf_event_intel_pt.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_pt.c b/arch/x86/kernel/cpu/perf_event_intel_pt.c
index 40ba5e4312..a2d407172d 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_pt.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_pt.c
@@ -187,15 +187,6 @@ static bool pt_event_valid(struct perf_event *event)
  * These all are cpu affine and operate on a local PT
  */
 
-static bool pt_is_running(void)
-{
-	u64 ctl;
-
-	rdmsrl(MSR_IA32_RTIT_CTL, ctl);
-
-	return !!(ctl & RTIT_CTL_TRACEEN);
-}
-
 static void pt_config(struct perf_event *event)
 {
 	u64 reg;
@@ -933,7 +924,7 @@ static void pt_event_start(struct perf_event *event, int mode)
 	struct pt *pt = this_cpu_ptr(&pt_ctx);
 	struct pt_buffer *buf = perf_get_aux(&pt->handle);
 
-	if (pt_is_running() || !buf || pt_buffer_is_full(buf, pt)) {
+	if (!buf || pt_buffer_is_full(buf, pt)) {
 		event->hw.state = PERF_HES_STOPPED;
 		return;
 	}
-- 
2.1.4


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

* [tip:perf/core] perf/x86/intel/pt: Kill an unused variable
  2015-04-21 13:16 ` [PATCH 1/5] perf/x86/intel/pt: Kill an unused variable Alexander Shishkin
@ 2015-05-27 10:07   ` tip-bot for Alexander Shishkin
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Alexander Shishkin @ 2015-05-27 10:07 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, tglx, linux-kernel, mingo, torvalds, hpa,
	alexander.shishkin, paulus

Commit-ID:  74387bcb711b7b2ed65c0ed08953e13d4e31969e
Gitweb:     http://git.kernel.org/tip/74387bcb711b7b2ed65c0ed08953e13d4e31969e
Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
AuthorDate: Tue, 21 Apr 2015 16:16:13 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 27 May 2015 09:17:46 +0200

perf/x86/intel/pt: Kill an unused variable

Currently, there's a set-but-not-used variable in setup_topa_index();
this patch gets rid of it. And while at it, fixes a style issue with
brackets around a one-line block.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: acme@infradead.org
Cc: adrian.hunter@intel.com
Cc: hpa@zytor.com
Link: http://lkml.kernel.org/r/1429622177-22843-2-git-send-email-alexander.shishkin@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/cpu/perf_event_intel_pt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_pt.c b/arch/x86/kernel/cpu/perf_event_intel_pt.c
index 5b804f9..8a45956 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_pt.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_pt.c
@@ -674,7 +674,7 @@ static void pt_buffer_setup_topa_index(struct pt_buffer *buf)
 	struct topa *cur = buf->first, *prev = buf->last;
 	struct topa_entry *te_cur = TOPA_ENTRY(cur, 0),
 		*te_prev = TOPA_ENTRY(prev, prev->last - 1);
-	int pg = 0, idx = 0, ntopa = 0;
+	int pg = 0, idx = 0;
 
 	while (pg < buf->nr_pages) {
 		int tidx;
@@ -689,9 +689,9 @@ static void pt_buffer_setup_topa_index(struct pt_buffer *buf)
 			/* advance to next topa table */
 			idx = 0;
 			cur = list_entry(cur->list.next, struct topa, list);
-			ntopa++;
-		} else
+		} else {
 			idx++;
+		}
 		te_cur = TOPA_ENTRY(cur, idx);
 	}
 

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

* [tip:perf/core] perf/x86/intel/pt: Document pt_buffer_reset_markers()
  2015-04-21 13:16 ` [PATCH 3/5] perf/x86/intel/pt: Document pt_buffer_reset_markers() Alexander Shishkin
@ 2015-05-27 10:07   ` tip-bot for Alexander Shishkin
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Alexander Shishkin @ 2015-05-27 10:07 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, mingo, paulus, linux-kernel, peterz, alexander.shishkin,
	torvalds, tglx

Commit-ID:  cf302bfdf3039853fce812ae1ffd0ac24f5b468f
Gitweb:     http://git.kernel.org/tip/cf302bfdf3039853fce812ae1ffd0ac24f5b468f
Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
AuthorDate: Tue, 21 Apr 2015 16:16:15 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 27 May 2015 09:17:47 +0200

perf/x86/intel/pt: Document pt_buffer_reset_markers()

The comments in the driver don't make it absolutely clear as to what
exactly is the calling order and other possible constraints of buffer
management functions.

Document constraints and calling order for the buffer configuration
functions. While at it, replace a redundant check in
pt_buffer_reset_markers() with an explanation why it is not needed.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: acme@infradead.org
Cc: adrian.hunter@intel.com
Cc: hpa@zytor.com
Link: http://lkml.kernel.org/r/1429622177-22843-4-git-send-email-alexander.shishkin@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/cpu/perf_event_intel_pt.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_pt.c b/arch/x86/kernel/cpu/perf_event_intel_pt.c
index 8a45956..b2746ea 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_pt.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_pt.c
@@ -609,7 +609,12 @@ static unsigned int pt_topa_next_entry(struct pt_buffer *buf, unsigned int pg)
  * @handle:	Current output handle.
  *
  * Place INT and STOP marks to prevent overwriting old data that the consumer
- * hasn't yet collected.
+ * hasn't yet collected and waking up the consumer after a certain fraction of
+ * the buffer has filled up. Only needed and sensible for non-snapshot counters.
+ *
+ * This obviously relies on buf::head to figure out buffer markers, so it has
+ * to be called after pt_buffer_reset_offsets() and before the hardware tracing
+ * is enabled.
  */
 static int pt_buffer_reset_markers(struct pt_buffer *buf,
 				   struct perf_output_handle *handle)
@@ -618,9 +623,6 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
 	unsigned long head = local64_read(&buf->head);
 	unsigned long idx, npages, wakeup;
 
-	if (buf->snapshot)
-		return 0;
-
 	/* 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))
@@ -901,6 +903,7 @@ void intel_pt_interrupt(void)
 		}
 
 		pt_buffer_reset_offsets(buf, pt->handle.head);
+		/* 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);

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

* [tip:perf/core] perf/x86/intel/pt: Document pt_buffer_reset_offsets()
  2015-04-21 13:16 ` [PATCH 4/5] perf/x86/intel/pt: Document pt_buffer_reset_offsets() Alexander Shishkin
@ 2015-05-27 10:08   ` tip-bot for Alexander Shishkin
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Alexander Shishkin @ 2015-05-27 10:08 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: paulus, alexander.shishkin, torvalds, mingo, linux-kernel,
	peterz, hpa, tglx

Commit-ID:  5b1dbd17c0dee679b154ce47f534677b7e0f7ad6
Gitweb:     http://git.kernel.org/tip/5b1dbd17c0dee679b154ce47f534677b7e0f7ad6
Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
AuthorDate: Tue, 21 Apr 2015 16:16:16 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 27 May 2015 09:17:47 +0200

perf/x86/intel/pt: Document pt_buffer_reset_offsets()

Currently, the description of pt_buffer_reset_offsets() lacks information
about its calling constraints and ordering with regards to other buffer
management functions.

Add a clarification about when this function has to be called.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: acme@infradead.org
Cc: adrian.hunter@intel.com
Cc: hpa@zytor.com
Link: http://lkml.kernel.org/r/1429622177-22843-5-git-send-email-alexander.shishkin@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/cpu/perf_event_intel_pt.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_pt.c b/arch/x86/kernel/cpu/perf_event_intel_pt.c
index b2746ea..40ba5e4 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_pt.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_pt.c
@@ -705,7 +705,14 @@ static void pt_buffer_setup_topa_index(struct pt_buffer *buf)
  * @head:	Write pointer (aux_head) from AUX buffer.
  *
  * Find the ToPA table and entry corresponding to given @head and set buffer's
- * "current" pointers accordingly.
+ * "current" pointers accordingly. This is done after we have obtained the
+ * current aux_head position from a successful call to perf_aux_output_begin()
+ * to make sure the hardware is writing to the right place.
+ *
+ * This function modifies buf::{cur,cur_idx,output_off} that will be programmed
+ * into PT msrs when the tracing is enabled and buf::head and buf::data_size,
+ * which are used to determine INT and STOP markers' locations by a subsequent
+ * call to pt_buffer_reset_markers().
  */
 static void pt_buffer_reset_offsets(struct pt_buffer *buf, unsigned long head)
 {

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

* [tip:perf/core] perf/x86/intel/pt: Kill pt_is_running()
  2015-04-21 13:16 ` [PATCH 5/5] perf/x86/intel/pt: Kill pt_is_running() Alexander Shishkin
@ 2015-05-27 10:08   ` tip-bot for Alexander Shishkin
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Alexander Shishkin @ 2015-05-27 10:08 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, linux-kernel, torvalds, hpa, paulus, alexander.shishkin,
	tglx, mingo

Commit-ID:  0a487aad2dfd088bcbbe1766944280b40ff969a5
Gitweb:     http://git.kernel.org/tip/0a487aad2dfd088bcbbe1766944280b40ff969a5
Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
AuthorDate: Tue, 21 Apr 2015 16:16:17 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 27 May 2015 09:17:48 +0200

perf/x86/intel/pt: Kill pt_is_running()

Initially, we were trying to guard against scenarios where somebody
attaches to the system with a hardware debugger while PT is enabled
from software and pt_is_running() tries to make sure we handle this
better, but the truth is, there is still a race window no matter what
and people with hardware debuggers should really know what they are
doing anyway.

In other words, there is no point in keeping this one around, and
it's one RDMSR instructions fewer in the fast path.

The case when PT is enabled by the BIOS at boot time is handled
in the driver initialization path and doesn't use pt_is_running().

This patch gets rid of it.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: acme@infradead.org
Cc: adrian.hunter@intel.com
Cc: hpa@zytor.com
Link: http://lkml.kernel.org/r/1429622177-22843-6-git-send-email-alexander.shishkin@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/cpu/perf_event_intel_pt.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_pt.c b/arch/x86/kernel/cpu/perf_event_intel_pt.c
index 40ba5e4..a2d4071 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_pt.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_pt.c
@@ -187,15 +187,6 @@ static bool pt_event_valid(struct perf_event *event)
  * These all are cpu affine and operate on a local PT
  */
 
-static bool pt_is_running(void)
-{
-	u64 ctl;
-
-	rdmsrl(MSR_IA32_RTIT_CTL, ctl);
-
-	return !!(ctl & RTIT_CTL_TRACEEN);
-}
-
 static void pt_config(struct perf_event *event)
 {
 	u64 reg;
@@ -933,7 +924,7 @@ static void pt_event_start(struct perf_event *event, int mode)
 	struct pt *pt = this_cpu_ptr(&pt_ctx);
 	struct pt_buffer *buf = perf_get_aux(&pt->handle);
 
-	if (pt_is_running() || !buf || pt_buffer_is_full(buf, pt)) {
+	if (!buf || pt_buffer_is_full(buf, pt)) {
 		event->hw.state = PERF_HES_STOPPED;
 		return;
 	}

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

end of thread, other threads:[~2015-05-27 10:09 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-21 13:16 [PATCH 0/5] perf/x86/intel/pt: Driver fixes and cleanups Alexander Shishkin
2015-04-21 13:16 ` [PATCH 1/5] perf/x86/intel/pt: Kill an unused variable Alexander Shishkin
2015-05-27 10:07   ` [tip:perf/core] " tip-bot for Alexander Shishkin
2015-04-21 13:16 ` [PATCH 2/5] perf/x86/intel/pt: Untangle pt_buffer_reset_markers() Alexander Shishkin
2015-04-21 13:16 ` [PATCH 3/5] perf/x86/intel/pt: Document pt_buffer_reset_markers() Alexander Shishkin
2015-05-27 10:07   ` [tip:perf/core] " tip-bot for Alexander Shishkin
2015-04-21 13:16 ` [PATCH 4/5] perf/x86/intel/pt: Document pt_buffer_reset_offsets() Alexander Shishkin
2015-05-27 10:08   ` [tip:perf/core] " tip-bot for Alexander Shishkin
2015-04-21 13:16 ` [PATCH 5/5] perf/x86/intel/pt: Kill pt_is_running() Alexander Shishkin
2015-05-27 10:08   ` [tip:perf/core] " tip-bot for Alexander Shishkin

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