[RFC: PATCH] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable.

[RFC: PATCH] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable.

[John Kacur]

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

Re: [RFC: PATCH v2] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable.

[John Kacur]

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

Re: [RFC: PATCH v2] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable.

[John Kacur]

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

Re: [RFC: PATCH v2] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable.

[Andrew Morton]

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

[PATCH v3] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable.

[John Kacur]

>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

Re: [PATCH v3] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable.

[Peter Zijlstra]

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?

Re: [PATCH v3] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable.

[Gregory Haskins]

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

Re: [PATCH v3] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable.

[Peter Zijlstra]

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?

Re: [PATCH v3] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable.

[John Kacur]

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

Re: [PATCH v3] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable.

[Peter Zijlstra]

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

Re: [PATCH v3] lockdep: Make MAX_STACK_TRACE_ENTRIES configurable.

[Sven-Thorsten Dietrich]

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