From: Peter Zijlstra <a.p.zijlstra@chello.nl>
To: Ingo Molnar <mingo@elte.hu>, Paul Mackerras <paulus@samba.org>,
Arnaldo Carvalho de Melo <acme@infradead.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>,
Steven Rostedt <rostedt@goodmis.org>,
Thomas Gleixner <tglx@linutronix.de>,
linux-kernel@vger.kernel.org,
Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: [PATCH 2/5] perf: Remove IRQ-disable from the perf_output path
Date: Tue, 18 May 2010 15:33:00 +0200 [thread overview]
Message-ID: <20100518133725.963596631@chello.nl> (raw)
In-Reply-To: <20100518133258.000434886@chello.nl>
[-- Attachment #1: perf-buffer-no-irq.patch --]
[-- Type: text/plain, Size: 6408 bytes --]
Since we can now assume there is only a single writer to each buffer,
we can remove per-cpu lock thingy and use a simply nest-count to the
same effect.
This removes the need to disable IRQs.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
include/linux/perf_event.h | 5 --
kernel/perf_event.c | 94 +++++++++++++--------------------------------
2 files changed, 30 insertions(+), 69 deletions(-)
Index: linux-2.6/include/linux/perf_event.h
===================================================================
--- linux-2.6.orig/include/linux/perf_event.h
+++ linux-2.6/include/linux/perf_event.h
@@ -597,12 +597,12 @@ struct perf_mmap_data {
atomic_t events; /* event_id limit */
atomic_long_t head; /* write position */
- atomic_long_t done_head; /* completed head */
- atomic_t lock; /* concurrent writes */
atomic_t wakeup; /* needs a wakeup */
atomic_t lost; /* nr records lost */
+ atomic_t nest; /* nested writers */
+
long watermark; /* wakeup watermark */
struct perf_event_mmap_page *user_page;
@@ -807,7 +807,6 @@ struct perf_output_handle {
unsigned long offset;
int nmi;
int sample;
- int locked;
};
#ifdef CONFIG_PERF_EVENTS
Index: linux-2.6/kernel/perf_event.c
===================================================================
--- linux-2.6.orig/kernel/perf_event.c
+++ linux-2.6/kernel/perf_event.c
@@ -2519,8 +2519,6 @@ perf_mmap_data_init(struct perf_event *e
{
long max_size = perf_data_size(data);
- atomic_set(&data->lock, -1);
-
if (event->attr.watermark) {
data->watermark = min_t(long, max_size,
event->attr.wakeup_watermark);
@@ -2906,82 +2904,56 @@ static void perf_output_wakeup(struct pe
}
/*
- * Curious locking construct.
- *
* We need to ensure a later event_id doesn't publish a head when a former
- * event_id isn't done writing. However since we need to deal with NMIs we
+ * event isn't done writing. However since we need to deal with NMIs we
* cannot fully serialize things.
*
- * What we do is serialize between CPUs so we only have to deal with NMI
- * nesting on a single CPU.
- *
* We only publish the head (and generate a wakeup) when the outer-most
- * event_id completes.
+ * event completes.
*/
-static void perf_output_lock(struct perf_output_handle *handle)
+static void perf_output_get_handle(struct perf_output_handle *handle)
{
struct perf_mmap_data *data = handle->data;
- int cur, cpu = get_cpu();
-
- handle->locked = 0;
-
- for (;;) {
- cur = atomic_cmpxchg(&data->lock, -1, cpu);
- if (cur == -1) {
- handle->locked = 1;
- break;
- }
- if (cur == cpu)
- break;
- cpu_relax();
- }
+ preempt_disable();
+ atomic_inc(&data->nest);
}
-static void perf_output_unlock(struct perf_output_handle *handle)
+static void perf_output_put_handle(struct perf_output_handle *handle)
{
struct perf_mmap_data *data = handle->data;
unsigned long head;
- int cpu;
-
- data->done_head = data->head;
-
- if (!handle->locked)
- goto out;
again:
- /*
- * The xchg implies a full barrier that ensures all writes are done
- * before we publish the new head, matched by a rmb() in userspace when
- * reading this position.
- */
- while ((head = atomic_long_xchg(&data->done_head, 0)))
- data->user_page->data_head = head;
+ head = atomic_long_read(&data->head);
/*
- * NMI can happen here, which means we can miss a done_head update.
+ * IRQ/NMI can happen here, which means we can miss a head update.
*/
- cpu = atomic_xchg(&data->lock, -1);
- WARN_ON_ONCE(cpu != smp_processor_id());
+ if (!atomic_dec_and_test(&data->nest))
+ return;
/*
- * Therefore we have to validate we did not indeed do so.
+ * Publish the known good head. Rely on the full barrier implied
+ * by atomic_dec_and_test() order the data->head read and this
+ * write.
*/
- if (unlikely(atomic_long_read(&data->done_head))) {
- /*
- * Since we had it locked, we can lock it again.
- */
- while (atomic_cmpxchg(&data->lock, -1, cpu) != -1)
- cpu_relax();
+ data->user_page->data_head = head;
+ /*
+ * Now check if we missed an update, rely on the (compiler)
+ * barrier in atomic_dec_and_test() to re-read data->head.
+ */
+ if (unlikely(head != atomic_long_read(&data->head))) {
+ atomic_inc(&data->nest);
goto again;
}
if (atomic_xchg(&data->wakeup, 0))
perf_output_wakeup(handle);
-out:
- put_cpu();
+
+ preempt_enable();
}
void perf_output_copy(struct perf_output_handle *handle,
@@ -3063,7 +3035,7 @@ int perf_output_begin(struct perf_output
if (have_lost)
size += sizeof(lost_event);
- perf_output_lock(handle);
+ perf_output_get_handle(handle);
do {
/*
@@ -3083,7 +3055,7 @@ int perf_output_begin(struct perf_output
handle->head = head;
if (head - tail > data->watermark)
- atomic_set(&data->wakeup, 1);
+ atomic_inc(&data->wakeup);
if (have_lost) {
lost_event.header.type = PERF_RECORD_LOST;
@@ -3099,7 +3071,7 @@ int perf_output_begin(struct perf_output
fail:
atomic_inc(&data->lost);
- perf_output_unlock(handle);
+ perf_output_put_handle(handle);
out:
rcu_read_unlock();
@@ -3117,11 +3089,11 @@ void perf_output_end(struct perf_output_
int events = atomic_inc_return(&data->events);
if (events >= wakeup_events) {
atomic_sub(wakeup_events, &data->events);
- atomic_set(&data->wakeup, 1);
+ atomic_inc(&data->wakeup);
}
}
- perf_output_unlock(handle);
+ perf_output_put_handle(handle);
rcu_read_unlock();
}
@@ -3457,22 +3429,13 @@ static void perf_event_task_output(struc
{
struct perf_output_handle handle;
struct task_struct *task = task_event->task;
- unsigned long flags;
int size, ret;
- /*
- * If this CPU attempts to acquire an rq lock held by a CPU spinning
- * in perf_output_lock() from interrupt context, it's game over.
- */
- local_irq_save(flags);
-
size = task_event->event_id.header.size;
ret = perf_output_begin(&handle, event, size, 0, 0);
- if (ret) {
- local_irq_restore(flags);
+ if (ret)
return;
- }
task_event->event_id.pid = perf_event_pid(event, task);
task_event->event_id.ppid = perf_event_pid(event, current);
@@ -3483,7 +3446,6 @@ static void perf_event_task_output(struc
perf_output_put(&handle, task_event->event_id);
perf_output_end(&handle);
- local_irq_restore(flags);
}
static int perf_event_task_match(struct perf_event *event)
next prev parent reply other threads:[~2010-05-18 13:46 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-18 13:32 [PATCH 0/5] Optimize perf ring-buffer Peter Zijlstra
2010-05-18 13:32 ` [PATCH 1/5] perf: Disallow mmap() on per-task inherited events Peter Zijlstra
2010-05-19 7:19 ` Frederic Weisbecker
2010-05-25 0:55 ` Paul Mackerras
2010-05-25 8:19 ` Peter Zijlstra
2010-05-18 13:33 ` Peter Zijlstra [this message]
2010-05-18 13:33 ` [PATCH 3/5] perf: Convert the perf output buffer to local_t Peter Zijlstra
2010-05-18 13:33 ` [PATCH 4/5] perf: Avoid local_xchg Peter Zijlstra
2010-05-18 13:33 ` [RFC PATCH 5/5] perf: Implement perf_output_addr() Peter Zijlstra
2010-05-18 14:09 ` Peter Zijlstra
2010-05-19 7:21 ` Frederic Weisbecker
2010-05-19 7:58 ` Peter Zijlstra
2010-05-19 9:03 ` Frederic Weisbecker
2010-05-19 14:47 ` Steven Rostedt
2010-05-19 15:05 ` Peter Zijlstra
2010-05-19 15:38 ` Steven Rostedt
2010-05-19 15:50 ` Peter Zijlstra
2010-05-19 16:08 ` Steven Rostedt
2010-05-19 16:15 ` Peter Zijlstra
2010-05-19 16:27 ` Steven Rostedt
2010-05-19 16:34 ` Peter Zijlstra
2010-05-19 7:14 ` [PATCH 0/5] Optimize perf ring-buffer Frederic Weisbecker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100518133725.963596631@chello.nl \
--to=a.p.zijlstra@chello.nl \
--cc=acme@infradead.org \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=paulus@samba.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome