xfs deadlock on 3.9-rc5 running xfstests case #78

xfs deadlock on 3.9-rc5 running xfstests case #78

[CAI Qian]

Saw on almost all the servers range from x64, ppc64 and s390x with kernel
3.9-rc5 and xfsprogs-3.1.10. Never caught this in 3.9-rc4, so looks like
something new broke this. Log is here with sysrq debug info.
http://people.redhat.com/qcai/stable/log

CAI Qian

Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]

[Dave Chinner]

[Added jens Axboe to CC]

On Tue, Apr 02, 2013 at 02:08:49AM -0400, CAI Qian wrote:
> Saw on almost all the servers range from x64, ppc64 and s390x with kernel
> 3.9-rc5 and xfsprogs-3.1.10. Never caught this in 3.9-rc4, so looks like
> something new broke this. Log is here with sysrq debug info.
> http://people.redhat.com/qcai/stable/log

Has nothing to do with XFS:

[34762.105676] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. 
[34762.152871] systemd-udevd   D ffff88030bc94440     0  4391    412 0x00000084 
[34762.196660]  ffff8801134d9a98 0000000000000082 ffff880303e09ac0 ffff8801134d9fd8 
[34762.240512]  ffff8801134d9fd8 ffff8801134d9fd8 ffff880304f98000 ffff880303e09ac0 
[34762.287047]  ffff8802936c1ac0 ffff88060521f998 ffff88060521f99c ffff880303e09ac0 
[34762.331446] Call Trace: 
[34762.345444]  [<ffffffff816274a9>] schedule+0x29/0x70 
[34762.373908]  [<ffffffff8162777e>] schedule_preempt_disabled+0xe/0x10 
[34762.411394]  [<ffffffff81625ae3>] __mutex_lock_slowpath+0xc3/0x140 
[34762.447597]  [<ffffffff8162561a>] mutex_lock+0x2a/0x50 
[34762.476785]  [<ffffffff811daddb>] __blkdev_get+0x6b/0x4b0 
[34762.508839]  [<ffffffff812a619a>] ? selinux_file_alloc_security+0x4a/0x80 
[34762.546436]  [<ffffffff811db3bd>] blkdev_get+0x19d/0x2e0 
[34762.580977]  [<ffffffff812a18fa>] ? inode_has_perm.isra.31.constprop.61+0x2a/0x30 
[34762.624346]  [<ffffffff811db55f>] blkdev_open+0x5f/0x90 
[34762.656318]  [<ffffffff811a00bf>] do_dentry_open+0x20f/0x2c0 
[34762.689762]  [<ffffffff811db500>] ? blkdev_get+0x2e0/0x2e0 
[34762.722221]  [<ffffffff811a01a5>] finish_open+0x35/0x50 
[34762.754363]  [<ffffffff811b082e>] do_last+0x6de/0xde0 
[34762.783808]  [<ffffffff811ad378>] ? inode_permission+0x18/0x50 
[34762.819265]  [<ffffffff811ad428>] ? link_path_walk+0x78/0x880 
[34762.853700]  [<ffffffff812a619a>] ? selinux_file_alloc_security+0x4a/0x80 
[34762.892881]  [<ffffffff811b0fe7>] path_openat+0xb7/0x4b0 
[34762.923621]  [<ffffffff811ad0e0>] ? getname_flags.part.33+0x30/0x150 
[34762.960839]  [<ffffffff811b16a1>] do_filp_open+0x41/0xa0 
[34762.992114]  [<ffffffff811bddc2>] ? __alloc_fd+0x42/0x110 
[34763.023342]  [<ffffffff811a14f4>] do_sys_open+0xf4/0x1e0 
[34763.054129]  [<ffffffff811a1601>] sys_open+0x21/0x30 
[34763.082134]  [<ffffffff81630d59>] system_call_fastpath+0x16/0x1b 


And:

[34763.116218] INFO: task umount:4421 blocked for more than 120 seconds. 
[34763.153670] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. 
[34763.200321] umount          D ffff88030bcb4440     0  4421    452 0x00000080 
[34763.242026]  ffff8801134fbcb8 0000000000000082 ffff8802936c1ac0 ffff8801134fbfd8 
[34763.287320]  ffff8801134fbfd8 ffff8801134fbfd8 ffff8803069d0000 ffff8802936c1ac0 
[34763.330722]  ffff8802936c1ac0 ffff88060521f998 ffff88060521f99c ffff8802936c1ac0 
[34763.374428] Call Trace: 
[34763.388977]  [<ffffffff816274a9>] schedule+0x29/0x70 
[34763.419164]  [<ffffffff8162777e>] schedule_preempt_disabled+0xe/0x10 
[34763.456279]  [<ffffffff81625ae3>] __mutex_lock_slowpath+0xc3/0x140 
[34763.492474]  [<ffffffff8162561a>] mutex_lock+0x2a/0x50 
[34763.520840]  [<ffffffff81417da5>] loop_clr_fd+0x285/0x480 
[34763.552050]  [<ffffffff81418800>] lo_release+0x70/0x80 
[34763.580787]  [<ffffffff811dabac>] __blkdev_put+0x17c/0x1d0 
[34763.612479]  [<ffffffff811dac57>] blkdev_put+0x57/0x140 
[34763.641557]  [<ffffffff811a409d>] kill_block_super+0x4d/0x80 
[34763.674257]  [<ffffffff811a4467>] deactivate_locked_super+0x57/0x80 
[34763.708755]  [<ffffffff811a4fee>] deactivate_super+0x4e/0x70 
[34763.742906]  [<ffffffff811bfbb7>] mntput_no_expire+0xd7/0x130 
[34763.776872]  [<ffffffff811c0a9c>] sys_umount+0x9c/0x3c0 
[34763.811819]  [<ffffffff81630d59>] system_call_fastpath+0x16/0x1b 


