lockdep: fix deadlock in lockdep_trace

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* lockdep: fix deadlock in lockdep_trace_alloc
@ 2009-03-20 10:13 Peter Zijlstra
  2009-03-20 10:26 ` [PATCH] " Peter Zijlstra
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Peter Zijlstra @ 2009-03-20 10:13 UTC (permalink / raw)
  To: Heiko Carstens, Ingo Molnar; +Cc: Nick Piggin, lkml,

Heiko reported that we grab the graph lock with irqs enabled.

Fix this by providng the same wrapper as all other lockdep entry
functions have.

Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index 3e1cc47..a39cb6d 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -2258,7 +2258,7 @@ void trace_softirqs_off(unsigned long ip)
 		debug_atomic_inc(&redundant_softirqs_off);
 }
 
-void lockdep_trace_alloc(gfp_t gfp_mask)
+static void __lockdep_trace_alloc(gfp_t gfp_mask)
 {
 	struct task_struct *curr = current;
 
@@ -2283,6 +2283,20 @@ void lockdep_trace_alloc(gfp_t gfp_mask)
 	mark_held_locks(curr, RECLAIM_FS);
 }
 
+void lockdep_trace_alloc(gfp_t gfp_mask)
+{
+	unsigned long flags;
+
+	if (unlikely(current->lockdep_recursion))
+		return;
+
+	raw_local_irq_save(flags);
+	current->lockdep_recursion = 1;
+	__lockdep_trace_alloc(gfp_mask);
+	current->lockdep_recursion = 0;
+	raw_local_irq_restore(flags);
+}
+
 static int mark_irqflags(struct task_struct *curr, struct held_lock *hlock)
 {
 	/*


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] lockdep: fix deadlock in lockdep_trace_alloc
  2009-03-20 10:13 lockdep: fix deadlock in lockdep_trace_alloc Peter Zijlstra
@ 2009-03-20 10:26 ` Peter Zijlstra
  2009-03-20 13:07   ` Heiko Carstens
  2009-03-20 10:26 ` [tip:core/locking] " Peter Zijlstra
  2009-03-30 21:24 ` [tip:core/locking-v2] lockdep: fix deadlock in lockdep_trace_alloc Peter Zijlstra
  2 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2009-03-20 10:26 UTC (permalink / raw)
  To: Heiko Carstens; +Cc: Ingo Molnar, Nick Piggin, lkml,

Heiko pointed out that checking for irqs_disabled() after we disable
them is quite pointless..

New patch below.

---
Subject: lockdep: fix deadlock in lockdep_trace_alloc
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Fri Mar 20 11:24:21 CET 2009

Heiko reported that we grab the graph lock with irqs enabled.

Fix this by providing the same wrapper as all other lockdep entry
functions have.

Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 kernel/lockdep.c |   21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

Index: linux-2.6/kernel/lockdep.c
===================================================================
--- linux-2.6.orig/kernel/lockdep.c
+++ linux-2.6/kernel/lockdep.c
@@ -2258,7 +2258,7 @@ void trace_softirqs_off(unsigned long ip
 		debug_atomic_inc(&redundant_softirqs_off);
 }
 
-void lockdep_trace_alloc(gfp_t gfp_mask)
+static void __lockdep_trace_alloc(gfp_t gfp_mask)
 {
 	struct task_struct *curr = current;
 
@@ -2277,10 +2277,27 @@ void lockdep_trace_alloc(gfp_t gfp_mask)
 	if (!(gfp_mask & __GFP_FS))
 		return;
 
+	mark_held_locks(curr, RECLAIM_FS);
+}
+
+static void check_flags(unsigned long flags);
+
+void lockdep_trace_alloc(gfp_t gfp_mask)
+{
+	unsigned long flags;
+
+	if (unlikely(current->lockdep_recursion))
+		return;
+
 	if (DEBUG_LOCKS_WARN_ON(irqs_disabled()))
 		return;
 
-	mark_held_locks(curr, RECLAIM_FS);
+	raw_local_irq_save(flags);
+	check_flags(flags);
+	current->lockdep_recursion = 1;
+	__lockdep_trace_alloc(gfp_mask);
+	current->lockdep_recursion = 0;
+	raw_local_irq_restore(flags);
 }
 
 static int mark_irqflags(struct task_struct *curr, struct held_lock *hlock)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] lockdep: fix deadlock in lockdep_trace_alloc
  2009-03-20 10:26 ` [PATCH] " Peter Zijlstra
