lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Wed, 8 May 2013 15:41:35 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	athorlton@....com
Cc:	linux-kernel@...r.kernel.org, Vineet Gupta <vgupta@...opsys.com>,
	"David S. Miller" <davem@...emloft.net>,
	Richard Kuo <rkuo@...eaurora.org>,
	Jesper Nilsson <jesper.nilsson@...s.com>,
	Robin Holt <holt@....com>
Subject: Re: [patch 1/2] dump_stack: serialize the output from dump_stack()

On Wed, 08 May 2013 16:01:03 -0500 athorlton@....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@...lum.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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