It's clearly a loopback device problem, stuck on the
bdev->bd_inode->i_mutex. And there's changesin the loop device
teardown since 3.9-rc4:

$ glo v3.9-rc4..HEAD -- drivers/block/loop.c  
c1681bf loop: prevent bdev freeing while device in use
8761a3d loop: cleanup partitions when detaching loop device
183cfb5 loop: fix error return code in loop_add()
$

So this looks like someone hasn't been testing their loopback
driver changes properly...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]

[Jens Axboe]

On Tue, Apr 02 2013, Dave Chinner wrote:
> [Added jens Axboe to CC]
> 
> On Tue, Apr 02, 2013 at 02:08:49AM -0400, CAI Qian wrote:
> > Saw on almost all the servers range from x64, ppc64 and s390x with kernel
> > 3.9-rc5 and xfsprogs-3.1.10. Never caught this in 3.9-rc4, so looks like
> > something new broke this. Log is here with sysrq debug info.
> > http://people.redhat.com/qcai/stable/log

CAI Qian, can you try and back the below out and test again?


commit 8761a3dc1f07b163414e2215a2cadbb4cfe2a107
Author: Phillip Susi <psusi@ubuntu.com>
Date:   Fri Mar 22 12:21:53 2013 -0600

    loop: cleanup partitions when detaching loop device
    
    Any partitions added by user space to the loop device were being
    left in place after detaching the loop device.  This was because
    the detach path issued a BLKRRPART to clean up partitions if
    LO_FLAGS_PARTSCAN was set, meaning that the partitions were auto
    scanned on attach.  Replace this BLKRRPART with code that
    unconditionally cleans up partitions on detach instead.
    
    Signed-off-by: Phillip Susi <psusi@ubuntu.com>
    
    Modified by Jens to export delete_partition().
    
    Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/block/partition-generic.c b/block/partition-generic.c
index 789cdea..ae95ee6 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -257,6 +257,7 @@ void delete_partition(struct gendisk *disk, int partno)
 
 	hd_struct_put(part);
 }
+EXPORT_SYMBOL(delete_partition);
 
 static ssize_t whole_disk_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index ee13a82..fe5f640 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1044,12 +1044,29 @@ static int loop_clr_fd(struct loop_device *lo)
 	lo->lo_state = Lo_unbound;
 	/* This is safe: open() is still holding a reference. */
 	module_put(THIS_MODULE);
-	if (lo->lo_flags & LO_FLAGS_PARTSCAN && bdev)
-		ioctl_by_bdev(bdev, BLKRRPART, 0);
 	lo->lo_flags = 0;
 	if (!part_shift)
 		lo->lo_disk->flags |= GENHD_FL_NO_PART_SCAN;
 	mutex_unlock(&lo->lo_ctl_mutex);
