linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/7] perf/x86/intel/pt: Fixes and cleanups
@ 2015-05-22 15:30 Alexander Shishkin
  2015-05-22 15:30 ` [PATCH v1 1/7] perf: Disallow sparse AUX allocations for non-SG PMUs in overwrite mode Alexander Shishkin
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Alexander Shishkin @ 2015-05-22 15:30 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar
  Cc: linux-kernel, Paul Mackerras, adrian.hunter, x86, hpa, acme,
	Alexander Shishkin

Hi Peter and Ingo,

I'm resending the previously posted PT fixes along with two more that I
produced since. One new fix (1/7) is for the perf ring buffer code to make
sure user only gets a single chunk high-order allocation for overwrite mode
on PMUs that don't support hardware scatter lists, such as some versions
of Intel PT.

Another not-entirely-trivial patch (3/7) fixes an issue with multientry
ToPA PT implementations that causes bigger data loss than it actually has
to be, which was due to stop and interrupt markers being misplaced in the
buffer, preventing a timely consumer wakeup.

The rest of the patches are trivial fixes or documentation updates.

These are against tip/perf/urgent.

Alexander Shishkin (7):
  perf: Disallow sparse AUX allocations for non-SG PMUs in overwrite
    mode
  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()
  perf/x86/intel/pt: Remove an extra variable declaration

 arch/x86/kernel/cpu/perf_event_intel_pt.c | 72 ++++++++++++++++++-------------
 kernel/events/ring_buffer.c               | 14 ++++++
 2 files changed, 55 insertions(+), 31 deletions(-)

-- 
2.1.4


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

* [PATCH v1 1/7] perf: Disallow sparse AUX allocations for non-SG PMUs in overwrite mode
  2015-05-22 15:30 [PATCH v1 0/7] perf/x86/intel/pt: Fixes and cleanups Alexander Shishkin
@ 2015-05-22 15:30 ` Alexander Shishkin
  2015-05-27 10:02   ` [tip:perf/core] " tip-bot for Alexander Shishkin
  2015-05-22 15:30 ` [PATCH v1 2/7] perf/x86/intel/pt: Kill an unused variable Alexander Shishkin
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Alexander Shishkin @ 2015-05-22 15:30 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar
  Cc: linux-kernel, Paul Mackerras, adrian.hunter, x86, hpa, acme,
	Alexander Shishkin

PMUs that don't support hardware scatter tables require big contiguous
chunks of memory and a PMI to switch between them. However, in overwrite
using a PMI for this purpose adds extra overhead that the users would
like to avoid. Thus, in overwrite mode for such PMUs we can only allow
one contiguous chunk for the entire requested buffer.

This patch changes the behavior accordingly, so that if the buddy allocator
fails to come up with a single high-order chunk for the entire requested
buffer, the allocation will fail.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 kernel/events/ring_buffer.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 232f00f273..725c416085 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -493,6 +493,20 @@ int rb_alloc_aux(struct ring_buffer *rb, struct perf_event *event,
 			rb->aux_pages[rb->aux_nr_pages] = page_address(page++);
 	}
 
+	/*
+	 * In overwrite mode, PMUs that don't support SG may not handle more
+	 * than one contiguous allocation, since they rely on PMI to do double
+	 * buffering. In this case, the entire buffer has to be one contiguous
+	 * chunk.
+	 */
+	if ((event->pmu->capabilities & PERF_PMU_CAP_AUX_NO_SG) &&
+	    overwrite) {
+		struct page *page = virt_to_page(rb->aux_pages[0]);
+
+		if (page_private(page) != max_order)
+			goto out;
+	}
+
 	rb->aux_priv = event->pmu->setup_aux(event->cpu, rb->aux_pages, nr_pages,
 					     overwrite);
 	if (!rb->aux_priv)
-- 
2.1.4


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

