2009-07-07 15:25:52

by Joao Correia

[permalink] [raw]
Subject: [PATCH 2/3] Increase lockdep limits: MAX_LOCK_DEPTH

(Applies to current Linus tree, as of 2.6.31-rc2)

As a result of increasing MAX_STACK_TRACE_ENTRIES on the previous
patch, another limit surfaced as being hit too soon.
This patch increases MAX_LOCK_DEPTH, being hit by false positives, and
turning off the locking correctness validator.

The new value is arbitrary, but I believe the old one was too. Given
the amount of changes happening with regards to the usage of lockdep,
the previous limit is just too low and keeping it that way would
defeat the purpose of lockdep.


Signed-off-by: Joao Correia <[email protected]>

---
include/linux/sched.h | 2 +-
1 files changes, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 0085d75..304231b 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1367,7 +1367,7 @@ struct task_struct {
int softirq_context;
#endif
#ifdef CONFIG_LOCKDEP
-# define MAX_LOCK_DEPTH 48UL
+# define MAX_LOCK_DEPTH 96UL
u64 curr_chain_key;
int lockdep_depth;
unsigned int lockdep_recursion;
---


2009-07-07 15:30:06

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 2/3] Increase lockdep limits: MAX_LOCK_DEPTH

On Tue, 2009-07-07 at 16:25 +0100, Joao Correia wrote:
> (Applies to current Linus tree, as of 2.6.31-rc2)
>
> As a result of increasing MAX_STACK_TRACE_ENTRIES on the previous
> patch, another limit surfaced as being hit too soon.
> This patch increases MAX_LOCK_DEPTH, being hit by false positives, and
> turning off the locking correctness validator.
>
> The new value is arbitrary, but I believe the old one was too. Given
> the amount of changes happening with regards to the usage of lockdep,
> the previous limit is just too low and keeping it that way would
> defeat the purpose of lockdep.
>
>
> Signed-off-by: Joao Correia <[email protected]>

NAK, find the site that triggers this and fix it. holding more than 48
locks at any one time is silly.

> ---
> include/linux/sched.h | 2 +-
> 1 files changes, 1 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 0085d75..304231b 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1367,7 +1367,7 @@ struct task_struct {
> int softirq_context;
> #endif
> #ifdef CONFIG_LOCKDEP
> -# define MAX_LOCK_DEPTH 48UL
> +# define MAX_LOCK_DEPTH 96UL
> u64 curr_chain_key;
> int lockdep_depth;
> unsigned int lockdep_recursion;
> ---

2009-07-07 15:36:35

by Joao Correia

[permalink] [raw]
Subject: Re: [PATCH 2/3] Increase lockdep limits: MAX_LOCK_DEPTH

Im running into this in places as diverse as modprobe, swapper, udevd
and ip6tables-restore. It doesnt happen always on the same one, and
the stack traces dont point to an (apparent) common point, hence me
flagging this as a limit that is just being triggered due to increased
lockdep usage.

Joao Correia



On Tue, Jul 7, 2009 at 4:29 PM, Peter Zijlstra<[email protected]> wrote:
> On Tue, 2009-07-07 at 16:25 +0100, Joao Correia wrote:
>> (Applies to current Linus tree, as of 2.6.31-rc2)
>>
>> As a result of increasing MAX_STACK_TRACE_ENTRIES on the previous
>> patch, another limit surfaced as being hit too soon.
>> This patch increases MAX_LOCK_DEPTH, being hit by false positives, and
>> turning off the locking correctness validator.
>>
>> The new value is arbitrary, but I believe the old one was too. Given
>> the amount of changes happening with regards to the usage of lockdep,
>> the previous limit is just too low and keeping it that way would
>> defeat the purpose of lockdep.
>>
>>
>> Signed-off-by: Joao Correia <[email protected]>
>
> NAK, find the site that triggers this and fix it. holding more than 48
> locks at any one time is silly.
>
>> ---
>> ?include/linux/sched.h | ? ?2 +-
>> ?1 files changes, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index 0085d75..304231b 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -1367,7 +1367,7 @@ struct task_struct {
>> ? ? ? ? int softirq_context;
>> ?#endif
>> ?#ifdef CONFIG_LOCKDEP
>> -# define MAX_LOCK_DEPTH 48UL
>> +# define MAX_LOCK_DEPTH 96UL
>> ? ? ? ? u64 curr_chain_key;
>> ? ? ? ? int lockdep_depth;
>> ? ? ? ? unsigned int lockdep_recursion;
>> ---
>
>

2009-07-07 15:41:25

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 2/3] Increase lockdep limits: MAX_LOCK_DEPTH


A: Because we read from top to bottom, left to right.
Q: Why should I start my reply below the quoted text?

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

A: The lost context.
Q: What makes top-posted replies harder to read than bottom-posted?


On Tue, 2009-07-07 at 16:36 +0100, Joao Correia wrote:

> On Tue, Jul 7, 2009 at 4:29 PM, Peter Zijlstra<[email protected]> wrote:
> > On Tue, 2009-07-07 at 16:25 +0100, Joao Correia wrote:
> >> (Applies to current Linus tree, as of 2.6.31-rc2)
> >>
> >> As a result of increasing MAX_STACK_TRACE_ENTRIES on the previous
> >> patch, another limit surfaced as being hit too soon.
> >> This patch increases MAX_LOCK_DEPTH, being hit by false positives, and
> >> turning off the locking correctness validator.
> >>
> >> The new value is arbitrary, but I believe the old one was too. Given
> >> the amount of changes happening with regards to the usage of lockdep,
> >> the previous limit is just too low and keeping it that way would
> >> defeat the purpose of lockdep.
> >>
> >>
> >> Signed-off-by: Joao Correia <[email protected]>
> >
> > NAK, find the site that triggers this and fix it. holding more than 48
> > locks at any one time is silly.

> Im running into this in places as diverse as modprobe, swapper, udevd
> and ip6tables-restore. It doesnt happen always on the same one, and
> the stack traces dont point to an (apparent) common point, hence me
> flagging this as a limit that is just being triggered due to increased
> lockdep usage.

This particular one will only trigger on the sites where you do indeed
hold >48 locks. If that triggers on multiple sites we really have a
problem.

> >> ---
> >> include/linux/sched.h | 2 +-
> >> 1 files changes, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/include/linux/sched.h b/include/linux/sched.h
> >> index 0085d75..304231b 100644
> >> --- a/include/linux/sched.h
> >> +++ b/include/linux/sched.h
> >> @@ -1367,7 +1367,7 @@ struct task_struct {
> >> int softirq_context;
> >> #endif
> >> #ifdef CONFIG_LOCKDEP
> >> -# define MAX_LOCK_DEPTH 48UL
> >> +# define MAX_LOCK_DEPTH 96UL
> >> u64 curr_chain_key;
> >> int lockdep_depth;
> >> unsigned int lockdep_recursion;
> >> ---
> >
> >

2009-07-07 15:47:18

by Joao Correia

[permalink] [raw]
Subject: Re: [PATCH 2/3] Increase lockdep limits: MAX_LOCK_DEPTH

On Tue, Jul 7, 2009 at 4:41 PM, Peter Zijlstra<[email protected]> wrote:
>
> A: Because we read from top to bottom, left to right.
> Q: Why should I start my reply below the quoted text?
>
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
>
> A: The lost context.
> Q: What makes top-posted replies harder to read than bottom-posted?
>

Interesting :-).

Ill keep that in mind, apologies.

Joao Correia