+
+	/*
+	 * Remove all partitions, since BLKRRPART won't remove user
+	 * added partitions when max_part=0
+	 */
+	if (bdev) {
+		struct disk_part_iter piter;
+		struct hd_struct *part;
+
+		mutex_lock_nested(&bdev->bd_mutex, 1);
+		invalidate_partition(bdev->bd_disk, 0);
+		disk_part_iter_init(&piter, bdev->bd_disk,
+					DISK_PITER_INCL_EMPTY);
+		while ((part = disk_part_iter_next(&piter)))
+			delete_partition(bdev->bd_disk, part->partno);
+		disk_part_iter_exit(&piter);
+		mutex_unlock(&bdev->bd_mutex);
+	}
+
 	/*
 	 * Need not hold lo_ctl_mutex to fput backing file.
 	 * Calling fput holding lo_ctl_mutex triggers a circular

-- 
Jens Axboe

Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]

[Jens Axboe]

On Tue, Apr 02 2013, Jens Axboe wrote:
> On Tue, Apr 02 2013, Dave Chinner wrote:
> > [Added jens Axboe to CC]
> > 
> > On Tue, Apr 02, 2013 at 02:08:49AM -0400, CAI Qian wrote:
> > > Saw on almost all the servers range from x64, ppc64 and s390x with kernel
> > > 3.9-rc5 and xfsprogs-3.1.10. Never caught this in 3.9-rc4, so looks like
> > > something new broke this. Log is here with sysrq debug info.
> > > http://people.redhat.com/qcai/stable/log
> 
> CAI Qian, can you try and back the below out and test again?

Nevermind, it's clearly that one. The below should improve the
situation, but it's not pretty. A better fix would be to allow
auto-deletion even if PART_NO_SCAN is set.

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index fe5f640..d6c5764 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1057,14 +1057,15 @@ static int loop_clr_fd(struct loop_device *lo)
 		struct disk_part_iter piter;
 		struct hd_struct *part;
 
-		mutex_lock_nested(&bdev->bd_mutex, 1);
-		invalidate_partition(bdev->bd_disk, 0);
-		disk_part_iter_init(&piter, bdev->bd_disk,
-					DISK_PITER_INCL_EMPTY);
-		while ((part = disk_part_iter_next(&piter)))
-			delete_partition(bdev->bd_disk, part->partno);
-		disk_part_iter_exit(&piter);
-		mutex_unlock(&bdev->bd_mutex);
+		if (mutex_trylock(&bdev->bd_mutex, 1))
+			invalidate_partition(bdev->bd_disk, 0);
+			disk_part_iter_init(&piter, bdev->bd_disk,
+						DISK_PITER_INCL_EMPTY);
+			while ((part = disk_part_iter_next(&piter)))
+				delete_partition(bdev->bd_disk, part->partno);
+			disk_part_iter_exit(&piter);
+			mutex_unlock(&bdev->bd_mutex);
+		}
 	}
 
 	/*

-- 
Jens Axboe

Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]

[CAI Qian]

----- Original Message -----
> From: "Jens Axboe" <axboe@kernel.dk>
> To: "Dave Chinner" <david@fromorbit.com>
> Cc: "CAI Qian" <caiqian@redhat.com>, xfs@oss.sgi.com, "LKML" <linux-kernel@vger.kernel.org>
> Sent: Tuesday, April 2, 2013 3:30:35 PM
> Subject: Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]
> 
> On Tue, Apr 02 2013, Jens Axboe wrote:
> > On Tue, Apr 02 2013, Dave Chinner wrote:
> > > [Added jens Axboe to CC]
> > > 
> > > On Tue, Apr 02, 2013 at 02:08:49AM -0400, CAI Qian wrote:
> > > > Saw on almost all the servers range from x64, ppc64 and s390x with
> > > > kernel
> > > > 3.9-rc5 and xfsprogs-3.1.10. Never caught this in 3.9-rc4, so looks
> > > > like
> > > > something new broke this. Log is here with sysrq debug info.
> > > > http://people.redhat.com/qcai/stable/log
> > 
> > CAI Qian, can you try and back the below out and test again?
> 
> Nevermind, it's clearly that one. The below should improve the
> situation, but it's not pretty. A better fix would be to allow
> auto-deletion even if PART_NO_SCAN is set.
Jens, when compiled the mainline (up to fefcdbe) with this patch,
it error-ed out,

drivers/block/loop.c: In function ‘loop_clr_fd’:
drivers/block/loop.c:1067:3: error: too many arguments to function ‘mutex_trylock’
In file included from include/linux/notifier.h:13:0,
                 from include/linux/memory_hotplug.h:6,
                 from include/linux/mmzone.h:771,
                 from include/linux/gfp.h:4,
                 from include/linux/kmod.h:22,
                 from include/linux/module.h:13,
                 from drivers/block/loop.c:52:
include/linux/mutex.h:168:12: note: declared here
drivers/block/loop.c: At top level:
drivers/block/loop.c:1084:2: warning: data definition has no type or storage class [enabled by default]
drivers/block/loop.c:1084:2: warning: type defaults to ‘int’ in declaration of ‘fput’ [-Wimplicit-int]
drivers/block/loop.c:1084:2: warning: parameter names (without types) in function declaration [enabled by default]
drivers/block/loop.c:1084:2: error: conflicting types for ‘fput’
In file included from drivers/block/loop.c:56:0:
include/linux/file.h:14:13: note: previous declaration of ‘fput’ was here
drivers/block/loop.c:1085:2: error: expected identifier or ‘(’ before ‘return’
drivers/block/loop.c:1086:1: error: expected identifier or ‘(’ before ‘}’ token
  CC      crypto/gf128mul.o
  CC      lib/sort.o
drivers/block/loop.c: In function ‘loop_clr_fd’:
drivers/block/loop.c:1076:2: warning: control reaches end of non-void function [-Wreturn-type]
  CC      lib/parser.o
  CC [M]  sound/pci/atiixp.o
make[2]: *** [drivers/block/loop.o] Error 1

CAI Qian
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index fe5f640..d6c5764 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -1057,14 +1057,15 @@ static int loop_clr_fd(struct loop_device *lo)
>  		struct disk_part_iter piter;
>  		struct hd_struct *part;
>  
> -		mutex_lock_nested(&bdev->bd_mutex, 1);
> -		invalidate_partition(bdev->bd_disk, 0);
> -		disk_part_iter_init(&piter, bdev->bd_disk,
> -					DISK_PITER_INCL_EMPTY);
> -		while ((part = disk_part_iter_next(&piter)))
> -			delete_partition(bdev->bd_disk, part->partno);
> -		disk_part_iter_exit(&piter);
> -		mutex_unlock(&bdev->bd_mutex);
> +		if (mutex_trylock(&bdev->bd_mutex, 1))
> +			invalidate_partition(bdev->bd_disk, 0);
> +			disk_part_iter_init(&piter, bdev->bd_disk,
> +						DISK_PITER_INCL_EMPTY);
> +			while ((part = disk_part_iter_next(&piter)))
> +				delete_partition(bdev->bd_disk, part->partno);
> +			disk_part_iter_exit(&piter);
> +			mutex_unlock(&bdev->bd_mutex);
> +		}
>  	}
>  
>  	/*
> 
> --
> Jens Axboe
> 
> 

Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]

[Jens Axboe]

On Tue, Apr 02 2013, CAI Qian wrote:
> 
> 
> ----- Original Message -----
> > From: "Jens Axboe" <axboe@kernel.dk>
> > To: "Dave Chinner" <david@fromorbit.com>
> > Cc: "CAI Qian" <caiqian@redhat.com>, xfs@oss.sgi.com, "LKML" <linux-kernel@vger.kernel.org>
> > Sent: Tuesday, April 2, 2013 3:30:35 PM
> > Subject: Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]
> > 
> > On Tue, Apr 02 2013, Jens Axboe wrote:
> > > On Tue, Apr 02 2013, Dave Chinner wrote:
> > > > [Added jens Axboe to CC]
> > > > 
> > > > On Tue, Apr 02, 2013 at 02:08:49AM -0400, CAI Qian wrote:
> > > > > Saw on almost all the servers range from x64, ppc64 and s390x with
> > > > > kernel
> > > > > 3.9-rc5 and xfsprogs-3.1.10. Never caught this in 3.9-rc4, so looks
> > > > > like
> > > > > something new broke this. Log is here with sysrq debug info.
> > > > > http://people.redhat.com/qcai/stable/log
> > > 
> > > CAI Qian, can you try and back the below out and test again?
> > 
> > Nevermind, it's clearly that one. The below should improve the
> > situation, but it's not pretty. A better fix would be to allow
> > auto-deletion even if PART_NO_SCAN is set.
> Jens, when compiled the mainline (up to fefcdbe) with this patch,
> it error-ed out,

Looks like I sent the wrong one, updated below.

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index fe5f640..faa3afa 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1057,14 +1057,15 @@ static int loop_clr_fd(struct loop_device *lo)
 		struct disk_part_iter piter;
 		struct hd_struct *part;
 
-		mutex_lock_nested(&bdev->bd_mutex, 1);
-		invalidate_partition(bdev->bd_disk, 0);
-		disk_part_iter_init(&piter, bdev->bd_disk,
-					DISK_PITER_INCL_EMPTY);
-		while ((part = disk_part_iter_next(&piter)))
-			delete_partition(bdev->bd_disk, part->partno);
-		disk_part_iter_exit(&piter);
-		mutex_unlock(&bdev->bd_mutex);
+		if (mutex_trylock(&bdev->bd_mutex)) {
+			invalidate_partition(bdev->bd_disk, 0);
+			disk_part_iter_init(&piter, bdev->bd_disk,
+						DISK_PITER_INCL_EMPTY);
+			while ((part = disk_part_iter_next(&piter)))
+				delete_partition(bdev->bd_disk, part->partno);
+			disk_part_iter_exit(&piter);
+			mutex_unlock(&bdev->bd_mutex);
+		}
 	}
 
 	/*

-- 
Jens Axboe

Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]

[CAI Qian]

----- Original Message -----
> From: "Jens Axboe" <axboe@kernel.dk>
> To: "CAI Qian" <caiqian@redhat.com>
> Cc: "Dave Chinner" <david@fromorbit.com>, xfs@oss.sgi.com, "LKML" <linux-kernel@vger.kernel.org>
> Sent: Tuesday, April 2, 2013 5:00:47 PM
> Subject: Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]
> 
> On Tue, Apr 02 2013, CAI Qian wrote:
> > 
> > 
> > ----- Original Message -----
> > > From: "Jens Axboe" <axboe@kernel.dk>
> > > To: "Dave Chinner" <david@fromorbit.com>
> > > Cc: "CAI Qian" <caiqian@redhat.com>, xfs@oss.sgi.com, "LKML"
> > > <linux-kernel@vger.kernel.org>
> > > Sent: Tuesday, April 2, 2013 3:30:35 PM
> > > Subject: Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5
> > > running xfstests case #78]
> > > 
> > > On Tue, Apr 02 2013, Jens Axboe wrote:
> > > > On Tue, Apr 02 2013, Dave Chinner wrote:
> > > > > [Added jens Axboe to CC]
> > > > > 
> > > > > On Tue, Apr 02, 2013 at 02:08:49AM -0400, CAI Qian wrote:
> > > > > > Saw on almost all the servers range from x64, ppc64 and s390x with
> > > > > > kernel
> > > > > > 3.9-rc5 and xfsprogs-3.1.10. Never caught this in 3.9-rc4, so looks
> > > > > > like
> > > > > > something new broke this. Log is here with sysrq debug info.
> > > > > > http://people.redhat.com/qcai/stable/log
> > > > 
> > > > CAI Qian, can you try and back the below out and test again?
> > > 
> > > Nevermind, it's clearly that one. The below should improve the
> > > situation, but it's not pretty. A better fix would be to allow
> > > auto-deletion even if PART_NO_SCAN is set.
> > Jens, when compiled the mainline (up to fefcdbe) with this patch,
> > it error-ed out,
> 
> Looks like I sent the wrong one, updated below.
The patch works well. Thanks!
CAI Qian
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index fe5f640..faa3afa 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -1057,14 +1057,15 @@ static int loop_clr_fd(struct loop_device *lo)
>  		struct disk_part_iter piter;
>  		struct hd_struct *part;
>  
> -		mutex_lock_nested(&bdev->bd_mutex, 1);
> -		invalidate_partition(bdev->bd_disk, 0);
> -		disk_part_iter_init(&piter, bdev->bd_disk,
> -					DISK_PITER_INCL_EMPTY);
> -		while ((part = disk_part_iter_next(&piter)))
> -			delete_partition(bdev->bd_disk, part->partno);
> -		disk_part_iter_exit(&piter);
> -		mutex_unlock(&bdev->bd_mutex);
> +		if (mutex_trylock(&bdev->bd_mutex)) {
> +			invalidate_partition(bdev->bd_disk, 0);
> +			disk_part_iter_init(&piter, bdev->bd_disk,
> +						DISK_PITER_INCL_EMPTY);
> +			while ((part = disk_part_iter_next(&piter)))
> +				delete_partition(bdev->bd_disk, part->partno);
> +			disk_part_iter_exit(&piter);
> +			mutex_unlock(&bdev->bd_mutex);
> +		}
>  	}
>  
>  	/*
> 
> --
> Jens Axboe
> 
> 

Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]

[Jens Axboe]

On Tue, Apr 02 2013, CAI Qian wrote:
> 
> 
> ----- Original Message -----
> > From: "Jens Axboe" <axboe@kernel.dk>
> > To: "CAI Qian" <caiqian@redhat.com>
> > Cc: "Dave Chinner" <david@fromorbit.com>, xfs@oss.sgi.com, "LKML" <linux-kernel@vger.kernel.org>
> > Sent: Tuesday, April 2, 2013 5:00:47 PM
> > Subject: Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]
> > 
> > On Tue, Apr 02 2013, CAI Qian wrote:
> > > 
> > > 
> > > ----- Original Message -----
> > > > From: "Jens Axboe" <axboe@kernel.dk>
> > > > To: "Dave Chinner" <david@fromorbit.com>
> > > > Cc: "CAI Qian" <caiqian@redhat.com>, xfs@oss.sgi.com, "LKML"
> > > > <linux-kernel@vger.kernel.org>
> > > > Sent: Tuesday, April 2, 2013 3:30:35 PM
> > > > Subject: Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5
> > > > running xfstests case #78]
> > > > 
> > > > On Tue, Apr 02 2013, Jens Axboe wrote:
> > > > > On Tue, Apr 02 2013, Dave Chinner wrote:
> > > > > > [Added jens Axboe to CC]
> > > > > > 
> > > > > > On Tue, Apr 02, 2013 at 02:08:49AM -0400, CAI Qian wrote:
> > > > > > > Saw on almost all the servers range from x64, ppc64 and s390x with
> > > > > > > kernel
> > > > > > > 3.9-rc5 and xfsprogs-3.1.10. Never caught this in 3.9-rc4, so looks
> > > > > > > like
> > > > > > > something new broke this. Log is here with sysrq debug info.
> > > > > > > http://people.redhat.com/qcai/stable/log
> > > > > 
> > > > > CAI Qian, can you try and back the below out and test again?
> > > > 
> > > > Nevermind, it's clearly that one. The below should improve the
> > > > situation, but it's not pretty. A better fix would be to allow
> > > > auto-deletion even if PART_NO_SCAN is set.
> > > Jens, when compiled the mainline (up to fefcdbe) with this patch,
> > > it error-ed out,
> > 
> > Looks like I sent the wrong one, updated below.
> The patch works well. Thanks!

Thanks for testing! I don't particularly like this stuff in loop,
though. It's quite nasty and depends on other behaviour. It would be
prettier if we just had rescan_partitions() do the right thing, and only
drop partitions and not rescan if NO_PART_SCAN is set.

Ala the below, dropping the loop change and implementing that change in
the core code. Phillip, can you check whether this does the right thing
for your bug too?

diff --git a/block/ioctl.c b/block/ioctl.c
index a31d91d..8b78b5a 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -155,7 +155,7 @@ static int blkdev_reread_part(struct block_device *bdev)
 	struct gendisk *disk = bdev->bd_disk;
 	int res;
 
-	if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains)
+	if (bdev != bdev->bd_contains)
 		return -EINVAL;
 	if (!capable(CAP_SYS_ADMIN))
 		return -EACCES;
diff --git a/block/partition-generic.c b/block/partition-generic.c
index ae95ee6..bf4bb60 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -431,6 +431,15 @@ rescan:
 		disk->fops->revalidate_disk(disk);
 	check_disk_size_change(disk, bdev);
 	bdev->bd_invalidated = 0;
+
+	/*
+	 * If partition scanning is disabled, we are done.
+	 */
+	if (!disk_part_scan_enabled(disk)) {
+		kobject_uevent(&disk_to_dev(disk)->kobj, KOBJ_CHANGE);
+		return 0;
+	}
+
 	if (!get_capacity(disk) || !(state = check_partition(disk, bdev)))
 		return 0;
 	if (IS_ERR(state)) {
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 2c127f9..8b6df76 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1057,24 +1057,6 @@ static int loop_clr_fd(struct loop_device *lo)
 	mutex_unlock(&lo->lo_ctl_mutex);
 
 	/*
-	 * Remove all partitions, since BLKRRPART won't remove user
-	 * added partitions when max_part=0
-	 */
-	if (bdev) {
-		struct disk_part_iter piter;
-		struct hd_struct *part;
-
-		mutex_lock_nested(&bdev->bd_mutex, 1);
-		invalidate_partition(bdev->bd_disk, 0);
-		disk_part_iter_init(&piter, bdev->bd_disk,
-					DISK_PITER_INCL_EMPTY);
-		while ((part = disk_part_iter_next(&piter)))
-			delete_partition(bdev->bd_disk, part->partno);
-		disk_part_iter_exit(&piter);
-		mutex_unlock(&bdev->bd_mutex);
-	}
-
-	/*
 	 * Need not hold lo_ctl_mutex to fput backing file.
 	 * Calling fput holding lo_ctl_mutex triggers a circular
 	 * lock dependency possibility warning as fput can take

-- 
Jens Axboe

Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]

[Jens Axboe]

On Tue, Apr 02 2013, Jens Axboe wrote:
> On Tue, Apr 02 2013, CAI Qian wrote:
> > 
> > 
> > ----- Original Message -----
> > > From: "Jens Axboe" <axboe@kernel.dk>
> > > To: "CAI Qian" <caiqian@redhat.com>
> > > Cc: "Dave Chinner" <david@fromorbit.com>, xfs@oss.sgi.com, "LKML" <linux-kernel@vger.kernel.org>
> > > Sent: Tuesday, April 2, 2013 5:00:47 PM
> > > Subject: Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]
> > > 
> > > On Tue, Apr 02 2013, CAI Qian wrote:
> > > > 
> > > > 
> > > > ----- Original Message -----
> > > > > From: "Jens Axboe" <axboe@kernel.dk>
> > > > > To: "Dave Chinner" <david@fromorbit.com>
> > > > > Cc: "CAI Qian" <caiqian@redhat.com>, xfs@oss.sgi.com, "LKML"
> > > > > <linux-kernel@vger.kernel.org>
> > > > > Sent: Tuesday, April 2, 2013 3:30:35 PM
> > > > > Subject: Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5
> > > > > running xfstests case #78]
> > > > > 
> > > > > On Tue, Apr 02 2013, Jens Axboe wrote:
> > > > > > On Tue, Apr 02 2013, Dave Chinner wrote:
> > > > > > > [Added jens Axboe to CC]
> > > > > > > 
> > > > > > > On Tue, Apr 02, 2013 at 02:08:49AM -0400, CAI Qian wrote:
> > > > > > > > Saw on almost all the servers range from x64, ppc64 and s390x with
> > > > > > > > kernel
> > > > > > > > 3.9-rc5 and xfsprogs-3.1.10. Never caught this in 3.9-rc4, so looks
> > > > > > > > like
> > > > > > > > something new broke this. Log is here with sysrq debug info.
> > > > > > > > http://people.redhat.com/qcai/stable/log
> > > > > > 
> > > > > > CAI Qian, can you try and back the below out and test again?
> > > > > 
> > > > > Nevermind, it's clearly that one. The below should improve the
> > > > > situation, but it's not pretty. A better fix would be to allow
> > > > > auto-deletion even if PART_NO_SCAN is set.
> > > > Jens, when compiled the mainline (up to fefcdbe) with this patch,
> > > > it error-ed out,
> > > 
> > > Looks like I sent the wrong one, updated below.
> > The patch works well. Thanks!
> 
> Thanks for testing! I don't particularly like this stuff in loop,
> though. It's quite nasty and depends on other behaviour. It would be
> prettier if we just had rescan_partitions() do the right thing, and only
> drop partitions and not rescan if NO_PART_SCAN is set.
> 
> Ala the below, dropping the loop change and implementing that change in
> the core code. Phillip, can you check whether this does the right thing
> for your bug too?

Phillip? I'm going to revert the loop change asap, so if you want this
fixed for 3.10, it's about that time to test it out.

-- 
Jens Axboe

Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 4/3/2013 7:41 AM, Jens Axboe wrote:
>> Thanks for testing! I don't particularly like this stuff in
>> loop, though. It's quite nasty and depends on other behaviour. It
>> would be prettier if we just had rescan_partitions() do the right
>> thing, and only drop partitions and not rescan if NO_PART_SCAN is
>> set.
>> 
>> Ala the below, dropping the loop change and implementing that
>> change in the core code. Phillip, can you check whether this does
>> the right thing for your bug too?
> 
> Phillip? I'm going to revert the loop change asap, so if you want
> this fixed for 3.10, it's about that time to test it out.

I have not tested it yet, but I am pretty sure it won't work.  It
looks like the patch changes the BLKRRPART path to go ahead and remove
existing partitions when GENHD_FL_NO_PARTSCAN is set.  loop doesn't
issue the BLKRRPART ioctl when !LO_FLAGS_PARTSCAN so this won't help.
 I think loop needs to set GENHD_FL_NO_PARTSCAN and then issue the
ioctl regardless of the LO_FLAGS_PARTSCAN flag to get the partitions
to be removed.  I will try to test tonight.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJRXE2dAAoJEJrBOlT6nu75PM0IAIxVmuHdxLPtdtUNPqkU2a1r
QanHb6F43qSbd7l37XlwYgzUlybVlntf1yvKGzh29g3QM0603sFqV1o+mbXd5LI3
b+I5QrQJh90Vou9oVSAxz1Ps/AlZvxVIDv8bRwNhpXcMmaj0EN5R+6pU5L7KU2BU
GFsvajssedFh3XnNskgkR3XlqevI7U7A8VqLRsswl7FJVu7R1s45xP/sQgBWgiUS
P5viykwhje4OTKmu0D7bFKrOVx6O3gK7IHzdOwwT9aWRxuxL+Y9yfBF9nx/xZXkc
I2G09w852KgYDVYUHgW3IfuRo4F+4Y7Mw0Klu4XX5OmEXhselIqhwwTmEKMvEns=
=OLri
-----END PGP SIGNATURE-----

Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]

[Phillip Susi]

[-- Attachment #1.1: Type: text/plain, Size: 829 bytes --]

> I have not tested it yet, but I am pretty sure it won't work.  It
> looks like the patch changes the BLKRRPART path to go ahead and remove
> existing partitions when GENHD_FL_NO_PARTSCAN is set.  loop doesn't
> issue the BLKRRPART ioctl when !LO_FLAGS_PARTSCAN so this won't help.
>  I think loop needs to set GENHD_FL_NO_PARTSCAN and then issue the
> ioctl regardless of the LO_FLAGS_PARTSCAN flag to get the partitions
> to be removed.  I will try to test tonight.

After testing, my initial thoughts appeared to have been correct.  I had
to modify the patch as follows.  To test, simply do:

truncate -s 10m img
losetup /dev/loop0 img
parted /dev/loop0
mklabel msdos
mkpart primary ext2 1m 2m
quit
ls /dev/loop0*

Note the /dev/loop0p1 node.  Run losetup -d /dev/loop0 and see if it is
still there.



[-- Attachment #1.2: loop.diff --]
[-- Type: text/plain, Size: 1722 bytes --]

diff --git a/block/ioctl.c b/block/ioctl.c
index a31d91d..8b78b5a 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -155,7 +155,7 @@ static int blkdev_reread_part(struct block_device *bdev)
 	struct gendisk *disk = bdev->bd_disk;
 	int res;
 
-	if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains)
+	if (bdev != bdev->bd_contains)
 		return -EINVAL;
 	if (!capable(CAP_SYS_ADMIN))
 		return -EACCES;
diff --git a/block/partition-generic.c b/block/partition-generic.c
index 1cb4dec..0e7d637 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -430,6 +430,15 @@ rescan:
 		disk->fops->revalidate_disk(disk);
 	check_disk_size_change(disk, bdev);
 	bdev->bd_invalidated = 0;
+
+	/*
+	 * If partition scanning is disabled, we are done.
+	 */
+	if (!disk_part_scan_enabled(disk)) {
+		kobject_uevent(&disk_to_dev(disk)->kobj, KOBJ_CHANGE);
+		return 0;
+	}
+
 	if (!get_capacity(disk) || !(state = check_partition(disk, bdev)))
 		return 0;
 	if (IS_ERR(state)) {
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 8bc6d39..326fac9 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1039,11 +1039,9 @@ static int loop_clr_fd(struct loop_device *lo)
 	lo->lo_state = Lo_unbound;
 	/* This is safe: open() is still holding a reference. */
 	module_put(THIS_MODULE);
-	if (lo->lo_flags & LO_FLAGS_PARTSCAN && bdev)
-		ioctl_by_bdev(bdev, BLKRRPART, 0);
+	lo->lo_disk->flags |= GENHD_FL_NO_PART_SCAN;
+	ioctl_by_bdev(bdev, BLKRRPART, 0);
 	lo->lo_flags = 0;
-	if (!part_shift)
-		lo->lo_disk->flags |= GENHD_FL_NO_PART_SCAN;
 	mutex_unlock(&lo->lo_ctl_mutex);
 	/*
 	 * Need not hold lo_ctl_mutex to fput backing file.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 553 bytes --]

Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]

[Dave Chinner]

On Thu, Apr 04, 2013 at 04:30:54PM -0400, Phillip Susi wrote:
> > I have not tested it yet, but I am pretty sure it won't work.  It
> > looks like the patch changes the BLKRRPART path to go ahead and remove
> > existing partitions when GENHD_FL_NO_PARTSCAN is set.  loop doesn't
> > issue the BLKRRPART ioctl when !LO_FLAGS_PARTSCAN so this won't help.
> >  I think loop needs to set GENHD_FL_NO_PARTSCAN and then issue the
> > ioctl regardless of the LO_FLAGS_PARTSCAN flag to get the partitions
> > to be removed.  I will try to test tonight.
> 
> After testing, my initial thoughts appeared to have been correct.  I had
> to modify the patch as follows.  To test, simply do:
> 
> truncate -s 10m img
> losetup /dev/loop0 img
> parted /dev/loop0
> mklabel msdos
> mkpart primary ext2 1m 2m
> quit
> ls /dev/loop0*
> 
> Note the /dev/loop0p1 node.  Run losetup -d /dev/loop0 and see if it is
> still there.

Jens, can we get one of these fixes merged quickly? xfstests is
unusable on any kernel more recent than 3.9-rc4 because of these
problems....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]

[Jens Axboe]

On Tue, Apr 09 2013, Dave Chinner wrote:
> On Thu, Apr 04, 2013 at 04:30:54PM -0400, Phillip Susi wrote:
> > > I have not tested it yet, but I am pretty sure it won't work.  It
> > > looks like the patch changes the BLKRRPART path to go ahead and remove
> > > existing partitions when GENHD_FL_NO_PARTSCAN is set.  loop doesn't
> > > issue the BLKRRPART ioctl when !LO_FLAGS_PARTSCAN so this won't help.
> > >  I think loop needs to set GENHD_FL_NO_PARTSCAN and then issue the
> > > ioctl regardless of the LO_FLAGS_PARTSCAN flag to get the partitions
> > > to be removed.  I will try to test tonight.
> > 
> > After testing, my initial thoughts appeared to have been correct.  I had
> > to modify the patch as follows.  To test, simply do:
> > 
> > truncate -s 10m img
> > losetup /dev/loop0 img
> > parted /dev/loop0
> > mklabel msdos
> > mkpart primary ext2 1m 2m
> > quit
> > ls /dev/loop0*
> > 
> > Note the /dev/loop0p1 node.  Run losetup -d /dev/loop0 and see if it is
> > still there.
> 
> Jens, can we get one of these fixes merged quickly? xfstests is
> unusable on any kernel more recent than 3.9-rc4 because of these
> problems....

Yep, did the revert yesterday and it's going out today.

-- 
Jens Axboe

Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]

[Dave Chinner]

On Tue, Apr 09, 2013 at 09:01:39AM +0200, Jens Axboe wrote:
> On Tue, Apr 09 2013, Dave Chinner wrote:
> > On Thu, Apr 04, 2013 at 04:30:54PM -0400, Phillip Susi wrote:
> > > > I have not tested it yet, but I am pretty sure it won't work.  It
> > > > looks like the patch changes the BLKRRPART path to go ahead and remove
> > > > existing partitions when GENHD_FL_NO_PARTSCAN is set.  loop doesn't
> > > > issue the BLKRRPART ioctl when !LO_FLAGS_PARTSCAN so this won't help.
> > > >  I think loop needs to set GENHD_FL_NO_PARTSCAN and then issue the
> > > > ioctl regardless of the LO_FLAGS_PARTSCAN flag to get the partitions
> > > > to be removed.  I will try to test tonight.
> > > 
> > > After testing, my initial thoughts appeared to have been correct.  I had
> > > to modify the patch as follows.  To test, simply do:
> > > 
> > > truncate -s 10m img
> > > losetup /dev/loop0 img
> > > parted /dev/loop0
> > > mklabel msdos
> > > mkpart primary ext2 1m 2m
> > > quit
> > > ls /dev/loop0*
> > > 
> > > Note the /dev/loop0p1 node.  Run losetup -d /dev/loop0 and see if it is
> > > still there.
> > 
> > Jens, can we get one of these fixes merged quickly? xfstests is
> > unusable on any kernel more recent than 3.9-rc4 because of these
> > problems....
> 
> Yep, did the revert yesterday and it's going out today.

Great, I didn't see an update in the latest -rc, so I wanted to make
sure it hadn't fallen through he cracks....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]

[Jens Axboe]

On Tue, Apr 09 2013, Dave Chinner wrote:
> On Tue, Apr 09, 2013 at 09:01:39AM +0200, Jens Axboe wrote:
> > On Tue, Apr 09 2013, Dave Chinner wrote:
> > > On Thu, Apr 04, 2013 at 04:30:54PM -0400, Phillip Susi wrote:
> > > > > I have not tested it yet, but I am pretty sure it won't work.  It
> > > > > looks like the patch changes the BLKRRPART path to go ahead and remove
> > > > > existing partitions when GENHD_FL_NO_PARTSCAN is set.  loop doesn't
> > > > > issue the BLKRRPART ioctl when !LO_FLAGS_PARTSCAN so this won't help.
> > > > >  I think loop needs to set GENHD_FL_NO_PARTSCAN and then issue the
> > > > > ioctl regardless of the LO_FLAGS_PARTSCAN flag to get the partitions
> > > > > to be removed.  I will try to test tonight.
> > > > 
> > > > After testing, my initial thoughts appeared to have been correct.  I had
> > > > to modify the patch as follows.  To test, simply do:
> > > > 
> > > > truncate -s 10m img
> > > > losetup /dev/loop0 img
> > > > parted /dev/loop0
> > > > mklabel msdos
> > > > mkpart primary ext2 1m 2m
> > > > quit
> > > > ls /dev/loop0*
> > > > 
> > > > Note the /dev/loop0p1 node.  Run losetup -d /dev/loop0 and see if it is
> > > > still there.
> > > 
> > > Jens, can we get one of these fixes merged quickly? xfstests is
> > > unusable on any kernel more recent than 3.9-rc4 because of these
> > > problems....
> > 
> > Yep, did the revert yesterday and it's going out today.
> 
> Great, I didn't see an update in the latest -rc, so I wanted to make
> sure it hadn't fallen through he cracks....

Got pushed out yesterday, it is now upstream.

-- 
Jens Axboe

Re: Loopback device hung [was Re: xfs deadlock on 3.9-rc5 running xfstests case #78]

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Jens, did this get lost in the shuffle, or just miss the window for
3.10 and will go in 3.11?

On 4/4/2013 4:30 PM, Phillip Susi wrote:
>> I have not tested it yet, but I am pretty sure it won't work.
>> It looks like the patch changes the BLKRRPART path to go ahead
>> and remove existing partitions when GENHD_FL_NO_PARTSCAN is set.
>> loop doesn't issue the BLKRRPART ioctl when !LO_FLAGS_PARTSCAN so
>> this won't help. I think loop needs to set GENHD_FL_NO_PARTSCAN
>> and then issue the ioctl regardless of the LO_FLAGS_PARTSCAN flag
>> to get the partitions to be removed.  I will try to test
>> tonight.
> 
> After testing, my initial thoughts appeared to have been correct.
> I had to modify the patch as follows.  To test, simply do:
> 
> truncate -s 10m img losetup /dev/loop0 img parted /dev/loop0 
> mklabel msdos mkpart primary ext2 1m 2m quit ls /dev/loop0*
> 
> Note the /dev/loop0p1 node.  Run losetup -d /dev/loop0 and see if
> it is still there.
> 
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJRpMRWAAoJEJrBOlT6nu75yUYH/1PSNmTOBYgvEqQclWPBTd8n
Fm95yILcIlJWdUr3gUvQmjBax1NzKnG3NZ2U4hucpCcJH4FkHSTTFl5uZ3wU1B/Q
nvuQoSqYXVP+V9PVSTsUsxOI4Mvw5YP5sFwSdwRKpkCldXOuHrRZsuccFtkQducU
AIij42jvI1un+/qc6NLW+/S+rcLGUj21boPmX3g80km1De9QMrYrbGAAEbFO3rrq
fzsvGuVvOroGppLGpze4iMDn060Lxrw6//KDGtiUBIeD8kZCFGkBh1KupHqLLzXm
gmfdlHIqAR5uWB29m9TOlRbHPzdeQutwt8zL2LOYlCft5OiBIOW8dT8EFkl2Buw=
=7BGq
-----END PGP SIGNATURE-----