public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org,
	"zhangwei(Jovi)" <jovi.zhangwei@huawei.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>
Subject: [ 66/73] tracing: Fix ftrace_dump()
Date: Thu,  9 May 2013 15:25:56 -0700	[thread overview]
Message-ID: <20130509222533.125643522@linuxfoundation.org> (raw)
In-Reply-To: <20130509222526.480204972@linuxfoundation.org>

3.9-stable review patch.  If anyone has any objections, please let me know.

------------------

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

commit 7fe70b579c9e3daba71635e31b6189394e7b79d3 upstream.

ftrace_dump() had a lot of issues. What ftrace_dump() does, is when
ftrace_dump_on_oops is set (via a kernel parameter or sysctl), it
will dump out the ftrace buffers to the console when either a oops,
panic, or a sysrq-z occurs.

This was written a long time ago when ftrace was fragile to recursion.
But it wasn't written well even for that.

There's a possible deadlock that can occur if a ftrace_dump() is happening
and an NMI triggers another dump. This is because it grabs a lock
before checking if the dump ran.

It also totally disables ftrace, and tracing for no good reasons.

As the ring_buffer now checks if it is read via a oops or NMI, where
there's a chance that the buffer gets corrupted, it will disable
itself. No need to have ftrace_dump() do the same.

ftrace_dump() is now cleaned up where it uses an atomic counter to
make sure only one dump happens at a time. A simple atomic_inc_return()
is enough that is needed for both other CPUs and NMIs. No need for
a spinlock, as if one CPU is running the dump, no other CPU needs
to do it too.

The tracing_on variable is turned off and not turned on. The original
code did this, but it wasn't pretty. By just disabling this variable
we get the result of not seeing traces that happen between crashes.

For sysrq-z, it doesn't get turned on, but the user can always write
a '1' to the tracing_on file. If they are using sysrq-z, then they should
know about tracing_on.

The new code is much easier to read and less error prone. No more
deadlock possibility when an NMI triggers here.

Reported-by: zhangwei(Jovi) <jovi.zhangwei@huawei.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 kernel/trace/trace.c          |   62 +++++++++++++++++-------------------------
 kernel/trace/trace_selftest.c |    9 +++---
 2 files changed, 31 insertions(+), 40 deletions(-)

--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -5168,36 +5168,32 @@ void trace_init_global_iter(struct trace
 	iter->cpu_file = TRACE_PIPE_ALL_CPU;
 }
 