@ 2009-03-20 13:07   ` Heiko Carstens
  2009-03-20 13:45     ` Peter Zijlstra
  0 siblings, 1 reply; 9+ messages in thread
From: Heiko Carstens @ 2009-03-20 13:07 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Ingo Molnar, Nick Piggin, lkml,

On Fri, 20 Mar 2009 11:26:38 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> Heiko pointed out that checking for irqs_disabled() after we disable
> them is quite pointless..
> 
> New patch below.
> 
> ---
> Subject: lockdep: fix deadlock in lockdep_trace_alloc
> From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date: Fri Mar 20 11:24:21 CET 2009
> 
> Heiko reported that we grab the graph lock with irqs enabled.
> 
> Fix this by providing the same wrapper as all other lockdep entry
> functions have.
> 
> Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> ---
>  kernel/lockdep.c |   21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/kernel/lockdep.c
> ===================================================================
> --- linux-2.6.orig/kernel/lockdep.c
> +++ linux-2.6/kernel/lockdep.c
> @@ -2258,7 +2258,7 @@ void trace_softirqs_off(unsigned long ip
>  		debug_atomic_inc(&redundant_softirqs_off);
>  }
> 
> -void lockdep_trace_alloc(gfp_t gfp_mask)
> +static void __lockdep_trace_alloc(gfp_t gfp_mask)
>  {
>  	struct task_struct *curr = current;
> 
> @@ -2277,10 +2277,27 @@ void lockdep_trace_alloc(gfp_t gfp_mask)
>  	if (!(gfp_mask & __GFP_FS))
>  		return;
> 
> +	mark_held_locks(curr, RECLAIM_FS);
> +}
> +
> +static void check_flags(unsigned long flags);
> +
> +void lockdep_trace_alloc(gfp_t gfp_mask)
> +{
> +	unsigned long flags;
> +
> +	if (unlikely(current->lockdep_recursion))
> +		return;
> +
>  	if (DEBUG_LOCKS_WARN_ON(irqs_disabled()))
>  		return;
> 
> -	mark_held_locks(curr, RECLAIM_FS);
> +	raw_local_irq_save(flags);
> +	check_flags(flags);
> +	current->lockdep_recursion = 1;
> +	__lockdep_trace_alloc(gfp_mask);
> +	current->lockdep_recursion = 0;
> +	raw_local_irq_restore(flags);

Hmm... still not working:

------------[ cut here ]------------
Badness at kernel/lockdep.c:2292
Modules linked in:
CPU: 0 Not tainted 2.6.29-rc8-next-20090320-dirty #19
Process swapper (pid: 0, task: 000000000061ddf0, ksp: 0000000000678000)
Krnl PSW : 0400000180000000 000000000007c784 (lockdep_trace_alloc+0xc0/0xf8)
           R:0 T:1 IO:0 EX:0 Key:0 M:0 W:0 P:0 AS:0 CC:0 PM:0 EA:3
Krnl GPRS: 0000000000000003 0000000000fe0f18 0000000000000000 000000000000000f
           0000000000667800 0000000000000000 0000000000000000 0000000000000000
           000000003fe09f00 0000000000000020 000000003fe0fc00 0000000000677910
           0000000000000020 0000000000432538 000000000007c76c 0000000000677910
Krnl Code: 000000000007c778: bf2f1000           icm     %rr2,15,0(%rr1)
           000000000007c77c: a774ffe8           brc     7,7c74c
           000000000007c780: a7f40001           brc     15,7c782
          >000000000007c784: a7f4ffe4           brc     15,7c74c
           000000000007c788: a7c10010           tmll    %rr12,16
           000000000007c78c: a784ffd4           brc     8,7c734
           000000000007c790: e32002e00004       lg      %rr2,736
           000000000007c796: 91082016           tm      22(%rr2),8
Call Trace:
([<0000000000677928>] init_thread_union+0x3928/0x4000)
 [<00000000000aaa48>] __alloc_pages_internal+0x354/0x570
 [<00000000000db184>] cache_alloc_refill+0x41c/0x7ac
 [<00000000000db7fa>] kmem_cache_alloc+0x11a/0x150
 [<00000000001fae3e>] idr_pre_get+0x92/0xcc
 [<00000000001faeac>] ida_pre_get+0x34/0xb4
 [<00000000000e399a>] set_anon_super+0x3e/0x108
 [<00000000000e46a8>] sget+0x3a4/0x47c
 [<00000000000e4fe8>] get_sb_single+0x48/0xec
 [<0000000000151d4c>] sysfs_get_sb+0x30/0x44
 [<00000000000e3764>] vfs_kern_mount+0x68/0xfc
 [<00000000000e382c>] kern_mount_data+0x34/0x44
 [<000000000068be74>] sysfs_init+0xb4/0xf4
 [<000000000068aa82>] mnt_init+0xbe/0x264
 [<000000000068a0f0>] vfs_caches_init+0xec/0x1a4
 [<0000000000678dd4>] start_kernel+0x4dc/0x6dc
 [<0000000000012020>] _ehead+0x20/0x80
INFO: lockdep is turned off.
Last Breaking-Event-Address:
 [<000000000007c780>] lockdep_trace_alloc+0xbc/0xf8

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] lockdep: fix deadlock in lockdep_trace_alloc
  2009-03-20 13:07   ` Heiko Carstens
@ 2009-03-20 13:45     ` Peter Zijlstra
  2009-03-20 14:00       ` Heiko Carstens
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2009-03-20 13:45 UTC (permalink / raw)
  To: Heiko Carstens; +Cc: Ingo Molnar, Nick Piggin, lkml,

On Fri, 2009-03-20 at 14:07 +0100, Heiko Carstens wrote:
> On Fri, 20 Mar 2009 11:26:38 +0100
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > Heiko pointed out that checking for irqs_disabled() after we disable
> > them is quite pointless..
> > 
> > New patch below.
> > 
> > ---
> > Subject: lockdep: fix deadlock in lockdep_trace_alloc
> > From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Date: Fri Mar 20 11:24:21 CET 2009
> > 
> > Heiko reported that we grab the graph lock with irqs enabled.
> > 
> > Fix this by providing the same wrapper as all other lockdep entry
> > functions have.
> > 
> > Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> > Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > ---
> >  kernel/lockdep.c |   21 +++++++++++++++++++--
> >  1 file changed, 19 insertions(+), 2 deletions(-)
> > 
> > Index: linux-2.6/kernel/lockdep.c
> > ===================================================================
> > --- linux-2.6.orig/kernel/lockdep.c
> > +++ linux-2.6/kernel/lockdep.c
> > @@ -2258,7 +2258,7 @@ void trace_softirqs_off(unsigned long ip
> >  		debug_atomic_inc(&redundant_softirqs_off);
> >  }
> > 
> > -void lockdep_trace_alloc(gfp_t gfp_mask)
> > +static void __lockdep_trace_alloc(gfp_t gfp_mask)
> >  {
> >  	struct task_struct *curr = current;
> > 
> > @@ -2277,10 +2277,27 @@ void lockdep_trace_alloc(gfp_t gfp_mask)
> >  	if (!(gfp_mask & __GFP_FS))
> >  		return;
> > 
> > +	mark_held_locks(curr, RECLAIM_FS);
> > +}
> > +
> > +static void check_flags(unsigned long flags);
> > +
> > +void lockdep_trace_alloc(gfp_t gfp_mask)
> > +{
> > +	unsigned long flags;
> > +
> > +	if (unlikely(current->lockdep_recursion))
> > +		return;
> > +
> >  	if (DEBUG_LOCKS_WARN_ON(irqs_disabled()))
> >  		return;
> > 
> > -	mark_held_locks(curr, RECLAIM_FS);
> > +	raw_local_irq_save(flags);
> > +	check_flags(flags);
> > +	current->lockdep_recursion = 1;
> > +	__lockdep_trace_alloc(gfp_mask);
> > +	current->lockdep_recursion = 0;
> > +	raw_local_irq_restore(flags);
> 
> Hmm... still not working:

Its Friday for sure... :/

Actually reading the code helps, we should only do that IRQ check if its
a __GFP_FS allocation -- those should never be done with IRQs disabled.

So we need to pass the flags down and fudge in that.

---
Subject: lockdep: fix deadlock in lockdep_trace_alloc
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Fri Mar 20 11:24:21 CET 2009

Heiko reported that we grab the graph lock with irqs enabled.

Fix this by providing the same wrapper as all other lockdep entry
functions have.

Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 kernel/lockdep.c |   21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

Index: linux-2.6/kernel/lockdep.c
===================================================================
--- linux-2.6.orig/kernel/lockdep.c
+++ linux-2.6/kernel/lockdep.c
@@ -2258,7 +2258,7 @@ void trace_softirqs_off(unsigned long ip
 		debug_atomic_inc(&redundant_softirqs_off);
 }
 
-void lockdep_trace_alloc(gfp_t gfp_mask)
+static void __lockdep_trace_alloc(gfp_t gfp_mask, unsigned long flags)
 {
 	struct task_struct *curr = current;
 
@@ -2277,12 +2277,29 @@ void lockdep_trace_alloc(gfp_t gfp_mask)
 	if (!(gfp_mask & __GFP_FS))
 		return;
 
-	if (DEBUG_LOCKS_WARN_ON(irqs_disabled()))
+	if (DEBUG_LOCKS_WARN_ON(raw_irqs_disabled_flags(flags)))
 		return;
 
 	mark_held_locks(curr, RECLAIM_FS);
 }
 
+static void check_flags(unsigned long flags);
+
+void lockdep_trace_alloc(gfp_t gfp_mask)
+{
+	unsigned long flags;
+
+	if (unlikely(current->lockdep_recursion))
+		return;
+
+	raw_local_irq_save(flags);
+	check_flags(flags);
+	current->lockdep_recursion = 1;
+	__lockdep_trace_alloc(gfp_mask, flags);
+	current->lockdep_recursion = 0;
+	raw_local_irq_restore(flags);
+}
+
 static int mark_irqflags(struct task_struct *curr, struct held_lock *hlock)
 {
 	/*


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] lockdep: fix deadlock in lockdep_trace_alloc
  2009-03-20 13:45     ` Peter Zijlstra