* [PATCH v1 2/7] perf/x86/intel/pt: Kill an unused variable
  2015-05-22 15:30 [PATCH v1 0/7] perf/x86/intel/pt: Fixes and cleanups Alexander Shishkin
  2015-05-22 15:30 ` [PATCH v1 1/7] perf: Disallow sparse AUX allocations for non-SG PMUs in overwrite mode Alexander Shishkin
@ 2015-05-22 15:30 ` Alexander Shishkin
  2015-05-22 15:30 ` [PATCH v1 3/7] perf/x86/intel/pt: Untangle pt_buffer_reset_markers() Alexander Shishkin
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Alexander Shishkin @ 2015-05-22 15:30 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] 12+ messages in thread

* [PATCH v1 3/7] perf/x86/intel/pt: Untangle pt_buffer_reset_markers()
  2015-05-22 15:30 [PATCH v1 0/7] perf/x86/intel/pt: Fixes and cleanups Alexander Shishkin
  2015-05-22 15:30 ` [PATCH v1 1/7] perf: Disallow sparse AUX allocations for non-SG PMUs in overwrite mode Alexander Shishkin
  2015-05-22 15:30 ` [PATCH v1 2/7] perf/x86/intel/pt: Kill an unused variable Alexander Shishkin
@ 2015-05-22 15:30 ` Alexander Shishkin
  2015-05-27 10:02   ` [tip:perf/core] " tip-bot for Alexander Shishkin
  2015-05-22 15:30 ` [PATCH v1 4/7] perf/x86/intel/pt: Document pt_buffer_reset_markers() Alexander Shishkin
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Alexander Shishkin @ 2015-05-22 15:30 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] 12+ messages in thread

* [PATCH v1 4/7] perf/x86/intel/pt: Document pt_buffer_reset_markers()
  2015-05-22 15:30 [PATCH v1 0/7] perf/x86/intel/pt: Fixes and cleanups Alexander Shishkin
                   ` (2 preceding siblings ...)
  2015-05-22 15:30 ` [PATCH v1 3/7] perf/x86/intel/pt: Untangle pt_buffer_reset_markers() Alexander Shishkin
@ 2015-05-22 15:30 ` Alexander Shishkin
  2015-05-22 15:30 ` [PATCH v1 5/7] perf/x86/intel/pt: Document pt_buffer_reset_offsets() Alexander Shishkin
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Alexander Shishkin @ 2015-05-22 15:30 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] 12+ messages in thread

* [PATCH v1 5/7] perf/x86/intel/pt: Document pt_buffer_reset_offsets()
  2015-05-22 15:30 [PATCH v1 0/7] perf/x86/intel/pt: Fixes and cleanups Alexander Shishkin
                   ` (3 preceding siblings ...)
  2015-05-22 15:30 ` [PATCH v1 4/7] perf/x86/intel/pt: Document pt_buffer_reset_markers() Alexander Shishkin
@ 2015-05-22 15:30 ` Alexander Shishkin
  2015-05-22 15:30 ` [PATCH v1 6/7] perf/x86/intel/pt: Kill pt_is_running() Alexander Shishkin
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Alexander Shishkin @ 2015-05-22 15:30 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] 12+ messages in thread

* [PATCH v1 6/7] perf/x86/intel/pt: Kill pt_is_running()
  2015-05-22 15:30 [PATCH v1 0/7] perf/x86/intel/pt: Fixes and cleanups Alexander Shishkin
                   ` (4 preceding siblings ...)
  2015-05-22 15:30 ` [PATCH v1 5/7] perf/x86/intel/pt: Document pt_buffer_reset_offsets() Alexander Shishkin
@ 2015-05-22 15:30 ` Alexander Shishkin
  2015-05-22 15:30 ` [PATCH v1 7/7] perf/x86/intel/pt: Remove an extra variable declaration Alexander Shishkin
  2015-05-22 17:04 ` [PATCH v1 0/7] perf/x86/intel/pt: Fixes and cleanups Alexander Shishkin
  7 siblings, 0 replies; 12+ messages in thread
From: Alexander Shishkin @ 2015-05-22 15:30 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] 12+ messages in thread

* [PATCH v1 7/7] perf/x86/intel/pt: Remove an extra variable declaration
  2015-05-22 15:30 [PATCH v1 0/7] perf/x86/intel/pt: Fixes and cleanups Alexander Shishkin
                   ` (5 preceding siblings ...)
  2015-05-22 15:30 ` [PATCH v1 6/7] perf/x86/intel/pt: Kill pt_is_running() Alexander Shishkin
@ 2015-05-22 15:30 ` Alexander Shishkin
  2015-05-27 10:08   ` [tip:perf/core] perf/x86/intel/pt: Remove redundant " tip-bot for Alexander Shishkin
  2015-05-22 17:04 ` [PATCH v1 0/7] perf/x86/intel/pt: Fixes and cleanups Alexander Shishkin
  7 siblings, 1 reply; 12+ messages in thread
From: Alexander Shishkin @ 2015-05-22 15:30 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar
  Cc: linux-kernel, Paul Mackerras, adrian.hunter, x86, hpa, acme,
	Alexander Shishkin

There is a pt variable in the outer scope of pt_event_stop() with the same
type, we don't really need another one in the inner scope.

This patch removes a redundant variable declaration.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 arch/x86/kernel/cpu/perf_event_intel_pt.c | 1 -
 1 file changed, 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 a2d407172d..59596d25cb 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_pt.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_pt.c
@@ -955,7 +955,6 @@ static void pt_event_stop(struct perf_event *event, int mode)
 	event->hw.state = PERF_HES_STOPPED;
 
 	if (mode & PERF_EF_UPDATE) {
-		struct pt *pt = this_cpu_ptr(&pt_ctx);
 		struct pt_buffer *buf = perf_get_aux(&pt->handle);
 
 		if (!buf)
-- 
2.1.4


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

* Re: [PATCH v1 0/7] perf/x86/intel/pt: Fixes and cleanups
  2015-05-22 15:30 [PATCH v1 0/7] perf/x86/intel/pt: Fixes and cleanups Alexander Shishkin
                   ` (6 preceding siblings ...)
  2015-05-22 15:30 ` [PATCH v1 7/7] perf/x86/intel/pt: Remove an extra variable declaration Alexander Shishkin
@ 2015-05-22 17:04 ` Alexander Shishkin
  7 siblings, 0 replies; 12+ messages in thread
From: Alexander Shishkin @ 2015-05-22 17:04 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar
  Cc: linux-kernel, Paul Mackerras, adrian.hunter, x86, hpa, acme

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

> Hi Peter and Ingo,

Hi again,

I didn't notice that you've already applied the previous batch to your
queue, apologies for that.

The two new patches in this series are:

>   perf: Disallow sparse AUX allocations for non-SG PMUs in overwrite
>     mode

and

>   perf/x86/intel/pt: Remove an extra variable declaration

Regards,
--
Alex

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

* [tip:perf/core] perf: Disallow sparse AUX allocations for non-SG PMUs in overwrite mode
  2015-05-22 15:30 ` [PATCH v1 1/7] perf: Disallow sparse AUX allocations for non-SG PMUs in overwrite mode Alexander Shishkin
@ 2015-05-27 10:02   ` tip-bot for Alexander Shishkin
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Alexander Shishkin @ 2015-05-27 10:02 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, torvalds, alexander.shishkin, mingo, peterz, paulus,
	linux-kernel, tglx

Commit-ID:  aa319bcd366349c6f72fcd331da89d3d06090651
Gitweb:     http://git.kernel.org/tip/aa319bcd366349c6f72fcd331da89d3d06090651
Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
AuthorDate: Fri, 22 May 2015 18:30:20 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 27 May 2015 09:16:20 +0200

perf: Disallow sparse AUX allocations for non-SG PMUs in overwrite mode

PMUs that don't support hardware scatter tables require big contiguous
chunks of memory and a PMI to switch between them. However, in overwrite
using a PMI for this purpose adds extra overhead that the users would
like to avoid. Thus, in overwrite mode for such PMUs we can only allow
one contiguous chunk for the entire requested buffer.

This patch changes the behavior accordingly, so that if the buddy allocator
fails to come up with a single high-order chunk for the entire requested
buffer, the allocation will fail.

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/1432308626-18845-2-git-send-email-alexander.shishkin@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/ring_buffer.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 232f00f..725c416 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -493,6 +493,20 @@ int rb_alloc_aux(struct ring_buffer *rb, struct perf_event *event,
 			rb->aux_pages[rb->aux_nr_pages] = page_address(page++);
 	}
 
+	/*
+	 * In overwrite mode, PMUs that don't support SG may not handle more
+	 * than one contiguous allocation, since they rely on PMI to do double
+	 * buffering. In this case, the entire buffer has to be one contiguous
+	 * chunk.
+	 */
+	if ((event->pmu->capabilities & PERF_PMU_CAP_AUX_NO_SG) &&
+	    overwrite) {
+		struct page *page = virt_to_page(rb->aux_pages[0]);
+
+		if (page_private(page) != max_order)
+			goto out;
+	}
+
 	rb->aux_priv = event->pmu->setup_aux(event->cpu, rb->aux_pages, nr_pages,
 					     overwrite);
 	if (!rb->aux_priv)

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

* [tip:perf/core] perf/x86/intel/pt: Untangle pt_buffer_reset_markers()
  2015-05-22 15:30 ` [PATCH v1 3/7] perf/x86/intel/pt: Untangle pt_buffer_reset_markers() Alexander Shishkin
@ 2015-05-27 10:02   ` tip-bot for Alexander Shishkin
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Alexander Shishkin @ 2015-05-27 10:02 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, alexander.shishkin, peterz, paulus, linux-kernel, torvalds,
	hpa, mingo

Commit-ID:  f73ec48c90016f89d05726f6c48e66991a790fd7
Gitweb:     http://git.kernel.org/tip/f73ec48c90016f89d05726f6c48e66991a790fd7
Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
AuthorDate: Fri, 22 May 2015 18:30:22 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 27 May 2015 09:16:20 +0200

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

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

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>
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/1432308626-18845-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 | 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 ffe666c..5b804f9 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;

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

* [tip:perf/core] perf/x86/intel/pt: Remove redundant variable declaration
  2015-05-22 15:30 ` [PATCH v1 7/7] perf/x86/intel/pt: Remove an extra variable declaration Alexander Shishkin
@ 2015-05-27 10:08   ` tip-bot for Alexander Shishkin
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Alexander Shishkin @ 2015-05-27 10:08 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, paulus, alexander.shishkin, mingo, linux-kernel, hpa,
	torvalds, peterz

Commit-ID:  a82d24edfeaf1ed244cf8b969916840c6feb5165
Gitweb:     http://git.kernel.org/tip/a82d24edfeaf1ed244cf8b969916840c6feb5165
Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
AuthorDate: Fri, 22 May 2015 18:30:26 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 27 May 2015 09:17:48 +0200

perf/x86/intel/pt: Remove redundant variable declaration

There is a 'pt' variable in the outer scope of pt_event_stop() with the same
type, we don't really need another one in the inner scope.

This patch removes the redundant variable declaration.

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/1432308626-18845-8-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 | 1 -
 1 file changed, 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 a2d4071..59596d2 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_pt.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_pt.c
@@ -955,7 +955,6 @@ static void pt_event_stop(struct perf_event *event, int mode)
 	event->hw.state = PERF_HES_STOPPED;
 
 	if (mode & PERF_EF_UPDATE) {
-		struct pt *pt = this_cpu_ptr(&pt_ctx);
 		struct pt_buffer *buf = perf_get_aux(&pt->handle);
 
 		if (!buf)

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

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

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-22 15:30 [PATCH v1 0/7] perf/x86/intel/pt: Fixes and cleanups Alexander Shishkin
2015-05-22 15:30 ` [PATCH v1 1/7] perf: Disallow sparse AUX allocations for non-SG PMUs in overwrite mode Alexander Shishkin
2015-05-27 10:02   ` [tip:perf/core] " tip-bot for Alexander Shishkin
2015-05-22 15:30 ` [PATCH v1 2/7] perf/x86/intel/pt: Kill an unused variable Alexander Shishkin
2015-05-22 15:30 ` [PATCH v1 3/7] perf/x86/intel/pt: Untangle pt_buffer_reset_markers() Alexander Shishkin
2015-05-27 10:02   ` [tip:perf/core] " tip-bot for Alexander Shishkin
2015-05-22 15:30 ` [PATCH v1 4/7] perf/x86/intel/pt: Document pt_buffer_reset_markers() Alexander Shishkin
2015-05-22 15:30 ` [PATCH v1 5/7] perf/x86/intel/pt: Document pt_buffer_reset_offsets() Alexander Shishkin
2015-05-22 15:30 ` [PATCH v1 6/7] perf/x86/intel/pt: Kill pt_is_running() Alexander Shishkin
2015-05-22 15:30 ` [PATCH v1 7/7] perf/x86/intel/pt: Remove an extra variable declaration Alexander Shishkin
2015-05-27 10:08   ` [tip:perf/core] perf/x86/intel/pt: Remove redundant " tip-bot for Alexander Shishkin
2015-05-22 17:04 ` [PATCH v1 0/7] perf/x86/intel/pt: Fixes and cleanups 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).