LOCKDEP: fix mismatched lockdep_depth/curr_chain_hash
PATCH 1/1 LOCKDEP: fix mismatched lockdep_depth/curr_chain_hash
Hi Ingo,
I am seeing a problem on the latest -rt where lockdep completely overwhelms the system to the point that it grinds to a halt on large (8-way+) systems. The problem seems to be that the class->locks_before and locks_after grow unbounded (I have observed over 1M+ entries in them) so a lock_acquire call can take over 10 seconds to finish resolving. Related to this seems to be that lockdep appears to see a chain-hash miss over and over for what I would assume should be an established graph (for instance, in double_lock_balance() in an rt_overload condition). Turning off PROVE_LOCKING (statically, or by setting debug_locks=0 dynamically restores the system to normal behavior.
I took some time tonight to study lockdep (it is quite an impressive body of code!), and came up with the following "fix". It does improve things significantly by addressing what I believe is the issue with the cache-misses (though it would appear there are still a few more issues there that need addressing as some boots are still very lethargic). I use the term "fix" loosely since I am not confident that I fully understand the intention of your logic here so I can't say for sure if it was really broken, or if I have made it worse ;)
Could you comment on what I have done here, or offer any advice on what to look for elsewhere? I based the patch on pure linux-2.6.git since I see the same issue (by visual inspection, that is) there as well.
Thanks in advance!
-Greg
------
LOCKDEP: fix mismatched lockdep_depth/curr_chain_hash
It is possible for the current->curr_chain_key to become inconsistent with the current index if the chain fails to validate. The end result is that future lock_acquire() operations may inadvertently fail to find a hit in the cache resulting in a new node being added to the graph for every acquire.
---
kernel/lockdep.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/lockdep.c +1 -1
@@ -2450,11 +2450,11 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass, chain_head =1 ; } chain_key =iterate_chain_key (chain_key, id); - curr->curr_chain_key = chain_key;if (!validate_chain (curr, lock, hlock, chain_head))return 0 ; + curr->curr_chain_key = chain_key; curr->lockdep_depth++;check_chain_key (curr);#ifdef CONFIG_DEBUG_LOCKDEP
PATCH 1/1 lockdep: fix mismatched lockdep_depth/curr_chain_hash
Stable team,
please consider this patch for the next 22-stable.
---
Subject: lockdep: fix mismatched lockdep_depth/curr_chain_hash
From: Gregory Haskins <[email protected]>
It is possible for the current->curr_chain_key to become inconsistent with the current index if the chain fails to validate. The end result is that future lock_acquire() operations may inadvertently fail to find a hit in the cache resulting in a new node being added to the graph for every acquire.
[ peterz: this might explain some of the lockdep is so _slow_ complaints. ] [ mingo: this does not impact the correctness of validation, but may slow
down future operations significantly, if the chain gets very long. ]
--- kernel/lockdep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/lockdep.c +1 -1
@@ -2166,7 +2166,6 @@ out_calc_hash: }#endif chain_key =iterate_chain_key (chain_key, id); - curr->curr_chain_key = chain_key;/* * Trylock needs to maintain the stack of held locks, but it
@@ -2215,6 +2214,7 @@ out_calc_hash:if (unlikely (!debug_locks))return 0 ; + curr->curr_chain_key = chain_key; curr->lockdep_depth++;check_chain_key (curr);#ifdef CONFIG_DEBUG_LOCKDEP
Re: [stable] [PATCH] lockdep: fix mismatched lockdep_depth/curr_chain_hash
Quoted 4 lines · Peter Zijlstra
Stable team,
please consider this patch for the next 22-stable.
I don't see this patch in Linus's upstream tree. We need it there to be able to accept it for -stable. Or is this just a bugfix of other things that are already in his tree?
thanks,
greg k-h
Quoted 47 lines
---
Subject: lockdep: fix mismatched lockdep_depth/curr_chain_hash
From: Gregory Haskins <[email protected]>It is possible for the
current->curr_chain_keyto become inconsistent with the current index if the chain fails to validate. The end result is that futurelock_acquire()operations may inadvertently fail to find a hit in the cache resulting in a new node being added to the graph for every acquire.[ peterz: this might explain some of the lockdep is so _slow_ complaints. ] [ mingo: this does not impact the correctness of validation, but may slow
down future operations significantly, if the chain gets very long. ]Signed-off-by Gregory Haskins <[email protected]>Signed-off-by Peter Zijlstra <[email protected]>Signed-off-by Ingo Molnar <[email protected]>--- kernel/lockdep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Index:
linux-2.6/kernel/lockdep.c
===================================================================kernel/lockdep.c +1
-1@@ -2166,7 +2166,6 @@ out_calc_hash: }#endif chain_key =iterate_chain_key (chain_key, id); - curr->curr_chain_key = chain_key;/* * Trylock needs to maintain the stack of held locks, but it @@ -2215,6 +2214,7 @@ out_calc_hash:if (unlikely (!debug_locks))return 0 ; + curr->curr_chain_key = chain_key; curr->lockdep_depth++;check_chain_key (curr);#ifdef CONFIG_DEBUG_LOCKDEP _______________________________________________
stable mailing list
[email protected]
http://linux.kernel.org/mailman/listinfo/stable
Re: [stable] [PATCH] lockdep: fix mismatched lockdep_depth/curr_chain_hash
Quoted 9 lines · Greg KH
Quoted 4 lines · Peter Zijlstra
Stable team,
please consider this patch for the next 22-stable.
I don't see this patch in Linus's upstream tree. We need it there to be able to accept it for -stable. Or is this just a bugfix of other things that are already in his tree?
I send Linus a similar patch, haven´t seem him pick it up yet. I´ll notify you when and if he picks it up.
Re: [stable] [PATCH] lockdep: fix mismatched lockdep_depth/curr_chain_hash
Quoted 6 lines · Peter Zijlstra
Quoted 4 lines · Greg KH
Quoted 3 lines · Peter Zijlstra
Stable team,
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
please consider this patch for the next 22-stable.
I don't see this patch in Linus's upstream tree. We need it there to be able to accept it for -stable. Or is this just a bugfix of other things that are already in his tree?
I send Linus a similar patch, haven??t seem him pick it up yet. I??ll notify you when and if he picks it up.
Great, that would be great for us -stable monkeys...
greg k-h
Re: [stable] [PATCH] lockdep: fix mismatched lockdep_depth/curr_chain_hash
Quoted 9 lines · Greg KH
Quoted 8 lines · Peter Zijlstra
Quoted 6 lines · Greg KH
Quoted 4 lines · Peter Zijlstra
Stable team,
please consider this patch for the next 22-stable.
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
I don't see this patch in Linus's upstream tree. We need it there to be able to accept it for -stable. Or is this just a bugfix of other things that are already in his tree?
I send Linus a similar patch, haven??t seem him pick it up yet. I??ll notify you when and if he picks it up.
Great, that would be great for us -stable monkeys...
3aa416b07f0adf01c090baab26fb70c35ec17623
Re: [stable] [PATCH] lockdep: fix mismatched lockdep_depth/curr_chain_hash
Quoted 7 lines · Peter Zijlstra
Quoted 6 lines · Greg KH
Quoted 5 lines · Peter Zijlstra
Quoted 4 lines · Greg KH
Quoted 3 lines · Peter Zijlstra
Stable team,
please consider this patch for the next 22-stable.
Quoted 10 lines
Quoted 6 lines
Quoted 5 lines
Quoted 3 lines
I don't see this patch in Linus's upstream tree. We need it there to be able to accept it for -stable. Or is this just a bugfix of other things that are already in his tree?
I send Linus a similar patch, haven??t seem him pick it up yet. I??ll notify you when and if he picks it up.
Great, that would be great for us -stable monkeys...
3aa416b07f0adf01c090baab26fb70c35ec17623
And for both 2.6.22 and 2.6.23...
Re: [stable] [PATCH] lockdep: fix mismatched lockdep_depth/curr_chain_hash
Quoted 13 lines · Chuck Ebbert
Quoted 12 lines · Peter Zijlstra
Quoted 11 lines · Greg KH
Quoted 10 lines · Peter Zijlstra
Quoted 7 lines · Greg KH
Quoted 3 lines · Peter Zijlstra
Stable team,
please consider this patch for the next 22-stable.
I don't see this patch in Linus's upstream tree. We need it there to be able to accept it for -stable. Or is this just a bugfix of other things that are already in his tree?
I send Linus a similar patch, haven??t seem him pick it up yet. I??ll notify you when and if he picks it up.
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Great, that would be great for us -stable monkeys...
3aa416b07f0adf01c090baab26fb70c35ec17623And for both 2.6.22 and 2.6.23...
It does not apply to 2.6.22 at all, so unless someone sends us a backported version, I'll not apply it there.
thanks,
greg k-h