linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch 1/2] dump_stack: serialize the output from dump_stack()
@ 2013-05-08 21:01 athorlton
  2013-05-08 22:41 ` Andrew Morton
  0 siblings, 1 reply; 2+ messages in thread
From: athorlton @ 2013-05-08 21:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Alex Thorlton, Vineet Gupta, Andrew Morton, David S. Miller,
	Richard Kuo, Jesper Nilsson, Robin Holt

These patches fix up issues with interspersed output from multiple
simultaneous calls to warn or dump_stack on multi-cpu systems.
References: <20130508210102.898396979@asylum.americas.sgi.com>
Content-Disposition: inline; filename=dump-stack-serialize.patch

This patch adds functionality to serialize the output from dump_stack() to 
avoid mangling of the output when dump_stack is called simultaneously from
multiple cpus.

Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Richard Kuo <rkuo@codeaurora.org>
Cc: Jesper Nilsson <jesper.nilsson@axis.com>
Reported-by: Russ Anderson <rja@sgi.com>
Reviewed-by: Robin Holt <holt@sgi.com>
Signed-off-by: Alex Thorlton <athorlton@sgi.com>
---
The original discussion regarding this patch can be found in this thread:
[PATCH] x86: Avoid intermixing cpu dump_stack output on multi-processor systems

 lib/dump_stack.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

--- linux.orig/lib/dump_stack.c
+++ linux/lib/dump_stack.c
@@ -7,6 +7,8 @@
 #include <linux/export.h>
 #include <linux/sched.h>
 
+static atomic_t dump_lock = ATOMIC_INIT(-1);
+
 /**
  * dump_stack - dump the current task information and its stack trace
  *
@@ -14,7 +16,30 @@
  */
 void dump_stack(void)
 {
+	int was_locked;
+	int old;
+	int cpu;
+
+	preempt_disable();
+
+retry:
+	cpu = smp_processor_id();
+	old = atomic_cmpxchg(&dump_lock, -1, cpu);
+	if (old == -1) {
+		was_locked = 0;
+	} else if (old == cpu) {
+		was_locked = 1;
+	} else {
+		cpu_relax();
+		goto retry;
+	}
+
 	dump_stack_print_info(KERN_DEFAULT);
 	show_stack(NULL, NULL);
+
+	if (!was_locked)
+		atomic_set(&dump_lock, -1);
+
+	preempt_enable();
 }
 EXPORT_SYMBOL(dump_stack);


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

* Re: [patch 1/2] dump_stack: serialize the output from dump_stack()
  2013-05-08 21:01 [patch 1/2] dump_stack: serialize the output from dump_stack() athorlton
@ 2013-05-08 22:41 ` Andrew Morton
  0 siblings, 0 replies; 2+ messages in thread
From: Andrew Morton @ 2013-05-08 22:41 UTC (permalink / raw)
  To: athorlton
  Cc: linux-kernel, Vineet Gupta, David S. Miller, Richard Kuo,
	Jesper Nilsson, Robin Holt

On Wed, 08 May 2013 16:01:03 -0500 athorlton@sgi.com wrote:

> These patches fix up issues with interspersed output from multiple
> simultaneous calls to warn or dump_stack on multi-cpu systems.
> References: <20130508210102.898396979@asylum.americas.sgi.com>
> Content-Disposition: inline; filename=dump-stack-serialize.patch
> 
> This patch adds functionality to serialize the output from dump_stack() to 
> avoid mangling of the output when dump_stack is called simultaneously from
> multiple cpus.
> 
> ...
>
> The original discussion regarding this patch can be found in this thread:
> [PATCH] x86: Avoid intermixing cpu dump_stack output on multi-processor systems

If there was anything useful or interesting in that discussion then it
should be included in this patch's changelog, please.  Don't send
everyone off hunting for emails.  Email to which they can't reply in
the context of this thread...

> +++ linux/lib/dump_stack.c
> @@ -7,6 +7,8 @@
>  #include <linux/export.h>
>  #include <linux/sched.h>
>  
> +static atomic_t dump_lock = ATOMIC_INIT(-1);
> +
>  /**
>   * dump_stack - dump the current task information and its stack trace
>   *
> @@ -14,7 +16,30 @@
>   */
>  void dump_stack(void)
>  {
> +	int was_locked;
> +	int old;
> +	int cpu;
> +
> +	preempt_disable();
> +
> +retry:
> +	cpu = smp_processor_id();
> +	old = atomic_cmpxchg(&dump_lock, -1, cpu);
> +	if (old == -1) {
> +		was_locked = 0;
> +	} else if (old == cpu) {
> +		was_locked = 1;
> +	} else {
> +		cpu_relax();
> +		goto retry;
> +	}
> +
>  	dump_stack_print_info(KERN_DEFAULT);
>  	show_stack(NULL, NULL);
> +
> +	if (!was_locked)
> +		atomic_set(&dump_lock, -1);
> +
> +	preempt_enable();

This would benefit from a comment explaining what it's doing and why. 
"Permit this cpu to perform nested stack dumps while serialising
against other CPUs".

The patch adds a load of goop which is unneeded on uniprocessor
kernels.  I guess a non-messy way of avoiding that is to just have two
versions of dump_stack() in this file.

It would be prudent to toss some more #includes in there.  For
atomic_foo() and cpu_relax(), for example.


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

end of thread, other threads:[~2013-05-08 22:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-08 21:01 [patch 1/2] dump_stack: serialize the output from dump_stack() athorlton
2013-05-08 22:41 ` Andrew Morton

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