[BUG?] possible recursive locking detected (blkdev_open)

[BUG?] possible recursive locking detected (blkdev_open)

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 1741 bytes --]

=============================================
[ INFO: possible recursive locking detected ]
---------------------------------------------
parted/7929 is trying to acquire lock:
 (&bdev->bd_mutex){--..}, at: [<c105eb8d>] __blkdev_put+0x1e/0x13c

but task is already holding lock:
 (&bdev->bd_mutex){--..}, at: [<c105eec6>] do_open+0x72/0x3a8

other info that might help us debug this:
1 lock held by parted/7929:
 #0:  (&bdev->bd_mutex){--..}, at: [<c105eec6>] do_open+0x72/0x3a8
stack backtrace:
 [<c1003aad>] show_trace_log_lvl+0x58/0x15b
 [<c100495f>] show_trace+0xd/0x10
 [<c1004979>] dump_stack+0x17/0x1a
 [<c102dee5>] __lock_acquire+0x753/0x99c
 [<c102e3b0>] lock_acquire+0x4a/0x6a
 [<c1204501>] mutex_lock_nested+0xc8/0x20c
 [<c105eb8d>] __blkdev_put+0x1e/0x13c
 [<c105ecc4>] blkdev_put+0xa/0xc
 [<c105f18a>] do_open+0x336/0x3a8
 [<c105f21b>] blkdev_open+0x1f/0x4c
 [<c1057b40>] __dentry_open+0xc7/0x1aa
 [<c1057c91>] nameidata_to_filp+0x1c/0x2e
 [<c1057cd1>] do_filp_open+0x2e/0x35
 [<c1057dd7>] do_sys_open+0x38/0x68
 [<c1057e33>] sys_open+0x16/0x18
 [<c1002845>] sysenter_past_esp+0x56/0x8d
DWARF2 unwinder stuck at sysenter_past_esp+0x56/0x8d
Leftover inexact backtrace:
 [<c100495f>] show_trace+0xd/0x10
 [<c1004979>] dump_stack+0x17/0x1a
 [<c102dee5>] __lock_acquire+0x753/0x99c
 [<c102e3b0>] lock_acquire+0x4a/0x6a
 [<c1204501>] mutex_lock_nested+0xc8/0x20c
 [<c105eb8d>] __blkdev_put+0x1e/0x13c
 [<c105ecc4>] blkdev_put+0xa/0xc
 [<c105f18a>] do_open+0x336/0x3a8
 [<c105f21b>] blkdev_open+0x1f/0x4c
 [<c1057b40>] __dentry_open+0xc7/0x1aa
 [<c1057c91>] nameidata_to_filp+0x1c/0x2e
 [<c1057cd1>] do_filp_open+0x2e/0x35
 [<c1057dd7>] do_sys_open+0x38/0x68
 [<c1057e33>] sys_open+0x16/0x18
 [<c1002845>] sysenter_past_esp+0x56/0x8d

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [BUG?] possible recursive locking detected (blkdev_open)

[Andrew Morton]

On Wed, 9 Aug 2006 07:57:31 +0200
Rolf Eike Beer <eike-kernel@sf-tec.de> wrote:

