diff mbox

[03/10] block: Revalidate i_bdev reference in bd_aquire()

Message ID 20170209155417.GA15818@quack2.suse.cz (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Kara Feb. 9, 2017, 3:54 p.m. UTC 
On Thu 09-02-17 13:44:26, Jan Kara wrote:
> When a device gets removed, block device inode unhashed so that it is not
> used anymore (bdget() will not find it anymore). Later when a new device
> gets created with the same device number, we create new block device
> inode. However there may be file system device inodes whose i_bdev still
> points to the original block device inode and thus we get two active
> block device inodes for the same device. They will share the same
> gendisk so the only visible differences will be that page caches will
> not be coherent and BDIs will be different (the old block device inode
> still points to unregistered BDI).
> 
> Fix the problem by checking in bd_acquire() whether i_bdev still points
> to active block device inode and re-lookup the block device if not. That
> way any open of a block device happening after the old device has been
> removed will get correct block device inode.

Thiago spotted a stupid bug in this patch (calling bd_forget() on bdev
instead of inode). Fixed version is attached.

								Honza
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/block_dev.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 601b71b76d7f..360439373a66 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -1043,13 +1043,22 @@ static struct block_device *bd_acquire(struct inode *inode)
>  
>  	spin_lock(&bdev_lock);
>  	bdev = inode->i_bdev;
> -	if (bdev) {
> +	if (bdev && !inode_unhashed(bdev->bd_inode)) {
>  		bdgrab(bdev);
>  		spin_unlock(&bdev_lock);
>  		return bdev;
>  	}
>  	spin_unlock(&bdev_lock);
>  
> +	/*
> +	 * i_bdev references block device inode that was already shut down
> +	 * (corresponding device got removed).  Remove the reference and look
> +	 * up block device inode again just in case new device got
> +	 * reestablished under the same device number.
> +	 */
> +	if (bdev)
> +		bd_forget(bdev);
> +
>  	bdev = bdget(inode->i_rdev);
>  	if (bdev) {
>  		spin_lock(&bdev_lock);
> -- 
> 2.10.2
>

Comments

Tejun Heo Feb. 12, 2017, 4:22 a.m. UTC | #1 
On Thu, Feb 09, 2017 at 04:54:17PM +0100, Jan Kara wrote:
> From aaf612333753b948a96aebe4a2f8066ed45ef164 Mon Sep 17 00:00:00 2001
> From: Jan Kara <jack@suse.cz>
> Date: Thu, 9 Feb 2017 12:16:30 +0100
> Subject: [PATCH 03/10] block: Revalidate i_bdev reference in bd_aquire()
> 
> When a device gets removed, block device inode unhashed so that it is not
> used anymore (bdget() will not find it anymore). Later when a new device
> gets created with the same device number, we create new block device
> inode. However there may be file system device inodes whose i_bdev still
> points to the original block device inode and thus we get two active
> block device inodes for the same device. They will share the same
> gendisk so the only visible differences will be that page caches will
> not be coherent and BDIs will be different (the old block device inode
> still points to unregistered BDI).
> 
> Fix the problem by checking in bd_acquire() whether i_bdev still points
> to active block device inode and re-lookup the block device if not. That
> way any open of a block device happening after the old device has been
> removed will get correct block device inode.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.
diff mbox

Patch

From aaf612333753b948a96aebe4a2f8066ed45ef164 Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Thu, 9 Feb 2017 12:16:30 +0100
Subject: [PATCH 03/10] block: Revalidate i_bdev reference in bd_aquire()

When a device gets removed, block device inode unhashed so that it is not
used anymore (bdget() will not find it anymore). Later when a new device
gets created with the same device number, we create new block device
inode. However there may be file system device inodes whose i_bdev still
points to the original block device inode and thus we get two active
block device inodes for the same device. They will share the same
gendisk so the only visible differences will be that page caches will
not be coherent and BDIs will be different (the old block device inode
still points to unregistered BDI).

Fix the problem by checking in bd_acquire() whether i_bdev still points
to active block device inode and re-lookup the block device if not. That
way any open of a block device happening after the old device has been
removed will get correct block device inode.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/block_dev.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 601b71b76d7f..68e855fdce58 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1043,13 +1043,22 @@  static struct block_device *bd_acquire(struct inode *inode)
 
 	spin_lock(&bdev_lock);
 	bdev = inode->i_bdev;
-	if (bdev) {
+	if (bdev && !inode_unhashed(bdev->bd_inode)) {
 		bdgrab(bdev);
 		spin_unlock(&bdev_lock);
 		return bdev;
 	}
 	spin_unlock(&bdev_lock);
 
+	/*
+	 * i_bdev references block device inode that was already shut down
+	 * (corresponding device got removed).  Remove the reference and look
+	 * up block device inode again just in case new device got
+	 * reestablished under the same device number.
+	 */
+	if (bdev)
+		bd_forget(inode);
+
 	bdev = bdget(inode->i_rdev);
 	if (bdev) {
 		spin_lock(&bdev_lock);
-- 
2.10.2