* [RFC: PATCH] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. @ 2010-04-16 11:45 John Kacur 2010-04-16 15:29 ` [RFC: PATCH v2] " John Kacur 0 siblings, 1 reply; 11+ messages in thread From: John Kacur @ 2010-04-16 11:45 UTC (permalink / raw) To: linux-kernel, Ingo Molnar, Peter Zijlstra Cc: linux-rt-users, Thomas Gleixner, Clark Williams, Luis Claudio R. Goncalves, Andrew Morton This patch was created against v2.6.33.2-rt13 but is also intended for mainline. Certain debug configurations tend to go over the MAX_STACK_TRACE_ENTRIES limit without there really being any problem. This patch keeps the old default, but allows the user to configure a higher value if desired. I tend to bump this value up for real-time debug configurations for example. This is preferrable to indiscriminately turning off the locking correctness validator. There have been some attempts to increase the default value in the past, that were met with resistance by some, because of the legitimate concern that this was growing too large and that we need to understand why. By making it configurable, I hope to satisfy both sets of people - those who believe they need to set it larger under special circumstances, and those who want a reasonably small default. >From 57479e7620d312353f5f5b173742e011896a9c74 Mon Sep 17 00:00:00 2001 From: John Kacur <jkacur@redhat.com> Date: Fri, 16 Apr 2010 13:24:02 +0200 Subject: [RFC: PATCH] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. 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@redhat.com> --- kernel/lockdep.c | 8 ++++---- kernel/lockdep_internals.h | 6 ------ lib/Kconfig.debug | 9 +++++++++ 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/kernel/lockdep.c b/kernel/lockdep.c index 1199bda..6030521 100644 --- a/kernel/lockdep.c +++ b/kernel/lockdep.c @@ -368,12 +368,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; @@ -395,11 +395,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 a2ee95a..6887711 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/lib/Kconfig.debug b/lib/Kconfig.debug index cbf6e02..ad35402 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -509,6 +509,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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC: PATCH v2] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. 2010-04-16 11:45 [RFC: PATCH] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable John Kacur @ 2010-04-16 15:29 ` John Kacur 2010-04-19 16:51 ` John Kacur 0 siblings, 1 reply; 11+ messages in thread From: John Kacur @ 2010-04-16 15:29 UTC (permalink / raw) To: linux-kernel, Ingo Molnar, Peter Zijlstra Cc: linux-rt-users, Thomas Gleixner, Clark Williams, Luis Claudio R. Goncalves, Andrew Morton On Fri, 16 Apr 2010, John Kacur wrote: > > This patch was created against v2.6.33.2-rt13 but is also intended for > mainline. > > Certain debug configurations tend to go over the MAX_STACK_TRACE_ENTRIES > limit without there really being any problem. > > This patch keeps the old default, but allows the user to configure a > higher value if desired. > > I tend to bump this value up for real-time debug configurations for > example. This is preferrable to indiscriminately turning off the locking > correctness validator. > > There have been some attempts to increase the default value in the past, > that were met with resistance by some, because of the legitimate concern > that this was growing too large and that we need to understand why. By > making it configurable, I hope to satisfy both sets of people - those who > believe they need to set it larger under special circumstances, and those > who want a reasonably small default. > > From 57479e7620d312353f5f5b173742e011896a9c74 Mon Sep 17 00:00:00 2001 > From: John Kacur <jkacur@redhat.com> > Date: Fri, 16 Apr 2010 13:24:02 +0200 > Subject: [RFC: PATCH] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. > > 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@redhat.com> > --- > kernel/lockdep.c | 8 ++++---- > kernel/lockdep_internals.h | 6 ------ > lib/Kconfig.debug | 9 +++++++++ > 3 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/kernel/lockdep.c b/kernel/lockdep.c > index 1199bda..6030521 100644 > --- a/kernel/lockdep.c > +++ b/kernel/lockdep.c > @@ -368,12 +368,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; > @@ -395,11 +395,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 a2ee95a..6887711 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/lib/Kconfig.debug b/lib/Kconfig.debug > index cbf6e02..ad35402 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -509,6 +509,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 > > I missed the MAX_STACK_TRACE_ENTRIES in kernel/lockdep_proc.c This corrects that. >From 66b4fa66afefb9e22047ddc5a68c9886e1a59cf6 Mon Sep 17 00:00:00 2001 From: John Kacur <jkacur@redhat.com> Date: Fri, 16 Apr 2010 13:24:02 +0200 Subject: [PATCH] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. 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@redhat.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 1199bda..6030521 100644 --- a/kernel/lockdep.c +++ b/kernel/lockdep.c @@ -368,12 +368,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; @@ -395,11 +395,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 a2ee95a..6887711 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 d4aba4f..97fcb31 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) * diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index cbf6e02..ad35402 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -509,6 +509,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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC: PATCH v2] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. 2010-04-16 15:29 ` [RFC: PATCH v2] " John Kacur @ 2010-04-19 16:51 ` John Kacur 2010-04-20 21:09 ` Andrew Morton 0 siblings, 1 reply; 11+ messages in thread From: John Kacur @ 2010-04-19 16:51 UTC (permalink / raw) To: John Kacur Cc: linux-kernel, Ingo Molnar, Peter Zijlstra, linux-rt-users, Thomas Gleixner, Clark Williams, Luis Claudio R. Goncalves, Andrew Morton 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 Thanks John >From b47fcc543e55b6d1a89c5cdf0f3f7501728bb8e1 Mon Sep 17 00:00:00 2001 From: John Kacur <jkacur@redhat.com> Date: Fri, 16 Apr 2010 13:24:02 +0200 Subject: [PATCH] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. Cc: Ingo Molnar <mingo@elte.hu>, Peter Zijlstra <a.p.zijlstra@chello.nl>, Thomas Gleixner <tglx@linutronix.de>, Clark Williams <williams@redhat.com>, Luis Claudio R. Goncalves <lgoncalv@redhat.com>, Andrew Morton <akpm@linux-foundation.org> 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@redhat.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) * 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC: PATCH v2] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. 2010-04-19 16:51 ` John Kacur @ 2010-04-20 21:09 ` Andrew Morton 2010-04-21 11:12 ` [PATCH v3] " John Kacur 0 siblings, 1 reply; 11+ messages in thread From: Andrew Morton @ 2010-04-20 21:09 UTC (permalink / raw) To: John Kacur Cc: John Kacur, linux-kernel, Ingo Molnar, Peter Zijlstra, linux-rt-users, Thomas Gleixner, Clark Williams, Luis Claudio R. Goncalves On Mon, 19 Apr 2010 18:51:57 +0200 (CEST) John Kacur <jkacur@gmail.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@redhat.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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. 2010-04-20 21:09 ` Andrew Morton @ 2010-04-21 11:12 ` John Kacur 2010-04-21 11:37 ` Peter Zijlstra 0 siblings, 1 reply; 11+ messages in thread From: John Kacur @ 2010-04-21 11:12 UTC (permalink / raw) To: Andrew Morton Cc: John Kacur, linux-kernel, Ingo Molnar, Peter Zijlstra, linux-rt-users, Thomas Gleixner, Clark Williams, Luis Claudio R. Goncalves >From f9a733a806a3a486fe24b54c82b74ad6a4137e8b Mon Sep 17 00:00:00 2001 From: John Kacur <jkacur@redhat.com> Date: Fri, 16 Apr 2010 13:24:02 +0200 Subject: [PATCH v3] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. Cc: Ingo Molnar <mingo@elte.hu>, Peter Zijlstra <a.p.zijlstra@chello.nl>, Thomas Gleixner <tglx@linutronix.de>, Clark Williams <williams@redhat.com>, Luis Claudio R. Goncalves <lgoncalv@redhat.com>, Andrew Morton <akpm@linux-foundation.org> 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. -v2 - generated against tip/master -v3 - fix compiler warning reported by Andrew Morton in lockdep_proc.c Signed-off-by: John Kacur <jkacur@redhat.com> --- kernel/lockdep.c | 8 ++++---- kernel/lockdep_internals.h | 6 ------ kernel/lockdep_proc.c | 4 ++-- lib/Kconfig.debug | 9 +++++++++ 4 files changed, 15 insertions(+), 12 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..9954401 100644 --- a/kernel/lockdep_proc.c +++ b/kernel/lockdep_proc.c @@ -305,8 +305,8 @@ static int lockdep_stats_show(struct seq_file *m, void *v) #endif 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); + seq_printf(m, " stack-trace entries: %11lu [max: %d]\n", + 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) * 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. 2010-04-21 11:12 ` [PATCH v3] " John Kacur @ 2010-04-21 11:37 ` Peter Zijlstra 2010-04-21 11:47 ` Gregory Haskins 2010-04-21 12:12 ` John Kacur 0 siblings, 2 replies; 11+ messages in thread From: Peter Zijlstra @ 2010-04-21 11:37 UTC (permalink / raw) To: John Kacur Cc: Andrew Morton, linux-kernel, Ingo Molnar, linux-rt-users, Thomas Gleixner, Clark Williams, Luis Claudio R. Goncalves On Wed, 2010-04-21 at 13:12 +0200, John Kacur wrote: > > 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. I'm not sure its worth having a CONFIG_ value for this, that'll just be yet another random value nobody knows what to do with. Do you actually have a machine that reproduces this? Can you see how many classes, avg stacktraces per class and the avg entries per stacktrace there are? Also, is there's lots of classes, are there many with a similar name? That is, is it a valid depletion or is there something wonkey with those setups? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. 2010-04-21 11:37 ` Peter Zijlstra @ 2010-04-21 11:47 ` Gregory Haskins 2010-04-21 12:03 ` Peter Zijlstra 2010-04-21 12:12 ` John Kacur 1 sibling, 1 reply; 11+ messages in thread From: Gregory Haskins @ 2010-04-21 11:47 UTC (permalink / raw) To: Peter Zijlstra, John Kacur Cc: Ingo Molnar, Thomas Gleixner, Andrew Morton, Luis Claudio R. Goncalves, Clark Williams, linux-kernel, linux-rt-users >>> On 4/21/2010 at 07:37 AM, in message <1271849823.1776.87.camel@laptop>, Peter Zijlstra <peterz@infradead.org> wrote: > On Wed, 2010-04-21 at 13:12 +0200, John Kacur wrote: >> >> 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. > > I'm not sure its worth having a CONFIG_ value for this, that'll just be > yet another random value nobody knows what to do with. > > Do you actually have a machine that reproduces this? Can you see how > many classes, avg stacktraces per class and the avg entries per > stacktrace there are? > > Also, is there's lots of classes, are there many with a similar name? > > That is, is it a valid depletion or is there something wonkey with those > setups? Hi John, Peter. I am not sure if Johns solution is the right/best one per se, but I can attest that I used to hit this problem _all_ the time and it was somewhat annoying to need to patch the kernel on all of my machines to fix it. I realize that I perhaps do not represent the average user, but it was a pain-point for me. FWIW, John's patch would indeed make my life easier since I tend to share the .config between builds. Kind Regards, -Greg > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. 2010-04-21 11:47 ` Gregory Haskins @ 2010-04-21 12:03 ` Peter Zijlstra 0 siblings, 0 replies; 11+ messages in thread From: Peter Zijlstra @ 2010-04-21 12:03 UTC (permalink / raw) To: Gregory Haskins Cc: John Kacur, Ingo Molnar, Thomas Gleixner, Andrew Morton, Luis Claudio R. Goncalves, Clark Williams, linux-kernel, linux-rt-users On Wed, 2010-04-21 at 05:47 -0600, Gregory Haskins wrote: > > I am not sure if Johns solution is the right/best one per se, but I can attest > that I used to hit this problem _all_ the time and it was somewhat annoying > to need to patch the kernel on all of my machines to fix it. I realize that I > perhaps do not represent the average user, but it was a pain-point for me. > FWIW, John's patch would indeed make my life easier since I tend to share the > .config between builds. Right, so all I'm wanting to know if its a symptom of some funny or an actual depletion, in the latter case I think the best solution is to simply increase the number. In the former case we should of course fix the real issue instead of making it disappear. So one case I remember is where some code managed to create 1k classes where 1 would have sufficed, this resulted in at least 1k extra stack traces to be stored, consuming vast amounts of stack_entries. So please, if you can reproduce, look at where these entries are going, lots of classes with the same name are a good hint that something is fishy. Classes with more than 13 (4*nr_states + 1) stacks should also never happen, etc.. Is this specific to -RT, or do we see it without as well? If so, what in -RT grows this? Are we creating a class per rt_mutex spinlock or something silly like that? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. 2010-04-21 11:37 ` Peter Zijlstra 2010-04-21 11:47 ` Gregory Haskins @ 2010-04-21 12:12 ` John Kacur 2010-04-21 12:24 ` Peter Zijlstra 2010-04-21 14:53 ` Sven-Thorsten Dietrich 1 sibling, 2 replies; 11+ messages in thread From: John Kacur @ 2010-04-21 12:12 UTC (permalink / raw) To: Peter Zijlstra Cc: Andrew Morton, linux-kernel, Ingo Molnar, linux-rt-users, Thomas Gleixner, Clark Williams, Luis Claudio R. Goncalves, Gregory Haskins On Wed, 21 Apr 2010, Peter Zijlstra wrote: > On Wed, 2010-04-21 at 13:12 +0200, John Kacur wrote: > > > > 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. > > I'm not sure its worth having a CONFIG_ value for this, that'll just be > yet another random value nobody knows what to do with. > > Do you actually have a machine that reproduces this? Can you see how > many classes, avg stacktraces per class and the avg entries per > stacktrace there are? This triggers every single time when I boot my T500 laptop with 2.6.33.2-rt13 with lots of debug options enabled. The problem is not specific to this kernel though. > > Also, is there's lots of classes, are there many with a similar name? > > That is, is it a valid depletion or is there something wonkey with those > setups? Here are the top 10 lines or so of /proc/lockdep_stats lock-classes: 1330 [max: 8191] direct dependencies: 12754 [max: 16384] indirect dependencies: 33245 all direct dependencies: 49074 dependency chains: 19641 [max: 32768] dependency chain hlocks: 73246 [max: 163840] in-hardirq chains: 25 in-softirq chains: 0 in-process chains: 19616 stack-trace entries: 262144 [max: 262144] I'm looking at more details in /proc/lockdep and friends to see if I can find any more details, or something that looks amiss. John ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. 2010-04-21 12:12 ` John Kacur @ 2010-04-21 12:24 ` Peter Zijlstra 2010-04-21 14:53 ` Sven-Thorsten Dietrich 1 sibling, 0 replies; 11+ messages in thread From: Peter Zijlstra @ 2010-04-21 12:24 UTC (permalink / raw) To: John Kacur Cc: Andrew Morton, linux-kernel, Ingo Molnar, linux-rt-users, Thomas Gleixner, Clark Williams, Luis Claudio R. Goncalves, Gregory Haskins On Wed, 2010-04-21 at 14:12 +0200, John Kacur wrote: > > lock-classes: 1330 [max: 8191] > direct dependencies: 12754 [max: 16384] > indirect dependencies: 33245 > all direct dependencies: 49074 > dependency chains: 19641 [max: 32768] Right, so each dependency also gets a stack trace, see add_lock_to_list(). > dependency chain hlocks: 73246 [max: 163840] > in-hardirq chains: 25 > in-softirq chains: 0 > in-process chains: 19616 > stack-trace entries: 262144 [max: 262144] > > > I'm looking at more details in /proc/lockdep and friends to see if > I can find any more details, or something that looks amiss. > It might be useful to add a counter that simply counts all save_trace() invocations, and maybe split them out according to new_bit in mark_lock(). ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable. 2010-04-21 12:12 ` John Kacur 2010-04-21 12:24 ` Peter Zijlstra @ 2010-04-21 14:53 ` Sven-Thorsten Dietrich 1 sibling, 0 replies; 11+ messages in thread From: Sven-Thorsten Dietrich @ 2010-04-21 14:53 UTC (permalink / raw) To: John Kacur Cc: Peter Zijlstra, Andrew Morton, linux-kernel, Ingo Molnar, linux-rt-users, Thomas Gleixner, Clark Williams, Luis Claudio R. Goncalves, Gregory Haskins On Wed, 2010-04-21 at 14:12 +0200, John Kacur wrote: > > On Wed, 21 Apr 2010, Peter Zijlstra wrote: > > > On Wed, 2010-04-21 at 13:12 +0200, John Kacur wrote: > > > > > > 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. > > > > I'm not sure its worth having a CONFIG_ value for this, that'll just be > > yet another random value nobody knows what to do with. > > > > Do you actually have a machine that reproduces this? Can you see how > > many classes, avg stacktraces per class and the avg entries per > > stacktrace there are? > > This triggers every single time when I boot my T500 laptop with > 2.6.33.2-rt13 with lots of debug options enabled. The problem is not > specific to this kernel though. > Working from ancient memory here: I think that this is seen more on the distro side - had the same problem with SLERT. And yes, it did show up more with additional debug options -- when structs with #ifdef DEBUG_XXX are populated. IIRC there is another caveat, if MAX_STACK_TRACE_ENTRIES is too large, you run out of memory uncompressing the Kernel (on 32 bit only?) and it hangs. We put patches in place to manually increase to something workable, but I would ACK this change, allowing tinkering. Regards, Sven ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-04-21 14:53 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-04-16 11:45 [RFC: PATCH] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable John Kacur 2010-04-16 15:29 ` [RFC: PATCH v2] " John Kacur 2010-04-19 16:51 ` John Kacur 2010-04-20 21:09 ` Andrew Morton 2010-04-21 11:12 ` [PATCH v3] " John Kacur 2010-04-21 11:37 ` Peter Zijlstra 2010-04-21 11:47 ` Gregory Haskins 2010-04-21 12:03 ` Peter Zijlstra 2010-04-21 12:12 ` John Kacur 2010-04-21 12:24 ` Peter Zijlstra 2010-04-21 14:53 ` Sven-Thorsten Dietrich
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).