@ 2009-03-20 14:00       ` Heiko Carstens
  0 siblings, 0 replies; 9+ messages in thread
From: Heiko Carstens @ 2009-03-20 14:00 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Ingo Molnar, Nick Piggin, lkml,

On Fri, 20 Mar 2009 14:45:00 +0100
Peter Zijlstra <peterz@infradead.org> wrote:
> Subject: lockdep: fix deadlock in lockdep_trace_alloc
> From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date: Fri Mar 20 11:24:21 CET 2009
> 
> Heiko reported that we grab the graph lock with irqs enabled.
> 
> Fix this by providing the same wrapper as all other lockdep entry
> functions have.
> 
> Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> ---
>  kernel/lockdep.c |   21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/kernel/lockdep.c
> ===================================================================
> --- linux-2.6.orig/kernel/lockdep.c
> +++ linux-2.6/kernel/lockdep.c
> @@ -2258,7 +2258,7 @@ void trace_softirqs_off(unsigned long ip
>  		debug_atomic_inc(&redundant_softirqs_off);
>  }
> 
> -void lockdep_trace_alloc(gfp_t gfp_mask)
> +static void __lockdep_trace_alloc(gfp_t gfp_mask, unsigned long flags)
>  {
>  	struct task_struct *curr = current;
> 
> @@ -2277,12 +2277,29 @@ void lockdep_trace_alloc(gfp_t gfp_mask)
>  	if (!(gfp_mask & __GFP_FS))
>  		return;
> 
> -	if (DEBUG_LOCKS_WARN_ON(irqs_disabled()))
> +	if (DEBUG_LOCKS_WARN_ON(raw_irqs_disabled_flags(flags)))
>  		return;
> 
>  	mark_held_locks(curr, RECLAIM_FS);
>  }
> 
> +static void check_flags(unsigned long flags);
> +
> +void lockdep_trace_alloc(gfp_t gfp_mask)
> +{
> +	unsigned long flags;
> +
> +	if (unlikely(current->lockdep_recursion))
> +		return;
> +
> +	raw_local_irq_save(flags);
> +	check_flags(flags);
> +	current->lockdep_recursion = 1;
> +	__lockdep_trace_alloc(gfp_mask, flags);
> +	current->lockdep_recursion = 0;
> +	raw_local_irq_restore(flags);
> +}
> +

This one works ;)

Tested-by: Heiko Carstens <heiko.carstens@de.ibm.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [tip:core/locking] lockdep: fix deadlock in lockdep_trace_alloc
  2009-03-20 10:13 lockdep: fix deadlock in lockdep_trace_alloc Peter Zijlstra
  2009-03-20 10:26 ` [PATCH] " Peter Zijlstra
