lockdep: Add nr_save_trace_invocations counter

From 5181c0296dd1549e4e706ff25a4cd81a1d90137d Mon Sep 17 00:00:00 2001

NOT FOR INCLUSION

I created this patch as a result of Peter Zilstra's request to get more info from lockdep. This patch is not for inclusion, at least in its present form, because it adds some redunant info to /proc/lockdep_stats

However, some of the fields are new, and it is worth examining, and / or applying if you are looking at the MAX_STACK_TRACE_ENTRIES too big problem.

I generated this patch against a recent tip/master but it applies without conflicts to the latest rt kernel as well. Comments are welcome, in fact they are appreciated.

Quoted 1 line

From 5181c0296dd1549e4e706ff25a4cd81a1d90137d Mon Sep 17 00:00:00 2001

From: John Kacur <[email protected]>
Date: Thu, 22 Apr 2010 17:02:42 +0200
Subject: [PATCH] lockdep: Add nr_save_trace_invocations counter

Add the nr_save_trace_invocations counter which counts the number of time save_trace() is invoked when relevant for trace enteries.

This means, those invocations from mark_lock() and add_lock_to_list()

When called from mark_lock() we break it down into LOCKSTATE categories.

Signed-off-by John Kacur <[email protected]>

+unsigned long nr_save_trace_invocations_type[LOCK_USAGE_STATES];

kernel/lockdep.c +17 -0

+
 static int save_trace(struct stack_trace *trace)
 {
 	trace->nr_entries = 0;

@@ -410,6 +414,19 @@ static int save_trace(struct stack_trace *trace)
 	return 1;
 }
 
+/*
+ * This function is only called from mark_lock() and add_lock_to_list()
+ * which are only called when holding the graph_lock. This counter
+ * piggybacks off of that lock
+ */
+static void inc_save_trace_invocations(enum lock_usage_bit new_bit)
+{
+	nr_save_trace_invocations++;
+	if (WARN_ON(new_bit >= LOCK_USAGE_STATES))
+		return;
+	nr_save_trace_invocations_type[new_bit]++;
+}
+
 unsigned int nr_hardirq_chains;
 unsigned int nr_softirq_chains;
 unsigned int nr_process_chains;

@@ -449,6 +466,7 @@ static const char *usage_str[] =
 #define LOCKDEP_STATE(__STATE) __USAGE(__STATE)
 #include "lockdep_states.h"
 #undef LOCKDEP_STATE
+#undef __USAGE
 	[LOCK_USED] = "INITIAL USE",
 };
 