> =============================================
> [ INFO: possible recursive locking detected ]
> ---------------------------------------------
> parted/7929 is trying to acquire lock:
>  (&bdev->bd_mutex){--..}, at: [<c105eb8d>] __blkdev_put+0x1e/0x13c
> 
> but task is already holding lock:
>  (&bdev->bd_mutex){--..}, at: [<c105eec6>] do_open+0x72/0x3a8
> 
> other info that might help us debug this:
> 1 lock held by parted/7929:
>  #0:  (&bdev->bd_mutex){--..}, at: [<c105eec6>] do_open+0x72/0x3a8
> stack backtrace:
>  [<c1003aad>] show_trace_log_lvl+0x58/0x15b
>  [<c100495f>] show_trace+0xd/0x10
>  [<c1004979>] dump_stack+0x17/0x1a
>  [<c102dee5>] __lock_acquire+0x753/0x99c
>  [<c102e3b0>] lock_acquire+0x4a/0x6a
>  [<c1204501>] mutex_lock_nested+0xc8/0x20c
>  [<c105eb8d>] __blkdev_put+0x1e/0x13c
>  [<c105ecc4>] blkdev_put+0xa/0xc
>  [<c105f18a>] do_open+0x336/0x3a8
>  [<c105f21b>] blkdev_open+0x1f/0x4c
>  [<c1057b40>] __dentry_open+0xc7/0x1aa
>  [<c1057c91>] nameidata_to_filp+0x1c/0x2e
>  [<c1057cd1>] do_filp_open+0x2e/0x35
>  [<c1057dd7>] do_sys_open+0x38/0x68
>  [<c1057e33>] sys_open+0x16/0x18
>  [<c1002845>] sysenter_past_esp+0x56/0x8d
> DWARF2 unwinder stuck at sysenter_past_esp+0x56/0x8d
> Leftover inexact backtrace:
>  [<c100495f>] show_trace+0xd/0x10
>  [<c1004979>] dump_stack+0x17/0x1a
>  [<c102dee5>] __lock_acquire+0x753/0x99c
>  [<c102e3b0>] lock_acquire+0x4a/0x6a
>  [<c1204501>] mutex_lock_nested+0xc8/0x20c
>  [<c105eb8d>] __blkdev_put+0x1e/0x13c
>  [<c105ecc4>] blkdev_put+0xa/0xc
>  [<c105f18a>] do_open+0x336/0x3a8
>  [<c105f21b>] blkdev_open+0x1f/0x4c
>  [<c1057b40>] __dentry_open+0xc7/0x1aa
>  [<c1057c91>] nameidata_to_filp+0x1c/0x2e
>  [<c1057cd1>] do_filp_open+0x2e/0x35
>  [<c1057dd7>] do_sys_open+0x38/0x68
>  [<c1057e33>] sys_open+0x16/0x18
>  [<c1002845>] sysenter_past_esp+0x56/0x8d
> 

kernel version?

Re: [BUG?] possible recursive locking detected (blkdev_open)

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 570 bytes --]

Andrew Morton wrote:
> On Wed, 9 Aug 2006 07:57:31 +0200
>
> Rolf Eike Beer <eike-kernel@sf-tec.de> wrote:
> > =============================================
> > [ INFO: possible recursive locking detected ]
> > ---------------------------------------------
> > parted/7929 is trying to acquire lock:
> >  (&bdev->bd_mutex){--..}, at: [<c105eb8d>] __blkdev_put+0x1e/0x13c
> >
> > but task is already holding lock:
> >  (&bdev->bd_mutex){--..}, at: [<c105eec6>] do_open+0x72/0x3a8

> kernel version?

compiled from git 2006-08-03 16:02 (+0200), it's -rc3 and a bit.

Eike

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [BUG?] possible recursive locking detected (blkdev_open)

[Dave Jones]

On Wed, Aug 09, 2006 at 01:30:34AM -0700, Andrew Morton wrote:
 > On Wed, 9 Aug 2006 07:57:31 +0200
 > Rolf Eike Beer <eike-kernel@sf-tec.de> wrote:
 > 
 > > =============================================
 > > [ INFO: possible recursive locking detected ]
 > > ---------------------------------------------
 > 
 > kernel version?
 
This question comes up time after time when we get lockdep reports.
Lets do the same thing we do for oopses - print out the version in the report.
It's an extra line of output though.  We could tack it on the end of the
INFO: lines, but that screws up Ingo's pretty output.

Signed-off-by: Dave Jones <davej@redhat.com>


--- linux-2.6/kernel/lockdep.c~	2006-08-09 13:53:49.000000000 -0400
+++ linux-2.6/kernel/lockdep.c	2006-08-09 13:53:59.000000000 -0400
@@ -36,6 +36,7 @@
 #include <linux/stacktrace.h>
 #include <linux/debug_locks.h>
 #include <linux/irqflags.h>
+#include <linux/utsname.h>
 
 #include <asm/sections.h>
@@ -524,6 +524,9 @@ print_circular_bug_header(struct lock_li
 
 	printk("\n=======================================================\n");
 	printk(  "[ INFO: possible circular locking dependency detected ]\n");
+	printk(  "%s %.*s\n", system_utsname.release,
+		(int)strcspn(system_utsname.version, " "),
+		system_utsname.version);
 	printk(  "-------------------------------------------------------\n");
 	printk("%s/%d is trying to acquire lock:\n",
 		curr->comm, curr->pid);
@@ -705,6 +708,9 @@ print_bad_irq_dependency(struct task_str
 	printk("\n======================================================\n");
 	printk(  "[ INFO: %s-safe -> %s-unsafe lock order detected ]\n",
 		irqclass, irqclass);
+	printk(  "%s %.*s\n", system_utsname.release,
+		(int)strcspn(system_utsname.version, " "),
+		system_utsname.version);
 	printk(  "------------------------------------------------------\n");
 	printk("%s/%d [HC%u[%lu]:SC%u[%lu]:HE%u:SE%u] is trying to acquire:\n",
 		curr->comm, curr->pid,
@@ -786,6 +792,9 @@ print_deadlock_bug(struct task_struct *c
 
 	printk("\n=============================================\n");
 	printk(  "[ INFO: possible recursive locking detected ]\n");
+	printk(  "%s %.*s\n", system_utsname.release,
+		(int)strcspn(system_utsname.version, " "),
+		system_utsname.version);
 	printk(  "---------------------------------------------\n");
 	printk("%s/%d is trying to acquire lock:\n",
 		curr->comm, curr->pid);
@@ -1368,6 +1377,9 @@ print_irq_inversion_bug(struct task_stru
 
 	printk("\n=========================================================\n");
 	printk(  "[ INFO: possible irq lock inversion dependency detected ]\n");
+	printk(  "%s %.*s\n", system_utsname.release,
+		(int)strcspn(system_utsname.version, " "),
+		system_utsname.version);
 	printk(  "---------------------------------------------------------\n");
 	printk("%s/%d just changed the state of lock:\n",
 		curr->comm, curr->pid);
@@ -1462,6 +1474,9 @@ print_usage_bug(struct task_struct *curr
 
 	printk("\n=================================\n");
 	printk(  "[ INFO: inconsistent lock state ]\n");
+	printk(  "%s %.*s\n", system_utsname.release,
+		(int)strcspn(system_utsname.version, " "),
+		system_utsname.version);
 	printk(  "---------------------------------\n");
 
 	printk("inconsistent {%s} -> {%s} usage.\n",

-- 
http://www.codemonkey.org.uk

Re: [BUG?] possible recursive locking detected (blkdev_open)

[Andrew Morton]

On Wed, 9 Aug 2006 13:56:42 -0400
Dave Jones <davej@redhat.com> wrote:

> On Wed, Aug 09, 2006 at 01:30:34AM -0700, Andrew Morton wrote:
>  > On Wed, 9 Aug 2006 07:57:31 +0200
>  > Rolf Eike Beer <eike-kernel@sf-tec.de> wrote:
>  > 
>  > > =============================================
>  > > [ INFO: possible recursive locking detected ]
>  > > ---------------------------------------------
>  > 
>  > kernel version?
>  
> This question comes up time after time when we get lockdep reports.
> Lets do the same thing we do for oopses - print out the version in the report.
> It's an extra line of output though.  We could tack it on the end of the
> INFO: lines, but that screws up Ingo's pretty output.
> 
> Signed-off-by: Dave Jones <davej@redhat.com>
> 
> 
> --- linux-2.6/kernel/lockdep.c~	2006-08-09 13:53:49.000000000 -0400
> +++ linux-2.6/kernel/lockdep.c	2006-08-09 13:53:59.000000000 -0400
> @@ -36,6 +36,7 @@
>  #include <linux/stacktrace.h>
>  #include <linux/debug_locks.h>
>  #include <linux/irqflags.h>
> +#include <linux/utsname.h>
>  
>  #include <asm/sections.h>
> @@ -524,6 +524,9 @@ print_circular_bug_header(struct lock_li

hm, corrupted patch.  Needed a blank line before the @@ line,

>  	printk("\n=======================================================\n");
>  	printk(  "[ INFO: possible circular locking dependency detected ]\n");
> +	printk(  "%s %.*s\n", system_utsname.release,
> +		(int)strcspn(system_utsname.version, " "),
> +		system_utsname.version);

argh.  Every time someone adds one of these I get to go and fix up
namespaces-utsname-*.patch again.

So I did it as below:

--- a/kernel/lockdep.c~lockdep-print-kernel-version
+++ a/kernel/lockdep.c
@@ -36,6 +36,7 @@
 #include <linux/stacktrace.h>
 #include <linux/debug_locks.h>
 #include <linux/irqflags.h>
+#include <linux/utsname.h>
 
 #include <asm/sections.h>
 
@@ -508,6 +509,13 @@ print_circular_bug_entry(struct lock_lis
 	return 0;
 }
 
+static void print_kernel_version(void)
+{
+	printk("%s %.*s\n", system_utsname.release,
+		(int)strcspn(system_utsname.version, " "),
+		system_utsname.version);
+}
+
 /*
  * When a circular dependency is detected, print the
  * header first:
@@ -524,6 +532,7 @@ print_circular_bug_header(struct lock_li
 
 	printk("\n=======================================================\n");
 	printk(  "[ INFO: possible circular locking dependency detected ]\n");
+	print_kernel_version();
 	printk(  "-------------------------------------------------------\n");
 	printk("%s/%d is trying to acquire lock:\n",
 		curr->comm, curr->pid);
@@ -705,6 +714,7 @@ print_bad_irq_dependency(struct task_str
 	printk("\n======================================================\n");
 	printk(  "[ INFO: %s-safe -> %s-unsafe lock order detected ]\n",
 		irqclass, irqclass);
+	print_kernel_version();
 	printk(  "------------------------------------------------------\n");
 	printk("%s/%d [HC%u[%lu]:SC%u[%lu]:HE%u:SE%u] is trying to acquire:\n",
 		curr->comm, curr->pid,
@@ -786,6 +796,7 @@ print_deadlock_bug(struct task_struct *c
 
 	printk("\n=============================================\n");
 	printk(  "[ INFO: possible recursive locking detected ]\n");
+	print_kernel_version();
 	printk(  "---------------------------------------------\n");
 	printk("%s/%d is trying to acquire lock:\n",
 		curr->comm, curr->pid);
@@ -1368,6 +1379,7 @@ print_irq_inversion_bug(struct task_stru
 
 	printk("\n=========================================================\n");
 	printk(  "[ INFO: possible irq lock inversion dependency detected ]\n");
+	print_kernel_version();
 	printk(  "---------------------------------------------------------\n");
 	printk("%s/%d just changed the state of lock:\n",
 		curr->comm, curr->pid);
@@ -1462,6 +1474,7 @@ print_usage_bug(struct task_struct *curr
 
 	printk("\n=================================\n");
 	printk(  "[ INFO: inconsistent lock state ]\n");
+	print_kernel_version();
 	printk(  "---------------------------------\n");
 
 	printk("inconsistent {%s} -> {%s} usage.\n",
_

Re: [BUG?] possible recursive locking detected (blkdev_open)

[Peter Zijlstra]

(partial CC list from commit 663d440eaa496db903cc58be04b9b602ba45e43b)

On Wed, 2006-08-09 at 07:57 +0200, Rolf Eike Beer wrote:
> =============================================
> [ INFO: possible recursive locking detected ]
> ---------------------------------------------
> parted/7929 is trying to acquire lock:
>  (&bdev->bd_mutex){--..}, at: [<c105eb8d>] __blkdev_put+0x1e/0x13c
> 
> but task is already holding lock:
>  (&bdev->bd_mutex){--..}, at: [<c105eec6>] do_open+0x72/0x3a8
> 
> other info that might help us debug this:
> 1 lock held by parted/7929:
>  #0:  (&bdev->bd_mutex){--..}, at: [<c105eec6>] do_open+0x72/0x3a8
> stack backtrace:
>  [<c1003aad>] show_trace_log_lvl+0x58/0x15b
>  [<c100495f>] show_trace+0xd/0x10
>  [<c1004979>] dump_stack+0x17/0x1a
>  [<c102dee5>] __lock_acquire+0x753/0x99c
>  [<c102e3b0>] lock_acquire+0x4a/0x6a
>  [<c1204501>] mutex_lock_nested+0xc8/0x20c
>  [<c105eb8d>] __blkdev_put+0x1e/0x13c
>  [<c105ecc4>] blkdev_put+0xa/0xc
>  [<c105f18a>] do_open+0x336/0x3a8
>  [<c105f21b>] blkdev_open+0x1f/0x4c
>  [<c1057b40>] __dentry_open+0xc7/0x1aa
>  [<c1057c91>] nameidata_to_filp+0x1c/0x2e
>  [<c1057cd1>] do_filp_open+0x2e/0x35
>  [<c1057dd7>] do_sys_open+0x38/0x68
>  [<c1057e33>] sys_open+0x16/0x18
>  [<c1002845>] sysenter_past_esp+0x56/0x8d

OK, I'm having a look here; its all new to me so bear with me.

blkdev_open() calls
  do_open(bdev, ...,BD_MUTEX_NORMAL) and takes
    mutex_lock_nested(&bdev->bd_mutex, BD_MUTEX_NORMAL)
    
then something fails, and we're thrown to:

out_first: where
    if (bdev != bdev->bd_contains)
      blkdev_put(bdev->bd_contains) which is
        __blkdev_put(bdev->bd_contains, BD_MUTEX_NORMAL) which does
          mutex_lock_nested(&bdev->bd_contains->bd_mutex, BD_MUTEX_NORMAL) <--- lockdep trigger

When going to out_first, dbev->bd_contains is either bdev or whole, and
since we take the branch it must be whole. So it seems to me the
following patch would be the right one:

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 fs/block_dev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/fs/block_dev.c
===================================================================
--- linux-2.6.orig/fs/block_dev.c
+++ linux-2.6/fs/block_dev.c
@@ -980,7 +980,7 @@ out_first:
 	bdev->bd_disk = NULL;
 	bdev->bd_inode->i_data.backing_dev_info = &default_backing_dev_info;
 	if (bdev != bdev->bd_contains)
-		blkdev_put(bdev->bd_contains);
+		__blkdev_put(bdev->bd_contains, BD_MUTEX_WHOLE);
 	bdev->bd_contains = NULL;
 	put_disk(disk);
 	module_put(owner);

Re: [BUG?] possible recursive locking detected (blkdev_open)

[Neil Brown]

On Friday August 18, a.p.zijlstra@chello.nl wrote:
> 
> blkdev_open() calls
>   do_open(bdev, ...,BD_MUTEX_NORMAL) and takes
>     mutex_lock_nested(&bdev->bd_mutex, BD_MUTEX_NORMAL)
>     
> then something fails, and we're thrown to:
> 
> out_first: where
>     if (bdev != bdev->bd_contains)
>       blkdev_put(bdev->bd_contains) which is
>         __blkdev_put(bdev->bd_contains, BD_MUTEX_NORMAL) which does
>           mutex_lock_nested(&bdev->bd_contains->bd_mutex, BD_MUTEX_NORMAL) <--- lockdep trigger
> 
> When going to out_first, dbev->bd_contains is either bdev or whole, and
> since we take the branch it must be whole. So it seems to me the
> following patch would be the right one:

Looks sensible to me.

> 
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Acked-by: NeilBrown <neilb@suse.de>

NeilBrown

> ---
>  fs/block_dev.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-2.6/fs/block_dev.c
> ===================================================================
> --- linux-2.6.orig/fs/block_dev.c
> +++ linux-2.6/fs/block_dev.c
> @@ -980,7 +980,7 @@ out_first:
>  	bdev->bd_disk = NULL;
>  	bdev->bd_inode->i_data.backing_dev_info = &default_backing_dev_info;
>  	if (bdev != bdev->bd_contains)
> -		blkdev_put(bdev->bd_contains);
> +		__blkdev_put(bdev->bd_contains, BD_MUTEX_WHOLE);
>  	bdev->bd_contains = NULL;
>  	put_disk(disk);
>  	module_put(owner);
> 
> 
> 
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/