linux-kernel - Re: [RFC: PATCH v2] lockdep: Make MAX_STACK_TRACE

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] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 20 Apr 2010 14:09:41 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	John Kacur <jkacur@...il.com>
Cc:	John Kacur <jkacur@...hat.com>, linux-kernel@...r.kernel.org,
	Ingo Molnar <mingo@...e.hu>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	linux-rt-users@...r.kernel.org,
	Thomas Gleixner <tglx@...utronix.de>,
	Clark Williams <williams@...hat.com>,
	"Luis Claudio R. Goncalves" <lgoncalv@...hat.com>
Subject: Re: [RFC: PATCH v2] lockdep: Make MAX_STACK_TRACE_ENTRIES
 configurable.

On Mon, 19 Apr 2010 18:51:57 +0200 (CEST)
John Kacur <jkacur@...il.com> wrote:

> Ingo, since nobody responded to my RFC, I am assuming that the change
> is not controversial, would you please consider pulling this into tip
> for 2.6.35?
> 
> What followed is v2 of the patch regenerated against the latest
> tip/master
> 
> ...
> 
> Certain configurations that have LOCKDEP turned on, run into the limit
> where the MAX_STACK_TRACE_ENTRIES are too small. Rather than simply
> turning of the locking correctness validator let the user configure this
> value to something reasonable for their system.
> 
> Signed-off-by: John Kacur <jkacur@...hat.com>
> ---
>  kernel/lockdep.c           |    8 ++++----
>  kernel/lockdep_internals.h |    6 ------
>  kernel/lockdep_proc.c      |    2 +-
>  lib/Kconfig.debug          |    9 +++++++++
>  4 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/kernel/lockdep.c b/kernel/lockdep.c
> index 78325f8..2acc25d 100644
> --- a/kernel/lockdep.c
> +++ b/kernel/lockdep.c
> @@ -369,12 +369,12 @@ static int verbose(struct lock_class *class)
>   * addresses. Protected by the graph_lock.
>   */
>  unsigned long nr_stack_trace_entries;
> -static unsigned long stack_trace[MAX_STACK_TRACE_ENTRIES];
> +static unsigned long stack_trace[CONFIG_MAX_STACK_TRACE_ENTRIES];
>  
>  static int save_trace(struct stack_trace *trace)
>  {
>  	trace->nr_entries = 0;
> -	trace->max_entries = MAX_STACK_TRACE_ENTRIES - nr_stack_trace_entries;
> +	trace->max_entries = CONFIG_MAX_STACK_TRACE_ENTRIES - nr_stack_trace_entries;
>  	trace->entries = stack_trace + nr_stack_trace_entries;
>  
>  	trace->skip = 3;
> @@ -396,11 +396,11 @@ static int save_trace(struct stack_trace *trace)
>  
>  	nr_stack_trace_entries += trace->nr_entries;
>  
> -	if (nr_stack_trace_entries >= MAX_STACK_TRACE_ENTRIES-1) {
> +	if (nr_stack_trace_entries >= CONFIG_MAX_STACK_TRACE_ENTRIES-1) {
>  		if (!debug_locks_off_graph_unlock())
>  			return 0;
>  
> -		printk("BUG: MAX_STACK_TRACE_ENTRIES too low!\n");
> +		printk("BUG: CONFIG_MAX_STACK_TRACE_ENTRIES = %d too low!\n", CONFIG_MAX_STACK_TRACE_ENTRIES);
>  		printk("turning off the locking correctness validator.\n");
>  		dump_stack();
>  
> diff --git a/kernel/lockdep_internals.h b/kernel/lockdep_internals.h
> index 8d7d4b6..e2585ff 100644
> --- a/kernel/lockdep_internals.h
> +++ b/kernel/lockdep_internals.h
> @@ -61,12 +61,6 @@ enum {
>  
>  #define MAX_LOCKDEP_CHAIN_HLOCKS (MAX_LOCKDEP_CHAINS*5)
>  
> -/*
> - * Stack-trace: tightly packed array of stack backtrace
> - * addresses. Protected by the hash_lock.
> - */
> -#define MAX_STACK_TRACE_ENTRIES	262144UL
> -
>  extern struct list_head all_lock_classes;
>  extern struct lock_chain lock_chains[];
>  
> diff --git a/kernel/lockdep_proc.c b/kernel/lockdep_proc.c
> index 59b76c8..924e0e9 100644
> --- a/kernel/lockdep_proc.c
> +++ b/kernel/lockdep_proc.c
> @@ -306,7 +306,7 @@ static int lockdep_stats_show(struct seq_file *m, void *v)
>  	seq_printf(m, " in-process chains:             %11u\n",
>  			nr_process_chains);
>  	seq_printf(m, " stack-trace entries:           %11lu [max: %lu]\n",
> -			nr_stack_trace_entries, MAX_STACK_TRACE_ENTRIES);
> +			nr_stack_trace_entries, CONFIG_MAX_STACK_TRACE_ENTRIES);
>  	seq_printf(m, " combined max dependencies:     %11u\n",
>  			(nr_hardirq_chains + 1) *
>  			(nr_softirq_chains + 1) *

Note that MAX_STACK_TRACE_ENTRIES was `unsigned long', but
CONFIG_MAX_STACK_TRACE_ENTRIES is now an `int'.  AFACIT all the
comparisons will handle that OK, but please review them for this.

But this seq_printf() is now wrong, isn't it?  Didn't it generate a
long-vs-int compiler warning?

<tests it>

yup:

kernel/lockdep_proc.c: In function 'lockdep_stats_show':
kernel/lockdep_proc.c:309: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'int'

> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 0bbd5c7..38d3bf3 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -533,6 +533,15 @@ config LOCKDEP
>  	select KALLSYMS
>  	select KALLSYMS_ALL
>  
> +config MAX_STACK_TRACE_ENTRIES
> +	int "MAX_STACK_TRACE_ENTRIES for LOCKDEP"
> +	depends on LOCKDEP
> +	default 262144
> +	help
> +	   This option allows you to change the default MAX_STACK_TRACE_ENTRIES
> +	   used for LOCKDEP. Warning, increasing this number will increase the
> +	   size of the stack_trace array, and thus the kernel size too.
> +
>  config LOCK_STAT
>  	bool "Lock usage statistics"
>  	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
> -- 
> 1.6.6.1
--
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/