-static void
-__ftrace_dump(bool disable_tracing, enum ftrace_dump_mode oops_dump_mode)
+void ftrace_dump(enum ftrace_dump_mode oops_dump_mode)
 {
-	static arch_spinlock_t ftrace_dump_lock =
-		(arch_spinlock_t)__ARCH_SPIN_LOCK_UNLOCKED;
 	/* use static because iter can be a bit big for the stack */
 	static struct trace_iterator iter;
+	static atomic_t dump_running;
 	unsigned int old_userobj;
-	static int dump_ran;
 	unsigned long flags;
 	int cnt = 0, cpu;
 
-	/* only one dump */
-	local_irq_save(flags);
-	arch_spin_lock(&ftrace_dump_lock);
-	if (dump_ran)
-		goto out;
-
-	dump_ran = 1;
+	/* Only allow one dump user at a time. */
+	if (atomic_inc_return(&dump_running) != 1) {
+		atomic_dec(&dump_running);
+		return;
+	}
 
+	/*
+	 * Always turn off tracing when we dump.
+	 * We don't need to show trace output of what happens
+	 * between multiple crashes.
+	 *
+	 * If the user does a sysrq-z, then they can re-enable
+	 * tracing with echo 1 > tracing_on.
+	 */
 	tracing_off();
 
-	/* Did function tracer already get disabled? */
-	if (ftrace_is_dead()) {
-		printk("# WARNING: FUNCTION TRACING IS CORRUPTED\n");
-		printk("#          MAY BE MISSING FUNCTION EVENTS\n");
-	}
-
-	if (disable_tracing)
-		ftrace_kill();
+	local_irq_save(flags);
 
 	/* Simulate the iterator */
 	trace_init_global_iter(&iter);
@@ -5227,6 +5223,12 @@ __ftrace_dump(bool disable_tracing, enum
 
 	printk(KERN_TRACE "Dumping ftrace buffer:\n");
 
+	/* Did function tracer already get disabled? */
+	if (ftrace_is_dead()) {
+		printk("# WARNING: FUNCTION TRACING IS CORRUPTED\n");
+		printk("#          MAY BE MISSING FUNCTION EVENTS\n");
+	}
+
 	/*
 	 * We need to stop all tracing on all CPUS to read the
 	 * the next buffer. This is a bit expensive, but is
@@ -5266,26 +5268,14 @@ __ftrace_dump(bool disable_tracing, enum
 		printk(KERN_TRACE "---------------------------------\n");
 
  out_enable:
-	/* Re-enable tracing if requested */
-	if (!disable_tracing) {
-		trace_flags |= old_userobj;
+	trace_flags |= old_userobj;
 
-		for_each_tracing_cpu(cpu) {
-			atomic_dec(&iter.tr->data[cpu]->disabled);
-		}
-		tracing_on();
+	for_each_tracing_cpu(cpu) {
+		atomic_dec(&iter.tr->data[cpu]->disabled);
 	}
-
- out:
-	arch_spin_unlock(&ftrace_dump_lock);
+ 	atomic_dec(&dump_running);
 	local_irq_restore(flags);
 }
-
-/* By default: disable tracing after the dump */
-void ftrace_dump(enum ftrace_dump_mode oops_dump_mode)
-{
-	__ftrace_dump(true, oops_dump_mode);
-}
 EXPORT_SYMBOL_GPL(ftrace_dump);
 
 __init static int tracer_alloc_buffers(void)
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -703,8 +703,6 @@ trace_selftest_startup_function(struct t
 /* Maximum number of functions to trace before diagnosing a hang */
 #define GRAPH_MAX_FUNC_TEST	100000000
 
-static void
-__ftrace_dump(bool disable_tracing, enum ftrace_dump_mode oops_dump_mode);
 static unsigned int graph_hang_thresh;
 
 /* Wrap the real function entry probe to avoid possible hanging */
@@ -714,8 +712,11 @@ static int trace_graph_entry_watchdog(st
 	if (unlikely(++graph_hang_thresh > GRAPH_MAX_FUNC_TEST)) {
 		ftrace_graph_stop();
 		printk(KERN_WARNING "BUG: Function graph tracer hang!\n");
-		if (ftrace_dump_on_oops)
-			__ftrace_dump(false, DUMP_ALL);
+		if (ftrace_dump_on_oops) {
+			ftrace_dump(DUMP_ALL);
+			/* ftrace_dump() disables tracing */
+			tracing_on();
+		}
 		return 0;
 	}
 



  parent reply	other threads:[~2013-05-09 22:26 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-09 22:24 [ 00/73] 3.9.2-stable review Greg Kroah-Hartman
2013-05-09 22:24 ` [ 01/73] xen/arm: actually pass a non-NULL percpu pointer to request_percpu_irq Greg Kroah-Hartman
2013-05-09 22:24 ` [ 02/73] powerpc: Emulate non privileged DSCR read and write Greg Kroah-Hartman
2013-05-09 22:24 ` [ 03/73] powerpc/tm: Fix null pointer deference in flush_hash_page Greg Kroah-Hartman
2013-05-09 22:24 ` [ 04/73] powerpc: fix numa distance for form0 device tree Greg Kroah-Hartman
2013-05-09 22:24 ` [ 05/73] pwm: spear: Fix checking return value of clk_enable() and clk_prepare() Greg Kroah-Hartman
2013-05-09 22:24 ` [ 06/73] autofs - remove autofs dentry mount check Greg Kroah-Hartman
2013-05-09 22:24 ` [ 07/73] rpmsg: fix kconfig dependencies for VIRTIO Greg Kroah-Hartman
2013-05-09 22:24 ` [ 08/73] remoteproc: " Greg Kroah-Hartman
2013-05-09 22:24 ` [ 09/73] hugetlbfs: fix mmap failure in unaligned size request Greg Kroah-Hartman
2013-05-09 22:25 ` [ 10/73] iommu/amd: Properly initialize irq-table lock Greg Kroah-Hartman
2013-05-09 22:25 ` [ 11/73] menuconfig: Fix memory leak introduced by jump keys feature Greg Kroah-Hartman
2013-05-09 22:25 ` [ 12/73] net/eth/ibmveth: Fixup retrieval of MAC address Greg Kroah-Hartman
2013-05-09 22:25 ` [ 13/73] perf/x86/intel: Add support for IvyBridge model 58 Uncore Greg Kroah-Hartman
2013-05-09 22:25 ` [ 14/73] perf/x86/intel: Fix unintended variable name reuse Greg Kroah-Hartman
2013-05-09 22:25 ` [ 15/73] perf/x86: Blacklist all MEM_*_RETIRED events for Ivy Bridge Greg Kroah-Hartman
2013-05-09 22:25 ` [ 16/73] perf/x86/intel/lbr: Fix LBR filter Greg Kroah-Hartman
2013-05-09 22:25 ` [ 17/73] perf/x86/intel/lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL Greg Kroah-Hartman
2013-05-09 22:25 ` [ 18/73] ipvs: ip_vs_sip_fill_param() BUG: bad check of return value Greg Kroah-Hartman
2013-05-09 22:25 ` [ 19/73] ext4: add check for inodes_count overflow in new resize ioctl Greg Kroah-Hartman
2013-05-09 22:25 ` [ 20/73] MODSIGN: do not send garbage to stderr when enabling modules signature Greg Kroah-Hartman
2013-05-09 22:25 ` [ 21/73] r8169: fix 8168evl frame padding Greg Kroah-Hartman
2013-05-09 22:25 ` [ 22/73] RDMA/cxgb4: Fix SQ allocation when on-chip SQ is disabled Greg Kroah-Hartman
2013-05-09 22:25 ` [ 23/73] arm64: Ignore the write ESR flag on cache maintenance faults Greg Kroah-Hartman
2013-05-09 22:25 ` [ 24/73] blkcg: fix "scheduling while atomic" in blk_queue_bypass_start Greg Kroah-Hartman
2013-05-09 22:25 ` [ 25/73] block: fix max discard sectors limit Greg Kroah-Hartman
2013-05-09 22:25 ` [ 26/73] drm/cirrus: deal with bo reserve fail in dirty update path Greg Kroah-Hartman
2013-05-09 22:25 ` [ 27/73] drm/mgag200: " Greg Kroah-Hartman
2013-05-09 22:25 ` [ 28/73] drm/gma500: fix backlight hotkeys behaviour on netbooks Greg Kroah-Hartman
2013-05-09 22:25 ` [ 29/73] drm: prime: fix refcounting on the dmabuf import error path Greg Kroah-Hartman
2013-05-09 22:25 ` [ 30/73] drm/prime: keep a reference from the handle to exported dma-buf (v6) Greg Kroah-Hartman
2013-05-09 22:25 ` [ 31/73] drm/ast: deal with bo reserve fail in dirty update path Greg Kroah-Hartman
2013-05-09 22:25 ` [ 32/73] drm/i915: Fix sdvo connector get_hw_state function Greg Kroah-Hartman
2013-05-09 22:25 ` [ 33/73] drm/i915: Add no-lvds quirk for Fujitsu Esprimo Q900 Greg Kroah-Hartman
2013-05-09 22:25 ` [ 34/73] drm/i915: Fix SDVO connector and encoder get_hw_state functions Greg Kroah-Hartman
2013-05-09 22:25 ` [ 35/73] drm/i915: Workaround incoherence between fences and LLC across multiple CPUs Greg Kroah-Hartman
2013-05-09 22:25 ` [ 36/73] drm/i915: Use MLC (l3$) for context objects Greg Kroah-Hartman
2013-05-09 22:25 ` [ 37/73] drm/i915: set CPT FDI RX polarity bits based on VBT Greg Kroah-Hartman
2013-05-09 22:25 ` [ 38/73] drm/i915: dont check inconsistent modeset state when force-restoring Greg Kroah-Hartman
2013-05-09 22:25 ` [ 39/73] drm/i915: ensure single initialization and cleanup of backlight device Greg Kroah-Hartman
2013-05-09 22:25 ` [ 40/73] drm/i915: dont intel_crt_init on any ULT machines Greg Kroah-Hartman
2013-05-09 22:25 ` [ 41/73] drm/i915: Fixup Oops in the pipe config computation Greg Kroah-Hartman
2013-05-09 22:25 ` [ 42/73] drm/i915: Fall back to bit banging mode for DVO transmitter detection Greg Kroah-Hartman
2013-05-09 22:25 ` [ 43/73] drm/i915: correct the calculation of first_pd_entry_in_global_pt Greg Kroah-Hartman
2013-05-09 22:25 ` [ 44/73] drm/radeon: dont use get_engine_clock() on APUs Greg Kroah-Hartman
2013-05-09 22:25 ` [ 45/73] drm/radeon: use frac fb div on RS780/RS880 Greg Kroah-Hartman
2013-05-09 22:25 ` [ 46/73] drm/radeon: fix typo in rv515_mc_resume() Greg Kroah-Hartman
2013-05-09 22:25 ` [ 47/73] drm/radeon/dce6: add missing display reg for tiling setup Greg Kroah-Hartman
2013-05-09 22:25 ` [ 48/73] drm/radeon: update wait_for_vblank for r5xx-r7xx Greg Kroah-Hartman
2013-05-09 22:25 ` [ 49/73] drm/radeon: update wait_for_vblank for evergreen+ Greg Kroah-Hartman
2013-05-09 22:25 ` [ 50/73] drm/radeon: properly lock disp in mc_stop/resume " Greg Kroah-Hartman
2013-05-09 22:25 ` [ 51/73] drm/radeon: properly lock disp in mc_stop/resume for r5xx-r7xx Greg Kroah-Hartman
2013-05-09 22:25 ` [ 52/73] drm/radeon: update wait_for_vblank for r1xx-r4xx Greg Kroah-Hartman
2013-05-09 22:25 ` [ 53/73] drm/radeon: disable the crtcs in mc_stop (evergreen+) (v2) Greg Kroah-Hartman
2013-05-09 22:25 ` [ 54/73] drm/radeon: add some new SI PCI ids Greg Kroah-Hartman
2013-05-09 22:25 ` [ 55/73] drm/radeon/evergreen+: dont enable HPD interrupts on eDP/LVDS Greg Kroah-Hartman
2013-05-09 22:25 ` [ 56/73] drm/radeon: cleanup properly if mmio mapping fails Greg Kroah-Hartman
2013-05-09 22:25 ` [ 57/73] drm/radeon: fix hdmi mode enable on RS600/RS690/RS740 Greg Kroah-Hartman
2013-05-09 22:25 ` [ 58/73] drm/radeon: fix typo in si_select_se_sh() Greg Kroah-Hartman
2013-05-09 22:25 ` [ 59/73] drm/radeon: Always flush the VM Greg Kroah-Hartman
2013-05-09 22:25 ` [ 60/73] drm/radeon: disable the crtcs in mc_stop (r5xx-r7xx) (v2) Greg Kroah-Hartman
2013-05-09 22:25 ` [ 61/73] drm/radeon: fix endian bugs in atom_allocate_fb_scratch() Greg Kroah-Hartman
2013-05-09 22:25 ` [ 62/73] drm/radeon: fix possible segfault when parsing pm tables Greg Kroah-Hartman
2013-05-09 22:25 ` [ 63/73] drm/radeon: add new richland pci ids Greg Kroah-Hartman
2013-05-09 22:25 ` [ 64/73] drm/radeon: fix handling of v6 power tables Greg Kroah-Hartman
2013-05-09 22:25 ` [ 65/73] drm/tilcdc: Fix an incorrect condition Greg Kroah-Hartman
2013-05-09 22:25 ` Greg Kroah-Hartman [this message]
2013-05-09 22:25 ` [ 67/73] Btrfs: compare relevant parts of delayed tree refs Greg Kroah-Hartman
2013-05-09 22:25 ` [ 68/73] Btrfs: fix extent logging with O_DIRECT into prealloc Greg Kroah-Hartman
2013-05-09 22:25 ` [ 69/73] EDAC: Dont give write permission to read-only files Greg Kroah-Hartman
2013-05-09 22:26 ` [ 70/73] PCI: Delay final fixups until resources are assigned Greg Kroah-Hartman
2013-05-09 22:26 ` [ 71/73] qmi_wwan/cdc_ether: add device IDs for Dell 5804 (Novatel E371) WWAN card Greg Kroah-Hartman
2013-05-09 22:26 ` [ 72/73] NFSv4.x: Fix handling of partially delegated locks Greg Kroah-Hartman
2013-05-09 22:26 ` [ 73/73] kernel/audit_tree.c: tree will leak memory when failure occurs in audit_trim_trees() Greg Kroah-Hartman
2013-05-10 15:23 ` [ 00/73] 3.9.2-stable review Shuah Khan
2013-05-10 15:28   ` Greg Kroah-Hartman
2013-05-11  5:26 ` Satoru Takeuchi
2013-05-11 13:53   ` Greg Kroah-Hartman

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=20130509222533.125643522@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=fweisbec@gmail.com \
    --cc=jovi.zhangwei@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=stable@vger.kernel.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