mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.cz>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@elte.hu>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Jiri Kosina <jkosina@suse.cz>,
	linux-kernel@vger.kernel.org, Petr Mladek <pmladek@suse.cz>
Subject: [PATCH v4 3/3] trace: Allow to call update_max_tr_single() from another CPU
Date: Tue, 29 Jul 2014 14:31:43 +0200	[thread overview]
Message-ID: <1406637103-15435-4-git-send-email-pmladek@suse.cz> (raw)
In-Reply-To: <1406637103-15435-1-git-send-email-pmladek@suse.cz>

Any CPU specific tracing buffer could get switched by

    echo 1 >/sys/kernel/debug/tracing/per_cpu/cpuN/snapshot

It triggers the following sequence of calls:

   + tracing_snapshot_write()
     + update_max_tr_single()
       + ring_buffer_swap_cpu()

We need to call ring_buffer_swap_cpu() with disabled interrupts because
it is done under "tr->max_lock" and it would cause races otherwise.
It also means that we need to call it from the right CPU but it is
not guaranteed by tracing_snapshot_write().

This patch solves the problem by renaming update_max_tr_single()
to update_max_tr_this_cpu(). It works with the current CPU instead
of the passed one. It can be called from check_critical_timing()
directly.

Also it introduces function with the original name update_max_tr_single()
that calls the CPU-specific one via smp_call_function_single().
It can be called from tracing_snapshot_write() but with enabled interrupts.
Otherwise, smp_call_function_single() would not work.

Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
 kernel/trace/trace.c         | 60 ++++++++++++++++++++++++++++++++++++++------
 kernel/trace/trace.h         |  1 +
 kernel/trace/trace_irqsoff.c |  2 +-
 3 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 4caa814d41c3..a2dc83b98cc0 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1017,22 +1017,29 @@ update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu)
 }
 
 /**
- * update_max_tr_single - only copy one trace over, and reset the rest
+ * update_max_tr_this_cpu_single - copy trace for this CPU, and reset the rest
  * @tr - tracer
  * @tsk - task with the latency
- * @cpu - the cpu of the buffer to copy.
  *
- * Flip the trace of a single CPU buffer between the @tr and the max_tr.
+ * Flip the trace of this CPU buffer between the @tr and the max_tr.
+ *
+ * It is called on this CPU. Otherwise, ring_buffer_swap_cpu() would not work
+ * with disabled interrupts. They must be disabled to avoid a race under
+ * tr->max_lock and because this is used in irqsoff tracer that is sensitive
+ * about it.
  */
 void
-update_max_tr_single(struct trace_array *tr, struct task_struct *tsk, int cpu)
+update_max_tr_this_cpu(struct trace_array *tr, struct task_struct *tsk)
 {
+	int cpu;
 	int ret;
 
 	if (tr->stop_count)
 		return;
 
 	WARN_ON_ONCE(!irqs_disabled());
+	cpu = smp_processor_id();
+
 	if (!tr->allocated_snapshot) {
 		/* Only the nop tracer should hit this when disabling */
 		WARN_ON_ONCE(tr->current_trace != &nop_trace);
@@ -1059,6 +1066,42 @@ update_max_tr_single(struct trace_array *tr, struct task_struct *tsk, int cpu)
 	__update_max_tr(tr, tsk, cpu);
 	arch_spin_unlock(&tr->max_lock);
 }
+
+struct update_max_tr_info {
+	struct trace_array *tr;
+	struct task_struct *tsk;
+};
+
+static void update_max_tr_function_single(void *arg)
+{
+	struct update_max_tr_info *tr_info = arg;
+	struct trace_array *tr = tr_info->tr;
+	struct task_struct *tsk = tr_info->tsk;
+
+	update_max_tr_this_cpu(tr, tsk);
+}
+
+/**
+ * update_max_tr_this_cpu_single - only copy one trace over, and reset the rest
+ * @tr - tracer
+ * @tsk - task with the latency
+ * @cpu - the cpu of the buffer to copy.
+ *
+ * Flip the trace of a single CPU buffer between the @tr and the max_tr.
+ */
+void
+update_max_tr_single(struct trace_array *tr, struct task_struct *tsk, int cpu)
+{
+	struct update_max_tr_info tr_info = {tr, tsk};
+
+
+	/*
+	 * The flip has to be done on the same CPU, see
+	 * update_max_tr_this_cpu() for more details.
+	 */
+	smp_call_function_single(cpu, update_max_tr_function_single,
+				 (void *)&tr_info, 1);
+}
 #endif /* CONFIG_TRACER_MAX_TRACE */
 
 static int wait_on_pipe(struct trace_iterator *iter)
@@ -5057,13 +5100,14 @@ tracing_snapshot_write(struct file *filp, const char __user *ubuf, size_t cnt,
 			if (ret < 0)
 				break;
 		}
-		local_irq_disable();
 		/* Now, we're going to swap */
-		if (iter->cpu_file == RING_BUFFER_ALL_CPUS)
+		if (iter->cpu_file == RING_BUFFER_ALL_CPUS) {
+			local_irq_disable();
 			update_max_tr(tr, current, smp_processor_id());
-		else
+			local_irq_enable();
+		} else {
 			update_max_tr_single(tr, current, iter->cpu_file);
-		local_irq_enable();
+		}
 		break;
 	default:
 		if (tr->allocated_snapshot) {
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 9258f5a815db..5aa876b9190e 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -616,6 +616,7 @@ extern unsigned long tracing_thresh;
 void update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu);
 void update_max_tr_single(struct trace_array *tr,
 			  struct task_struct *tsk, int cpu);
+void update_max_tr_this_cpu(struct trace_array *tr, struct task_struct *tsk);
 #endif /* CONFIG_TRACER_MAX_TRACE */
 
 #ifdef CONFIG_STACKTRACE
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index 1d4eeb304583..90a28d5105e4 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -347,7 +347,7 @@ check_critical_timing(struct trace_array *tr,
 
 	if (likely(!is_tracing_stopped())) {
 		tr->max_latency = delta;
-		update_max_tr_single(tr, current, cpu);
+		update_max_tr_this_cpu(tr, current);
 	}
 
 	max_sequence++;
-- 
1.8.4


      parent reply	other threads:[~2014-07-29 12:33 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-29 12:31 [PATCH v4 0/3] ring-buffer/ftrace: Race when writing and swapping cpu buffer in parallel Petr Mladek
2014-07-29 12:31 ` [PATCH v4 1/3] ring-buffer: " Petr Mladek
2014-07-29 12:31 ` [PATCH v4 2/3] trace: process entire stop_critical_timing() on the same CPU Petr Mladek
2014-07-29 12:31 ` Petr Mladek [this message]

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=1406637103-15435-4-git-send-email-pmladek@suse.cz \
    --to=pmladek@suse.cz \
    --cc=fweisbec@gmail.com \
    --cc=jkosina@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=rostedt@goodmis.org \
    /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

all inboxes | Powered by JetHome®