@ 2009-03-20 10:26 ` Peter Zijlstra
  2009-03-21 12:33   ` [PATCH] lockdep: fix deadlock in lockdep_trace_alloc -v2 Peter Zijlstra
  2009-03-30 21:24 ` [tip:core/locking-v2] lockdep: fix deadlock in lockdep_trace_alloc Peter Zijlstra
  2 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2009-03-20 10:26 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, peterz, npiggin,
	heiko.carstens, tglx, mingo

Commit-ID:  b0992675bc7a5eca3c6a5eeea5ed680cec09c8e7
Gitweb:     http://git.kernel.org/tip/b0992675bc7a5eca3c6a5eeea5ed680cec09c8e7
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Fri, 20 Mar 2009 11:13:20 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 20 Mar 2009 11:23:15 +0100

lockdep: fix deadlock in lockdep_trace_alloc

Heiko reported that we grab the graph lock with irqs enabled.

Fix this by providng the same wrapper as all other lockdep entry
functions have.

Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Nick Piggin <npiggin@suse.de>
LKML-Reference: <1237544000.24626.52.camel@twins>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 kernel/lockdep.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index 77ac37f..c750038 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -2257,7 +2257,7 @@ void trace_softirqs_off(unsigned long ip)
 		debug_atomic_inc(&redundant_softirqs_off);
 }
 
-void lockdep_trace_alloc(gfp_t gfp_mask)
+static void __lockdep_trace_alloc(gfp_t gfp_mask)
 {
 	struct task_struct *curr = current;
 
@@ -2282,6 +2282,20 @@ void lockdep_trace_alloc(gfp_t gfp_mask)
 	mark_held_locks(curr, RECLAIM_FS);
 }
 
+void lockdep_trace_alloc(gfp_t gfp_mask)
+{
+	unsigned long flags;
+
+	if (unlikely(current->lockdep_recursion))
+		return;
+
+	raw_local_irq_save(flags);
+	current->lockdep_recursion = 1;
+	__lockdep_trace_alloc(gfp_mask);
+	current->lockdep_recursion = 0;
+	raw_local_irq_restore(flags);
+}
+
 static int mark_irqflags(struct task_struct *curr, struct held_lock *hlock)
 {
 	/*

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] lockdep:  fix deadlock in lockdep_trace_alloc -v2
  2009-03-20 10:26 ` [tip:core/locking] " Peter Zijlstra
@ 2009-03-21 12:33   ` Peter Zijlstra
  2009-03-21 16:18     ` [tip:core/locking] lockdep: fix deadlock in lockdep_trace_alloc, take 2 Peter Zijlstra
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2009-03-21 12:33 UTC (permalink / raw)
  To: npiggin, mingo, hpa, linux-kernel, heiko.carstens, tglx, mingo
  Cc: linux-tip-commits

On Fri, 2009-03-20 at 10:26 +0000, Peter Zijlstra wrote:
> Commit-ID:  b0992675bc7a5eca3c6a5eeea5ed680cec09c8e7
> Gitweb:     http://git.kernel.org/tip/b0992675bc7a5eca3c6a5eeea5ed680cec09c8e7
> Author:     Peter Zijlstra <peterz@infradead.org>
> AuthorDate: Fri, 20 Mar 2009 11:13:20 +0100
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Fri, 20 Mar 2009 11:23:15 +0100
> 
> lockdep: fix deadlock in lockdep_trace_alloc
> 
> Heiko reported that we grab the graph lock with irqs enabled.
> 
> Fix this by providng the same wrapper as all other lockdep entry
> functions have.
> 
> Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Nick Piggin <npiggin@suse.de>
> LKML-Reference: <1237544000.24626.52.camel@twins>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 