@@ -816,6 +834,7 @@ static int add_lock_to_list(struct lock_class *class, struct lock_class *this,
 	if (!entry)
 		return 0;
 
+	nr_save_trace_invocations++;
 	if (!save_trace(&entry->trace))
 		return 0;
 

@@ -2615,6 +2634,7 @@ static int mark_lock(struct task_struct *curr, struct held_lock *this,
 
 	hlock_class(this)->usage_mask |= new_mask;
 
+	inc_save_trace_invocations(new_bit);
 	if (!save_trace(hlock_class(this)->usage_traces + new_bit))
 		return 0;
 

kernel/lockdep_internals.h +2 -0

index 8d7d4b6..6149358 100644
@@ -84,6 +84,8 @@ extern unsigned long nr_list_entries;
 extern unsigned long nr_lock_chains;
 extern int nr_chain_hlocks;
 extern unsigned long nr_stack_trace_entries;
+extern unsigned long nr_save_trace_invocations;
+extern unsigned long nr_save_trace_invocations_type[LOCK_USAGE_STATES];
 
 extern unsigned int nr_hardirq_chains;
 extern unsigned int nr_softirq_chains;

kernel/lockdep_proc.c +23 -0

index 59b76c8..ef5f372 100644
@@ -215,8 +215,24 @@ static void lockdep_stats_debug_show(struct seq_file *m)
 #endif
 }
 
+#define __USAGE(__STATE)	\
+[LOCK_USED_IN_##__STATE] = "LOCK_USED_IN_"__stringify(__STATE),	\
+[LOCK_ENABLED_##__STATE] = "LOCK_ENABLED_"__stringify(__STATE), \
+[LOCK_USED_IN_##__STATE##_READ] = "LOCK_USED_IN_"__stringify(__STATE)"_READ", \
+[LOCK_ENABLED_##__STATE##_READ] = "LOCK_ENABLED_"__stringify(__STATE)"_READ",
+
+static const char *lockstate_tostr[] =
+{
+#define LOCKDEP_STATE(__STATE) __USAGE(__STATE)
+#include "lockdep_states.h"
+#undef LOCKDEP_STATE
+#undef __USAGE
+	[LOCK_USED] = "LOCK_USED",
+};
+
 static int lockdep_stats_show(struct seq_file *m, void *v)
 {
+	int bit;
 	struct lock_class *class;
 	unsigned long nr_unused = 0, nr_uncategorized = 0,
 		      nr_irq_safe = 0, nr_irq_unsafe = 0,

@@ -307,6 +323,13 @@ static int lockdep_stats_show(struct seq_file *m, void *v)
 			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 invocations: %lu\n",
+			nr_save_trace_invocations);
+
+	for (bit=0; bit < LOCK_USAGE_STATES; bit++)
+		seq_printf(m, "\t%s: %lu\n", lockstate_tostr[bit],
+			       nr_save_trace_invocations_type[bit]);	
+
 	seq_printf(m, " combined max dependencies:     %11u\n",
 			(nr_hardirq_chains + 1) *
 			(nr_softirq_chains + 1) *

Signature · 7 lines

1.6.6.1

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

Quoted 2 lines

+unsigned long nr_save_trace_invocations_type[LOCK_USAGE_STATES];

And each item in this array will equal to nr_hardirq_[un]safe, nr_softirq_[un]safe and such things under lockdep_stats_show(). Right?

Quoted 19 lines · John Kacur

NOT FOR INCLUSION

I created this patch as a result of Peter Zilstra's request to get more info from lockdep. This patch is not for inclusion, at least in its present form, because it adds some redunant info to /proc/lockdep_stats

However, some of the fields are new, and it is worth examining, and / or applying if you are looking at the MAX_STACK_TRACE_ENTRIES too big problem.

I generated this patch against a recent tip/master but it applies without conflicts to the latest rt kernel as well. Comments are welcome, in fact they are appreciated.

Quoted 1 line

From 5181c0296dd1549e4e706ff25a4cd81a1d90137d Mon Sep 17 00:00:00 2001

From: John Kacur <[email protected]>
Date: Thu, 22 Apr 2010 17:02:42 +0200
Subject: [PATCH] lockdep: Add nr_save_trace_invocations counter

Quoted 8 lines

Add the nr_save_trace_invocations counter which counts the number of time save_trace() is invoked when relevant for trace enteries.

This means, those invocations from mark_lock() and add_lock_to_list()

When called from mark_lock() we break it down into LOCKSTATE categories.

Signed-off-by John Kacur <[email protected]>

Just take a rough look at it. I don't think this can give more info.

kernel/lockdep.c static int verbose(struct lock_class *class) · line 375

Quoted 2 lines

kernel/lockdep.c +2 -0

+/* Calls to save_trace() from mark_lock() and add_lock_to_list() only*/
+unsigned long nr_save_trace_invocations;

It will equal to nr_list_entries.

Quoted 1 line

+unsigned long nr_save_trace_invocations_type[LOCK_USAGE_STATES];

And each item in this array will equal to nr_hardirq_[un]safe, nr_softirq_[un]safe and such things under lockdep_stats_show(). Right?

Thanks,
Yong

Some context here - Peter asked me to see if we could get some more detailed stats on why some configurations reach the MAX_STACK_TRACE_ENTRIES limit - whether the limit was really too low for some circumstances, or whether we were counting somethings unnecessarily.

In any case, I stamped a big NOT FOR INCLUSION on my mail, because I noticed that somethings were redundant - albeit, obtained in a slightly different manner, however, not everything is redundant.

In particular, nr_save_trace_invocations is NOT equal to nr_list_entries. You will see that reported in /proc/lockdep_stats as
direct dependencies: 8752 [max: 16384]
I have
stack-trace invocations: 10888
from the same run.

Still trying to figure out what the meaning is of that though to be honest.

Here is a portion of the lockdep_stats, with all of the new fields and the redundant ones.

stack-trace invocations: 10888

	LOCK_USED_IN_HARDIRQ: 15
	LOCK_USED_IN_HARDIRQ_READ: 0
	LOCK_ENABLED_HARDIRQ: 543
	LOCK_ENABLED_HARDIRQ_READ: 28
	LOCK_USED_IN_SOFTIRQ: 0
	LOCK_USED_IN_SOFTIRQ_READ: 0
	LOCK_ENABLED_SOFTIRQ: 543
	LOCK_ENABLED_SOFTIRQ_READ: 28
	LOCK_USED_IN_RECLAIM_FS: 5
	LOCK_USED_IN_RECLAIM_FS_READ: 0
	LOCK_ENABLED_RECLAIM_FS: 95
	LOCK_ENABLED_RECLAIM_FS_READ: 8
	LOCK_USED: 871

combined max dependencies: 139841
hardirq-safe locks: 15
hardirq-unsafe locks: 543
softirq-safe locks: 0
softirq-unsafe locks: 543
irq-safe locks: 15
irq-unsafe locks: 543
hardirq-read-safe locks: 0
hardirq-read-unsafe locks: 28
softirq-read-safe locks: 0
softirq-read-unsafe locks: 28
irq-read-safe locks: 0
irq-read-unsafe locks: 28

So, you see that all of the reclaim fields are new,
LOCK_USED_IN_RECLAIM_FS: 5 LOCK_USED_IN_RECLAIM_FS_READ: 0
LOCK_ENABLED_RECLAIM_FS: 95 LOCK_ENABLED_RECLAIM_FS_READ: 8

I can create a patch for inclusion that adds the reclaim fields, the question is, is the nr_save_trace_invocations a useful stat for us or not?

Quoted 5 lines · Yong Zhang

Quoted 2 lines

+unsigned long nr_save_trace_invocations_type[LOCK_USAGE_STATES];

And each item in this array will equal to nr_hardirq_[un]safe, nr_softirq_[un]safe and such things under lockdep_stats_show(). Right?

Well, the stats for RECLAIM_FS as missing, so at the very least we need to re-write lockdep_stats_show() to use the lockdep_states.h magic.

direct dependencies: 8752 [max: 16384]
I have
stack-trace invocations: 10888
from the same run.

Still trying to figure out what the meaning is of that though to be honest.

Here is a portion of the lockdep_stats, with all of the new fields and the redundant ones.

stack-trace invocations: 10888

        LOCK_USED_IN_HARDIRQ: 15
        LOCK_USED_IN_HARDIRQ_READ: 0
        LOCK_ENABLED_HARDIRQ: 543
        LOCK_ENABLED_HARDIRQ_READ: 28
        LOCK_USED_IN_SOFTIRQ: 0
        LOCK_USED_IN_SOFTIRQ_READ: 0
        LOCK_ENABLED_SOFTIRQ: 543
        LOCK_ENABLED_SOFTIRQ_READ: 28
        LOCK_USED_IN_RECLAIM_FS: 5
        LOCK_USED_IN_RECLAIM_FS_READ: 0
        LOCK_ENABLED_RECLAIM_FS: 95
        LOCK_ENABLED_RECLAIM_FS_READ: 8
        LOCK_USED: 871 

Quoted 11 lines · John Kacur

direct dependencies:                  8752 [max: 16384] I have
stack-trace invocations: 10888
from the same run.

Still trying to figure out what the meaning is of that though to be honest.

Here is a portion of the lockdep_stats, with all of the new fields and the redundant ones.

Quoted 14 lines

stack-trace invocations: 10888
        LOCK_USED_IN_HARDIRQ: 15
        LOCK_USED_IN_HARDIRQ_READ: 0
        LOCK_ENABLED_HARDIRQ: 543
        LOCK_ENABLED_HARDIRQ_READ: 28
        LOCK_USED_IN_SOFTIRQ: 0
        LOCK_USED_IN_SOFTIRQ_READ: 0
        LOCK_ENABLED_SOFTIRQ: 543
        LOCK_ENABLED_SOFTIRQ_READ: 28
        LOCK_USED_IN_RECLAIM_FS: 5
        LOCK_USED_IN_RECLAIM_FS_READ: 0
        LOCK_ENABLED_RECLAIM_FS: 95
        LOCK_ENABLED_RECLAIM_FS_READ: 8
        LOCK_USED: 871

8752+871+8+95+5+28+543+28+543+15=10888

So you get a stack-trace for each direct-dependency, and you get a stack-trace for each LOCK_state, the sum seems to match the total invocations.

Non of these numbers look strange..

Quoted 8 lines

8752+871+8+95+5+28+543+28+543+15=10888

So you get a stack-trace for each direct-dependency, and you get a stack-trace for each LOCK_state, the sum seems to match the total invocations.

Non of these numbers look strange..

As I told Peter privately the laptop that triggered the MAX_STACK_TRACE_ENTRIES every time, has met an
unfortunate early demise. However, I think it was the config - not the hardware. On this machine where the above
numbers come from, I believe I have less debug options configured - but it is running the exact same kernel as
the laptop was. (2.6.33.2-rt13)

Quoted 9 lines · John Kacur

Quoted 8 lines

8752+871+8+95+5+28+543+28+543+15=10888

So you get a stack-trace for each direct-dependency, and you get a stack-trace for each LOCK_state, the sum seems to match the total invocations.

Non of these numbers look strange..

Quoted 7 lines

As I told Peter privately the laptop that triggered the MAX_STACK_TRACE_ENTRIES every time, has met an
unfortunate early demise. However, I think it was the config - not the hardware. On this machine where the above
numbers come from, I believe I have less debug options configured - but it is running the exact same kernel as
the laptop was. (2.6.33.2-rt13)

Through a rough computation:
MAX_STACK_TRACE_ENTRIES/10888 = 24
That means the average stack deepth is about 24.
So I'm thinking if we can take a check on the biggest deepth? Could this make sense?

Quoted 2 lines

Non of these numbers look strange..

As I told Peter privately the laptop that triggered the MAX_STACK_TRACE_ENTRIES every time, has met an
unfortunate early demise. However, I think it was the config - not the hardware. On this machine where the above
numbers come from, I believe I have less debug options configured - but it is running the exact same kernel as
the laptop was. (2.6.33.2-rt13)

From 6b9d513b7c417c0805ef0acc1cb3227bddba0889 Mon Sep 17 00:00:00 2001

Hi John,

(checking mail at home).
I find some place which can be hacked. Below is the patch. But I don't even compile it. Can you test it to see if it can smooth your problem.

---cut here ---

Quoted 1 line

From 6b9d513b7c417c0805ef0acc1cb3227bddba0889 Mon Sep 17 00:00:00 2001

From: Yong Zhang <[email protected]>
Date: Fri, 23 Apr 2010 21:13:54 +0800
Subject: [PATCH] lockdep: reduce stack_trace usage

When calling check_prevs_add(), if all validations passed add_lock_to_list() will add new lock to dependency tree and alloc stack_trace for each list_entry. But at this time, we are always on the same stack, so stack_trace for each list_entry has the same value. This is redundant and eats up lots of memory which could lead to warning on low MAX_STACK_TRACE_ENTRIES.
Using one copy of stack_trace instead.

cat /proc/lockdep_stats

Signed-off-by Yong Zhang <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc David S. Miller <[email protected]>

---
 kernel/lockdep.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

kernel/lockdep.c +12 -8

index 2594e1c..097d5fb 100644
@@ -818,7 +818,8 @@ static struct lock_list *alloc_list_entry(void)
  * Add a new dependency to the head of the list:
  */
 static int add_lock_to_list(struct lock_class *class, struct lock_class *this,
-			    struct list_head *head, unsigned long ip, int distance)
+			    struct list_head *head, unsigned long ip,
+			    int distance, struct stack_trace *trace)
 {
 	struct lock_list *entry;
 	/*

@@ -829,11 +830,9 @@ static int add_lock_to_list(struct lock_class *class, struct lock_class *this,
 	if (!entry)
 		return 0;
 
-	if (!save_trace(&entry->trace))
-		return 0;
-
 	entry->class = this;
 	entry->distance = distance;
+	entry->trace = *trace;
 	/*
 	 * Since we never remove from the dependency list, the list can
 	 * be walked lockless by other CPUs, it's only allocation

@@ -1635,7 +1634,7 @@ check_deadlock(struct task_struct *curr, struct held_lock *next,
  */
 static int
 check_prev_add(struct task_struct *curr, struct held_lock *prev,
-	       struct held_lock *next, int distance)
+	       struct held_lock *next, int distance, struct stack_trace *trace)
 {
 	struct lock_list *entry;
 	int ret;

@@ -1694,14 +1693,14 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev,
 	 */
 	ret = add_lock_to_list(hlock_class(prev), hlock_class(next),
 			       &hlock_class(prev)->locks_after,
-			       next->acquire_ip, distance);
+			       next->acquire_ip, distance, trace);
 
 	if (!ret)
 		return 0;
 
 	ret = add_lock_to_list(hlock_class(next), hlock_class(prev),
 			       &hlock_class(next)->locks_before,
-			       next->acquire_ip, distance);
+			       next->acquire_ip, distance, trace);
 	if (!ret)
 		return 0;
 

@@ -1732,6 +1731,7 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next)
 {
 	int depth = curr->lockdep_depth;
 	struct held_lock *hlock;
+	struct stack_trace trace;
 
 	/*
 	 * Debugging checks.

@@ -1748,6 +1748,9 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next)
 			curr->held_locks[depth-1].irq_context)
 		goto out_bug;
 
+	if (!save_trace(&trace))
+		return 0;
+
 	for (;;) {
 		int distance = curr->lockdep_depth - depth + 1;
 		hlock = curr->held_locks + depth-1;

@@ -1756,7 +1759,8 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next)
 		 * added:
 		 */
 		if (hlock->read != 2) {
-			if (!check_prev_add(curr, hlock, next, distance))
+			if (!check_prev_add(curr, hlock, next,
+						distance, &trace))
 				return 0;
 			/*
 			 * Stop after the first non-trylock entry,

Signature · 1 line

1.6.3.3

From 6b9d513b7c417c0805ef0acc1cb3227bddba0889 Mon Sep 17 00:00:00 2001
From: Yong Zhang <[email protected]>
Date: Fri, 23 Apr 2010 21:13:54 +0800
Subject: [PATCH] lockdep: reduce stack_trace usage

When calling check_prevs_add(), if all validations passed add_lock_to_list() will add new lock to dependency tree and alloc stack_trace for each list_entry. But at this time, we are always on the same stack, so stack_trace for each list_entry has the same value. This is redundant and eats up lots of memory which could lead to warning on low MAX_STACK_TRACE_ENTRIES.
Using one copy of stack_trace instead.

Signed-off-by Yong Zhang <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc David S. Miller <[email protected]>

---
 kernel/lockdep.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

kernel/lockdep.c +12 -8

index 2594e1c..097d5fb 100644
@@ -818,7 +818,8 @@ static struct lock_list *alloc_list_entry(void)
  * Add a new dependency to the head of the list:
  */
 static int add_lock_to_list(struct lock_class *class, struct lock_class *this,
-			    struct list_head *head, unsigned long ip, int distance)
+			    struct list_head *head, unsigned long ip,
+			    int distance, struct stack_trace *trace)
 {
 	struct lock_list *entry;
 	/*

@@ -829,11 +830,9 @@ static int add_lock_to_list(struct lock_class *class, struct lock_class *this,
 	if (!entry)
 		return 0;
 
-	if (!save_trace(&entry->trace))
-		return 0;
-
 	entry->class = this;
 	entry->distance = distance;
+	entry->trace = *trace;
 	/*
 	 * Since we never remove from the dependency list, the list can
 	 * be walked lockless by other CPUs, it's only allocation

@@ -1635,7 +1634,7 @@ check_deadlock(struct task_struct *curr, struct held_lock *next,
  */
 static int
 check_prev_add(struct task_struct *curr, struct held_lock *prev,
-	       struct held_lock *next, int distance)
+	       struct held_lock *next, int distance, struct stack_trace *trace)
 {
 	struct lock_list *entry;
 	int ret;

@@ -1694,14 +1693,14 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev,
 	 */
 	ret = add_lock_to_list(hlock_class(prev), hlock_class(next),
 			       &hlock_class(prev)->locks_after,
-			       next->acquire_ip, distance);
+			       next->acquire_ip, distance, trace);
 
 	if (!ret)
 		return 0;
 
 	ret = add_lock_to_list(hlock_class(next), hlock_class(prev),
 			       &hlock_class(next)->locks_before,
-			       next->acquire_ip, distance);
+			       next->acquire_ip, distance, trace);
 	if (!ret)
 		return 0;
 

@@ -1732,6 +1731,7 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next)
 {
 	int depth = curr->lockdep_depth;
 	struct held_lock *hlock;
+	struct stack_trace trace;
 
 	/*
 	 * Debugging checks.

@@ -1748,6 +1748,9 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next)
 			curr->held_locks[depth-1].irq_context)
 		goto out_bug;
 
+	if (!save_trace(&trace))
+		return 0;
+
 	for (;;) {
 		int distance = curr->lockdep_depth - depth + 1;
 		hlock = curr->held_locks + depth-1;

@@ -1756,7 +1759,8 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next)
 		 * added:
 		 */
 		if (hlock->read != 2) {
-			if (!check_prev_add(curr, hlock, next, distance))
+			if (!check_prev_add(curr, hlock, next,
+						distance, &trace))
 				return 0;
 			/*
 			 * Stop after the first non-trylock entry,

OK, I like the idea, but I'm a little confused as to why you pull save_trace() up two functions,

from what I can see we can now end up
saving a trace where we previously would not have done one (the whole recursive lock mess.

So please respin this with save_trace() in check_prev_add() right before the first add_lock_to_list().

Quoted 90 lines

Signed-off-by Yong Zhang <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc David S. Miller <[email protected]>

---
 kernel/lockdep.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

kernel/lockdep.c +12 -8

index 2594e1c..097d5fb 100644
@@ -818,7 +818,8 @@ static struct lock_list *alloc_list_entry(void)
  * Add a new dependency to the head of the list:
  */
 static int add_lock_to_list(struct lock_class *class, struct lock_class *this,
-			    struct list_head *head, unsigned long ip, int distance)
+			    struct list_head *head, unsigned long ip,
+			    int distance, struct stack_trace *trace)
 {
 	struct lock_list *entry;
 	/*

@@ -829,11 +830,9 @@ static int add_lock_to_list(struct lock_class *class, struct lock_class *this,
 	if (!entry)
 		return 0;
 
-	if (!save_trace(&entry->trace))
-		return 0;
-
 	entry->class = this;
 	entry->distance = distance;
+	entry->trace = *trace;
 	/*
 	 * Since we never remove from the dependency list, the list can
 	 * be walked lockless by other CPUs, it's only allocation

@@ -1635,7 +1634,7 @@ check_deadlock(struct task_struct *curr, struct held_lock *next,
  */
 static int
 check_prev_add(struct task_struct *curr, struct held_lock *prev,
-	       struct held_lock *next, int distance)
+	       struct held_lock *next, int distance, struct stack_trace *trace)
 {
 	struct lock_list *entry;
 	int ret;

@@ -1694,14 +1693,14 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev,
 	 */
 	ret = add_lock_to_list(hlock_class(prev), hlock_class(next),
 			       &hlock_class(prev)->locks_after,
-			       next->acquire_ip, distance);
+			       next->acquire_ip, distance, trace);
 
 	if (!ret)
 		return 0;
 
 	ret = add_lock_to_list(hlock_class(next), hlock_class(prev),
 			       &hlock_class(next)->locks_before,
-			       next->acquire_ip, distance);
+			       next->acquire_ip, distance, trace);
 	if (!ret)
 		return 0;
 

@@ -1732,6 +1731,7 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next)
 {
 	int depth = curr->lockdep_depth;
 	struct held_lock *hlock;
+	struct stack_trace trace;
 
 	/*
 	 * Debugging checks.

@@ -1748,6 +1748,9 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next)
 			curr->held_locks[depth-1].irq_context)
 		goto out_bug;
 
+	if (!save_trace(&trace))
+		return 0;
+
 	for (;;) {
 		int distance = curr->lockdep_depth - depth + 1;
 		hlock = curr->held_locks + depth-1;

@@ -1756,7 +1759,8 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next)
 		 * added:
 		 */
 		if (hlock->read != 2) {
-			if (!check_prev_add(curr, hlock, next, distance))
+			if (!check_prev_add(curr, hlock, next,
+						distance, &trace))
 				return 0;
 			/*
 			 * Stop after the first non-trylock entry,

From 04395389820e89c0bd4bb57b939ec1882e0bb5da Mon Sep 17 00:00:00 2001

From 04395389820e89c0bd4bb57b939ec1882e0bb5da Mon Sep 17 00:00:00 2001
From: Yong Zhang <[email protected]>
Date: Tue, 4 May 2010 14:16:48 +0800
Subject: [PATCH] lockdep: Reduce stack_trace usage

When calling check_prevs_add(), if all validations passed add_lock_to_list() will add new lock to dependency tree and alloc stack_trace for each list_entry. But at this time, we are always on the same stack, so stack_trace for each list_entry has the same value. This is redundant and eats up lots of memory which could lead to warning on low MAX_STACK_TRACE_ENTRIES.
Using one copy of stack_trace instead.

Right, this looks like it ought to do as intended.

I added the below snipped because I know I'll fail to remember and that 'static' qualifier is fairly easy to miss.

Quoted 4 lines · Peter Zijlstra

Right, this looks like it ought to do as intended.

I added the below snipped because I know I'll fail to remember and that 'static' qualifier is fairly easy to miss.

OK. Thanks for your comments.

BTW, do you need me resend the patch with your comments in?

Quoted 1 line

BTW, do you need me resend the patch with your comments in?

No, I've got it.