* [PATCH v2 1/2] perf/aux: Make aux_{head,wakeup} ring_buffer members long
@ 2017-08-16 16:18 Will Deacon
2017-08-16 16:18 ` [PATCH v2 2/2] perf/aux: Ensure aux_wakeup represents most recent wakeup index Will Deacon
2017-08-25 11:52 ` [tip:perf/core] perf/aux: Make aux_{head,wakeup} ring_buffer members long tip-bot for Will Deacon
0 siblings, 2 replies; 4+ messages in thread
From: Will Deacon @ 2017-08-16 16:18 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arm-kernel, mark.rutland, alexander.shishkin, Will Deacon,
Peter Zijlstra
The aux_head and aux_wakeup members of struct ring_buffer are defined
using the local_t type, despite the fact that they are only accessed via
the perf_aux_output_* functions, which cannot race with each other for a
given ring buffer.
This patch changes the type of the members to long, so we can avoid
using the local_* API where it isn't needed.
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
kernel/events/internal.h | 4 ++--
kernel/events/ring_buffer.c | 31 ++++++++++++++-----------------
2 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/kernel/events/internal.h b/kernel/events/internal.h
index 486fd78eb8d5..2941b868353c 100644
--- a/kernel/events/internal.h
+++ b/kernel/events/internal.h
@@ -38,9 +38,9 @@ struct ring_buffer {
struct user_struct *mmap_user;
/* AUX area */
- local_t aux_head;
+ long aux_head;
local_t aux_nest;
- local_t aux_wakeup;
+ long aux_wakeup;
unsigned long aux_pgoff;
int aux_nr_pages;
int aux_overwrite;
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 14a240ff439d..87c28faca8e4 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -367,7 +367,7 @@ void *perf_aux_output_begin(struct perf_output_handle *handle,
if (WARN_ON_ONCE(local_xchg(&rb->aux_nest, 1)))
goto err_put;
- aux_head = local_read(&rb->aux_head);
+ aux_head = rb->aux_head;
handle->rb = rb;
handle->event = event;
@@ -382,7 +382,7 @@ void *perf_aux_output_begin(struct perf_output_handle *handle,
*/
if (!rb->aux_overwrite) {
aux_tail = ACCESS_ONCE(rb->user_page->aux_tail);
- handle->wakeup = local_read(&rb->aux_wakeup) + rb->aux_watermark;
+ handle->wakeup = rb->aux_wakeup + rb->aux_watermark;
if (aux_head - aux_tail < perf_aux_size(rb))
handle->size = CIRC_SPACE(aux_head, aux_tail, perf_aux_size(rb));
@@ -434,12 +434,12 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
handle->aux_flags |= PERF_AUX_FLAG_OVERWRITE;
aux_head = handle->head;
- local_set(&rb->aux_head, aux_head);
+ 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);
+ aux_head = rb->aux_head;
+ rb->aux_head += size;
}
if (size || handle->aux_flags) {
@@ -451,11 +451,10 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
handle->aux_flags);
}
- aux_head = rb->user_page->aux_head = local_read(&rb->aux_head);
-
- if (aux_head - local_read(&rb->aux_wakeup) >= rb->aux_watermark) {
+ rb->user_page->aux_head = rb->aux_head;
+ if (rb->aux_head - rb->aux_wakeup >= rb->aux_watermark) {
wakeup = true;
- local_add(rb->aux_watermark, &rb->aux_wakeup);
+ rb->aux_wakeup += rb->aux_watermark;
}
if (wakeup) {
@@ -480,22 +479,20 @@ EXPORT_SYMBOL_GPL(perf_aux_output_end);
int perf_aux_output_skip(struct perf_output_handle *handle, unsigned long size)
{
struct ring_buffer *rb = handle->rb;
- unsigned long aux_head;
if (size > handle->size)
return -ENOSPC;
- local_add(size, &rb->aux_head);
+ rb->aux_head += size;
- aux_head = rb->user_page->aux_head = local_read(&rb->aux_head);
- if (aux_head - local_read(&rb->aux_wakeup) >= rb->aux_watermark) {
+ rb->user_page->aux_head = rb->aux_head;
+ if (rb->aux_head - rb->aux_wakeup >= rb->aux_watermark) {
perf_output_wakeup(handle);
- local_add(rb->aux_watermark, &rb->aux_wakeup);
- handle->wakeup = local_read(&rb->aux_wakeup) +
- rb->aux_watermark;
+ rb->aux_wakeup += rb->aux_watermark;
+ handle->wakeup = rb->aux_wakeup + rb->aux_watermark;
}
- handle->head = aux_head;
+ handle->head = rb->aux_head;
handle->size -= size;
return 0;
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] perf/aux: Ensure aux_wakeup represents most recent wakeup index
2017-08-16 16:18 [PATCH v2 1/2] perf/aux: Make aux_{head,wakeup} ring_buffer members long Will Deacon
@ 2017-08-16 16:18 ` Will Deacon
2017-08-25 11:52 ` [tip:perf/core] " tip-bot for Will Deacon
2017-08-25 11:52 ` [tip:perf/core] perf/aux: Make aux_{head,wakeup} ring_buffer members long tip-bot for Will Deacon
1 sibling, 1 reply; 4+ messages in thread
From: Will Deacon @ 2017-08-16 16:18 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arm-kernel, mark.rutland, alexander.shishkin, Will Deacon,
Peter Zijlstra
The aux_watermark member of struct ring_buffer represents the period (in
terms of bytes) at which wakeup events should be generated when data is
written to the aux buffer in non-snapshot mode. On hardware that cannot
generate an interrupt when the aux_head reaches an arbitrary wakeup index
(such as ARM SPE), the aux_head sampled from handle->head in
perf_aux_output_{skip,end} may in fact be past the wakeup index. This
can lead to wakeup slowly falling behind the head. For example, consider
the case where hardware can only generate an interrupt on a page-boundary
and the aux buffer is initialised as follows:
// Buffer size is 2 * PAGE_SIZE
rb->aux_head = rb->aux_wakeup = 0
rb->aux_watermark = PAGE_SIZE / 2
following the first perf_aux_output_begin call, the handle is
initialised with:
handle->head = 0
handle->size = 2 * PAGE_SIZE
handle->wakeup = PAGE_SIZE / 2
and the hardware will be programmed to generate an interrupt at
PAGE_SIZE.
When the interrupt is raised, the hardware head will be at PAGE_SIZE,
so calling perf_aux_output_end(handle, PAGE_SIZE) puts the ring buffer
into the following state:
rb->aux_head = PAGE_SIZE
rb->aux_wakeup = PAGE_SIZE / 2
rb->aux_watermark = PAGE_SIZE / 2
and then the next call to perf_aux_output_begin will result in:
handle->head = handle->wakeup = PAGE_SIZE
for which the semantics are unclear and, for a smaller aux_watermark
(e.g. PAGE_SIZE / 4), then the wakeup would in fact be behind head at
this point.
This patch fixes the problem by rounding down the aux_head (as sampled
from the handle) to the nearest aux_watermark boundary when updating
rb->aux_wakeup, therefore taking into account any overruns by the
hardware.
Cc: Peter Zijlstra <peterz@infradead.org>
Acked-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Reported-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
kernel/events/internal.h | 2 +-
kernel/events/ring_buffer.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/events/internal.h b/kernel/events/internal.h
index 2941b868353c..5377c591c57a 100644
--- a/kernel/events/internal.h
+++ b/kernel/events/internal.h
@@ -40,7 +40,7 @@ struct ring_buffer {
/* AUX area */
long aux_head;
local_t aux_nest;
- long aux_wakeup;
+ long aux_wakeup; /* last aux_watermark boundary crossed by aux_head */
unsigned long aux_pgoff;
int aux_nr_pages;
int aux_overwrite;
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 87c28faca8e4..dcb2e8c4ad54 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -454,7 +454,7 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
rb->user_page->aux_head = rb->aux_head;
if (rb->aux_head - rb->aux_wakeup >= rb->aux_watermark) {
wakeup = true;
- rb->aux_wakeup += rb->aux_watermark;
+ rb->aux_wakeup = rounddown(rb->aux_head, rb->aux_watermark);
}
if (wakeup) {
@@ -488,7 +488,7 @@ int perf_aux_output_skip(struct perf_output_handle *handle, unsigned long size)
rb->user_page->aux_head = rb->aux_head;
if (rb->aux_head - rb->aux_wakeup >= rb->aux_watermark) {
perf_output_wakeup(handle);
- rb->aux_wakeup += rb->aux_watermark;
+ rb->aux_wakeup = rounddown(rb->aux_head, rb->aux_watermark);
handle->wakeup = rb->aux_wakeup + rb->aux_watermark;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [tip:perf/core] perf/aux: Make aux_{head,wakeup} ring_buffer members long
2017-08-16 16:18 [PATCH v2 1/2] perf/aux: Make aux_{head,wakeup} ring_buffer members long Will Deacon
2017-08-16 16:18 ` [PATCH v2 2/2] perf/aux: Ensure aux_wakeup represents most recent wakeup index Will Deacon
@ 2017-08-25 11:52 ` tip-bot for Will Deacon
1 sibling, 0 replies; 4+ messages in thread
From: tip-bot for Will Deacon @ 2017-08-25 11:52 UTC (permalink / raw)
To: linux-tip-commits
Cc: torvalds, mark.rutland, hpa, tglx, alexander.shishkin, mingo,
linux-kernel, peterz, will.deacon
Commit-ID: 2ab346cfb0decf01523949e29f5cf542f2304611
Gitweb: http://git.kernel.org/tip/2ab346cfb0decf01523949e29f5cf542f2304611
Author: Will Deacon <will.deacon@arm.com>
AuthorDate: Wed, 16 Aug 2017 17:18:16 +0100
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 25 Aug 2017 11:04:15 +0200
perf/aux: Make aux_{head,wakeup} ring_buffer members long
The aux_head and aux_wakeup members of struct ring_buffer are defined
using the local_t type, despite the fact that they are only accessed via
the perf_aux_output_*() functions, which cannot race with each other for a
given ring buffer.
This patch changes the type of the members to long, so we can avoid
using the local_*() API where it isn't needed.
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/1502900297-21839-1-git-send-email-will.deacon@arm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/events/internal.h | 4 ++--
kernel/events/ring_buffer.c | 31 ++++++++++++++-----------------
2 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/kernel/events/internal.h b/kernel/events/internal.h
index 486fd78..2941b86 100644
--- a/kernel/events/internal.h
+++ b/kernel/events/internal.h
@@ -38,9 +38,9 @@ struct ring_buffer {
struct user_struct *mmap_user;
/* AUX area */
- local_t aux_head;
+ long aux_head;
local_t aux_nest;
- local_t aux_wakeup;
+ long aux_wakeup;
unsigned long aux_pgoff;
int aux_nr_pages;
int aux_overwrite;
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index ee97196..25437fd 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -367,7 +367,7 @@ void *perf_aux_output_begin(struct perf_output_handle *handle,
if (WARN_ON_ONCE(local_xchg(&rb->aux_nest, 1)))
goto err_put;
- aux_head = local_read(&rb->aux_head);
+ aux_head = rb->aux_head;
handle->rb = rb;
handle->event = event;
@@ -382,7 +382,7 @@ void *perf_aux_output_begin(struct perf_output_handle *handle,
*/
if (!rb->aux_overwrite) {
aux_tail = ACCESS_ONCE(rb->user_page->aux_tail);
- handle->wakeup = local_read(&rb->aux_wakeup) + rb->aux_watermark;
+ handle->wakeup = rb->aux_wakeup + rb->aux_watermark;
if (aux_head - aux_tail < perf_aux_size(rb))
handle->size = CIRC_SPACE(aux_head, aux_tail, perf_aux_size(rb));
@@ -433,12 +433,12 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
handle->aux_flags |= PERF_AUX_FLAG_OVERWRITE;
aux_head = handle->head;
- local_set(&rb->aux_head, aux_head);
+ 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);
+ aux_head = rb->aux_head;
+ rb->aux_head += size;
}
if (size || handle->aux_flags) {
@@ -450,11 +450,10 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
handle->aux_flags);
}
- aux_head = rb->user_page->aux_head = local_read(&rb->aux_head);
-
- if (aux_head - local_read(&rb->aux_wakeup) >= rb->aux_watermark) {
+ rb->user_page->aux_head = rb->aux_head;
+ if (rb->aux_head - rb->aux_wakeup >= rb->aux_watermark) {
wakeup = true;
- local_add(rb->aux_watermark, &rb->aux_wakeup);
+ rb->aux_wakeup += rb->aux_watermark;
}
if (wakeup) {
@@ -478,22 +477,20 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
int perf_aux_output_skip(struct perf_output_handle *handle, unsigned long size)
{
struct ring_buffer *rb = handle->rb;
- unsigned long aux_head;
if (size > handle->size)
return -ENOSPC;
- local_add(size, &rb->aux_head);
+ rb->aux_head += size;
- aux_head = rb->user_page->aux_head = local_read(&rb->aux_head);
- if (aux_head - local_read(&rb->aux_wakeup) >= rb->aux_watermark) {
+ rb->user_page->aux_head = rb->aux_head;
+ if (rb->aux_head - rb->aux_wakeup >= rb->aux_watermark) {
perf_output_wakeup(handle);
- local_add(rb->aux_watermark, &rb->aux_wakeup);
- handle->wakeup = local_read(&rb->aux_wakeup) +
- rb->aux_watermark;
+ rb->aux_wakeup += rb->aux_watermark;
+ handle->wakeup = rb->aux_wakeup + rb->aux_watermark;
}
- handle->head = aux_head;
+ handle->head = rb->aux_head;
handle->size -= size;
return 0;
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [tip:perf/core] perf/aux: Ensure aux_wakeup represents most recent wakeup index
2017-08-16 16:18 ` [PATCH v2 2/2] perf/aux: Ensure aux_wakeup represents most recent wakeup index Will Deacon
@ 2017-08-25 11:52 ` tip-bot for Will Deacon
0 siblings, 0 replies; 4+ messages in thread
From: tip-bot for Will Deacon @ 2017-08-25 11:52 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, will.deacon, alexander.shishkin, tglx, peterz,
mark.rutland, torvalds, mingo
Commit-ID: d9a50b0256f06bd39a1bed1ba40baec37c356b11
Gitweb: http://git.kernel.org/tip/d9a50b0256f06bd39a1bed1ba40baec37c356b11
Author: Will Deacon <will.deacon@arm.com>
AuthorDate: Wed, 16 Aug 2017 17:18:17 +0100
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 25 Aug 2017 11:04:16 +0200
perf/aux: Ensure aux_wakeup represents most recent wakeup index
The aux_watermark member of struct ring_buffer represents the period (in
terms of bytes) at which wakeup events should be generated when data is
written to the aux buffer in non-snapshot mode. On hardware that cannot
generate an interrupt when the aux_head reaches an arbitrary wakeup index
(such as ARM SPE), the aux_head sampled from handle->head in
perf_aux_output_{skip,end} may in fact be past the wakeup index. This
can lead to wakeup slowly falling behind the head. For example, consider
the case where hardware can only generate an interrupt on a page-boundary
and the aux buffer is initialised as follows:
// Buffer size is 2 * PAGE_SIZE
rb->aux_head = rb->aux_wakeup = 0
rb->aux_watermark = PAGE_SIZE / 2
following the first perf_aux_output_begin call, the handle is
initialised with:
handle->head = 0
handle->size = 2 * PAGE_SIZE
handle->wakeup = PAGE_SIZE / 2
and the hardware will be programmed to generate an interrupt at
PAGE_SIZE.
When the interrupt is raised, the hardware head will be at PAGE_SIZE,
so calling perf_aux_output_end(handle, PAGE_SIZE) puts the ring buffer
into the following state:
rb->aux_head = PAGE_SIZE
rb->aux_wakeup = PAGE_SIZE / 2
rb->aux_watermark = PAGE_SIZE / 2
and then the next call to perf_aux_output_begin will result in:
handle->head = handle->wakeup = PAGE_SIZE
for which the semantics are unclear and, for a smaller aux_watermark
(e.g. PAGE_SIZE / 4), then the wakeup would in fact be behind head at
this point.
This patch fixes the problem by rounding down the aux_head (as sampled
from the handle) to the nearest aux_watermark boundary when updating
rb->aux_wakeup, therefore taking into account any overruns by the
hardware.
Reported-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/1502900297-21839-2-git-send-email-will.deacon@arm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/events/internal.h | 2 +-
kernel/events/ring_buffer.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/events/internal.h b/kernel/events/internal.h
index 2941b86..5377c59 100644
--- a/kernel/events/internal.h
+++ b/kernel/events/internal.h
@@ -40,7 +40,7 @@ struct ring_buffer {
/* AUX area */
long aux_head;
local_t aux_nest;
- long aux_wakeup;
+ long aux_wakeup; /* last aux_watermark boundary crossed by aux_head */
unsigned long aux_pgoff;
int aux_nr_pages;
int aux_overwrite;
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 25437fd..af71a84e 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -453,7 +453,7 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size)
rb->user_page->aux_head = rb->aux_head;
if (rb->aux_head - rb->aux_wakeup >= rb->aux_watermark) {
wakeup = true;
- rb->aux_wakeup += rb->aux_watermark;
+ rb->aux_wakeup = rounddown(rb->aux_head, rb->aux_watermark);
}
if (wakeup) {
@@ -486,7 +486,7 @@ int perf_aux_output_skip(struct perf_output_handle *handle, unsigned long size)
rb->user_page->aux_head = rb->aux_head;
if (rb->aux_head - rb->aux_wakeup >= rb->aux_watermark) {
perf_output_wakeup(handle);
- rb->aux_wakeup += rb->aux_watermark;
+ rb->aux_wakeup = rounddown(rb->aux_head, rb->aux_watermark);
handle->wakeup = rb->aux_wakeup + rb->aux_watermark;
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-08-25 11:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-16 16:18 [PATCH v2 1/2] perf/aux: Make aux_{head,wakeup} ring_buffer members long Will Deacon
2017-08-16 16:18 ` [PATCH v2 2/2] perf/aux: Ensure aux_wakeup represents most recent wakeup index Will Deacon
2017-08-25 11:52 ` [tip:perf/core] " tip-bot for Will Deacon
2017-08-25 11:52 ` [tip:perf/core] perf/aux: Make aux_{head,wakeup} ring_buffer members long tip-bot for Will Deacon
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).