The below delta fixes it up proper -- saw that I forgot to update the
subject yesterday for the new iterations.

---
Since we now disabled IRQs, checking for IRQs disabled is a bit
pointless, check for it in the saved flags.

Also, add the missing check_flags() check for completeness.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index a39cb6d..b0f0118 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -2258,7 +2258,7 @@ void trace_softirqs_off(unsigned long ip)
 		debug_atomic_inc(&redundant_softirqs_off);
 }
 
-static void __lockdep_trace_alloc(gfp_t gfp_mask)
+static void __lockdep_trace_alloc(gfp_t gfp_mask, unsigned long flags)
 {
 	struct task_struct *curr = current;
 
@@ -2277,12 +2277,14 @@ static void __lockdep_trace_alloc(gfp_t gfp_mask)
 	if (!(gfp_mask & __GFP_FS))
 		return;
 
-	if (DEBUG_LOCKS_WARN_ON(irqs_disabled()))
+	if (DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags)))
 		return;
 
 	mark_held_locks(curr, RECLAIM_FS);
 }
 
+static void check_flags(unsigned long flags);
+
 void lockdep_trace_alloc(gfp_t gfp_mask)
 {
 	unsigned long flags;
@@ -2291,8 +2293,9 @@ void lockdep_trace_alloc(gfp_t gfp_mask)
 		return;
 
 	raw_local_irq_save(flags);
+	check_flags(flags);
 	current->lockdep_recursion = 1;
-	__lockdep_trace_alloc(gfp_mask);
+	__lockdep_trace_alloc(gfp_mask, flags);
 	current->lockdep_recursion = 0;
 	raw_local_irq_restore(flags);
 }



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [tip:core/locking] lockdep: fix deadlock in lockdep_trace_alloc, take 2
  2009-03-21 12:33   ` [PATCH] lockdep: fix deadlock in lockdep_trace_alloc -v2 Peter Zijlstra
@ 2009-03-21 16:18     ` Peter Zijlstra
  0 siblings, 0 replies; 9+ messages in thread
From: Peter Zijlstra @ 2009-03-21 16:18 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, a.p.zijlstra, tglx, mingo

Commit-ID:  da40a037c5ed51693e739ff9b21e013414244644
Gitweb:     http://git.kernel.org/tip/da40a037c5ed51693e739ff9b21e013414244644
Author:     Peter Zijlstra <a.p.zijlstra@chello.nl>
AuthorDate: Sat, 21 Mar 2009 13:33:08 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sat, 21 Mar 2009 17:15:42 +0100

lockdep: fix deadlock in lockdep_trace_alloc, take 2

Since we now disabled IRQs, checking for IRQs disabled is a bit
pointless, check for it in the saved flags.

Also, add the missing check_flags() check for completeness.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: npiggin@suse.de
Cc: heiko.carstens@de.ibm.com
LKML-Reference: <1237638788.4667.263.camel@laptop>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 kernel/lockdep.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index c750038..a288ae1 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -2257,7 +2257,7 @@ void trace_softirqs_off(unsigned long ip)
 		debug_atomic_inc(&redundant_softirqs_off);
 }
 
-static void __lockdep_trace_alloc(gfp_t gfp_mask)
+static void __lockdep_trace_alloc(gfp_t gfp_mask, unsigned long flags)
 {
 	struct task_struct *curr = current;
 
@@ -2276,12 +2276,14 @@ static void __lockdep_trace_alloc(gfp_t gfp_mask)
 	if (!(gfp_mask & __GFP_FS))
 		return;
 
-	if (DEBUG_LOCKS_WARN_ON(irqs_disabled()))
+	if (DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags)))
 		return;
 
 	mark_held_locks(curr, RECLAIM_FS);
 }
 
+static void check_flags(unsigned long flags);
+
 void lockdep_trace_alloc(gfp_t gfp_mask)
 {
 	unsigned long flags;
@@ -2290,8 +2292,9 @@ void lockdep_trace_alloc(gfp_t gfp_mask)
 		return;
 
 	raw_local_irq_save(flags);
+	check_flags(flags);
 	current->lockdep_recursion = 1;
-	__lockdep_trace_alloc(gfp_mask);
+	__lockdep_trace_alloc(gfp_mask, flags);
 	current->lockdep_recursion = 0;
 	raw_local_irq_restore(flags);
 }

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [tip:core/locking-v2] lockdep: fix deadlock in lockdep_trace_alloc
  2009-03-20 10:13 lockdep: fix deadlock in lockdep_trace_alloc Peter Zijlstra
  2009-03-20 10:26 ` [PATCH] " Peter Zijlstra
  2009-03-20 10:26 ` [tip:core/locking] " Peter Zijlstra
@ 2009-03-30 21:24 ` Peter Zijlstra
  2 siblings, 0 replies; 9+ messages in thread
From: Peter Zijlstra @ 2009-03-30 21:24 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, peterz, npiggin,
	heiko.carstens, tglx, mingo

Commit-ID:  2f8501815256af8498904e68bd0984b1afffd6f8
Gitweb:     http://git.kernel.org/tip/2f8501815256af8498904e68bd0984b1afffd6f8
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Fri, 20 Mar 2009 11:13:20 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 30 Mar 2009 23:19:24 +0200

lockdep: fix deadlock in lockdep_trace_alloc

Heiko reported that we grab the graph lock with irqs enabled.

Fix this by providng the same wrapper as all other lockdep entry
functions have.

Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Nick Piggin <npiggin@suse.de>
LKML-Reference: <1237544000.24626.52.camel@twins>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 kernel/lockdep.c |   21 +++++++++++++++++++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index 022d2ed..3673a3f 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -2260,7 +2260,7 @@ void trace_softirqs_off(unsigned long ip)
 		debug_atomic_inc(&redundant_softirqs_off);
 }
 
-void lockdep_trace_alloc(gfp_t gfp_mask)
+static void __lockdep_trace_alloc(gfp_t gfp_mask, unsigned long flags)
 {
 	struct task_struct *curr = current;
 
@@ -2279,12 +2279,29 @@ void lockdep_trace_alloc(gfp_t gfp_mask)
 	if (!(gfp_mask & __GFP_FS))
 		return;
 
-	if (DEBUG_LOCKS_WARN_ON(irqs_disabled()))
+	if (DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags)))
 		return;
 
 	mark_held_locks(curr, RECLAIM_FS);
 }
 
+static void check_flags(unsigned long flags);
+
+void lockdep_trace_alloc(gfp_t gfp_mask)
+{
+	unsigned long flags;
+
+	if (unlikely(current->lockdep_recursion))
+		return;
+
+	raw_local_irq_save(flags);
+	check_flags(flags);
+	current->lockdep_recursion = 1;
+	__lockdep_trace_alloc(gfp_mask, flags);
+	current->lockdep_recursion = 0;
+	raw_local_irq_restore(flags);
+}
+
 static int mark_irqflags(struct task_struct *curr, struct held_lock *hlock)
 {
 	/*

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2009-03-30 21:25 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-20 10:13 lockdep: fix deadlock in lockdep_trace_alloc Peter Zijlstra
2009-03-20 10:26 ` [PATCH] " Peter Zijlstra
2009-03-20 13:07   ` Heiko Carstens
2009-03-20 13:45     ` Peter Zijlstra
2009-03-20 14:00       ` Heiko Carstens
2009-03-20 10:26 ` [tip:core/locking] " Peter Zijlstra
2009-03-21 12:33   ` [PATCH] lockdep: fix deadlock in lockdep_trace_alloc -v2 Peter Zijlstra
2009-03-21 16:18     ` [tip:core/locking] lockdep: fix deadlock in lockdep_trace_alloc, take 2 Peter Zijlstra
2009-03-30 21:24 ` [tip:core/locking-v2] lockdep: fix deadlock in lockdep_trace_alloc Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox