Skip to contentSkip to conversation

Patch "block: destroy bdi before blockdev is unregistered." has been added to the 4.0-stable tree

PATCH [email protected] 4 participants 5 messages May 15, 2015 – May 19, 2015

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

block: destroy bdi before blockdev is unregistered.

to the 4.0-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
block-destroy-bdi-before-blockdev-is-unregistered.patch
and it can be found in the queue-4.0 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 6cd18e711dd8075da9d78cfc1239f912ff28968a Mon Sep 17 00:00:00 2001

From: NeilBrown <[email protected]>
Date: Mon, 27 Apr 2015 14:12:22 +1000
Subject: block: destroy bdi before blockdev is unregistered.

From: NeilBrown <[email protected]>

commit 6cd18e711dd8075da9d78cfc1239f912ff28968a upstream.

Because of the peculiar way that md devices are created (automatically when the device node is opened), a new device can be created and registered immediately after the

	blk_unregister_region(disk_devt(disk), disk->minors);

call in del_gendisk().

Therefore it is important that all visible artifacts of the previous device are removed before this call. In particular, the 'bdi'.

Since:
commit c4db59d31e39ea067c32163ac961e9c80198fd37
Author: Christoph Hellwig <[email protected]>
fs: don't reassign dirty inodes to default_backing_dev_info

moved the
device_unregister(bdi->dev);
call from bdi_unregister() to bdi_destroy() it has been quite easy to lose a race and have a new (e.g.) "md127" be created after the blk_unregister_region() call and before bdi_destroy() is ultimately called by the final 'put_disk', which must come after del_gendisk().

The new device finds that the bdi name is already registered in sysfs and complains

Quoted 2 lines
[ 9627.630029] WARNING: CPU: 18 PID: 3330 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x5a/0x70()
[ 9627.630032] sysfs: cannot create duplicate filename '/devices/virtual/bdi/9:127'

We can fix this by moving the bdi_destroy() call out of blk_release_queue() (which can happen very late when a refcount reaches zero) and into blk_cleanup_queue() - which happens exactly when the md device driver calls it.

Then it is only necessary for md to call blk_cleanup_queue() before del_gendisk(). As loop.c devices are also created on demand by opening the device node, we make the same change there.

Fixes c4db59d31e39ea067c32163ac961e9c80198fd37
Cc Christoph Hellwig <[email protected]>
---
 block/blk-core.c     |    2 ++
 block/blk-sysfs.c    |    2 --
 drivers/block/loop.c |    2 +-
 drivers/md/md.c      |    4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)
block/blk-core.c +2 -0
@@ -552,6 +552,8 @@ void blk_cleanup_queue(struct request_qu
 		q->queue_lock = &q->__queue_lock;
 	spin_unlock_irq(lock);
 
+	bdi_destroy(&q->backing_dev_info);
+
 	/* @q is and will stay empty, shutdown and put */
 	blk_put_queue(q);
 }
block/blk-sysfs.c +0 -2
@@ -522,8 +522,6 @@ static void blk_release_queue(struct kob
 
 	blk_trace_shutdown(q);
 
-	bdi_destroy(&q->backing_dev_info);
-
 	ida_simple_remove(&blk_queue_ida, q->id);
 	call_rcu(&q->rcu_head, blk_free_queue_rcu);
 }
drivers/block/loop.c +1 -1
@@ -1672,8 +1672,8 @@ out:
 
 static void loop_remove(struct loop_device *lo)
 {
-	del_gendisk(lo->lo_disk);
 	blk_cleanup_queue(lo->lo_queue);
+	del_gendisk(lo->lo_disk);
 	blk_mq_free_tag_set(&lo->tag_set);
 	put_disk(lo->lo_disk);
 	kfree(lo);
drivers/md/md.c +2 -2
@@ -4754,12 +4754,12 @@ static void md_free(struct kobject *ko)
 	if (mddev->sysfs_state)
 		sysfs_put(mddev->sysfs_state);
 
+	if (mddev->queue)
+		blk_cleanup_queue(mddev->queue);
 	if (mddev->gendisk) {
 		del_gendisk(mddev->gendisk);
 		put_disk(mddev->gendisk);
 	}
-	if (mddev->queue)
-		blk_cleanup_queue(mddev->queue);
 
 	kfree(mddev);
 }

Patches currently in stable-queue which might be from [email protected] are

queue-4.0/block-destroy-bdi-before-blockdev-is-unregistered.patch

#1 lore raw
Quoted 7 lines · On (05/14/15 19:18), [email protected] wrote:

This is a note to let you know that I've just added the patch titled

block: destroy bdi before blockdev is unregistered.

to the 4.0-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

Quoted 8 lines

The filename of the patch is:
block-destroy-bdi-before-blockdev-is-unregistered.patch
and it can be found in the queue-4.0 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Hello Greg,

jfi, I think this commit will WARN_ON(). fixed by https://lkml.org/lkml/2015/5/8/29

(https://lkml.org/lkml/2015/4/28/568)

	-ss
Quoted 121 lines

From 6cd18e711dd8075da9d78cfc1239f912ff28968a Mon Sep 17 00:00:00 2001
From: NeilBrown <[email protected]>
Date: Mon, 27 Apr 2015 14:12:22 +1000
Subject: block: destroy bdi before blockdev is unregistered.

From: NeilBrown <[email protected]>

commit 6cd18e711dd8075da9d78cfc1239f912ff28968a upstream.

Because of the peculiar way that md devices are created (automatically when the device node is opened), a new device can be created and registered immediately after the

	blk_unregister_region(disk_devt(disk), disk->minors);

call in del_gendisk().

Therefore it is important that all visible artifacts of the previous device are removed before this call. In particular, the 'bdi'.

Since:
commit c4db59d31e39ea067c32163ac961e9c80198fd37
Author: Christoph Hellwig <[email protected]>
fs: don't reassign dirty inodes to default_backing_dev_info

moved the
device_unregister(bdi->dev);
call from bdi_unregister() to bdi_destroy() it has been quite easy to lose a race and have a new (e.g.) "md127" be created after the blk_unregister_region() call and before bdi_destroy() is ultimately called by the final 'put_disk', which must come after del_gendisk().

The new device finds that the bdi name is already registered in sysfs and complains

Quoted 2 lines
[ 9627.630029] WARNING: CPU: 18 PID: 3330 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x5a/0x70()
[ 9627.630032] sysfs: cannot create duplicate filename '/devices/virtual/bdi/9:127'

We can fix this by moving the bdi_destroy() call out of blk_release_queue() (which can happen very late when a refcount reaches zero) and into blk_cleanup_queue() - which happens exactly when the md device driver calls it.

Then it is only necessary for md to call blk_cleanup_queue() before del_gendisk(). As loop.c devices are also created on demand by opening the device node, we make the same change there.

Fixes c4db59d31e39ea067c32163ac961e9c80198fd37
Cc Christoph Hellwig <[email protected]>
---
 block/blk-core.c     |    2 ++
 block/blk-sysfs.c    |    2 --
 drivers/block/loop.c |    2 +-
 drivers/md/md.c      |    4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)
block/blk-core.c +2 -0
@@ -552,6 +552,8 @@ void blk_cleanup_queue(struct request_qu
 		q->queue_lock = &q->__queue_lock;
 	spin_unlock_irq(lock);
 
+	bdi_destroy(&q->backing_dev_info);
+
 	/* @q is and will stay empty, shutdown and put */
 	blk_put_queue(q);
 }
block/blk-sysfs.c +0 -2
@@ -522,8 +522,6 @@ static void blk_release_queue(struct kob
 
 	blk_trace_shutdown(q);
 
-	bdi_destroy(&q->backing_dev_info);
-
 	ida_simple_remove(&blk_queue_ida, q->id);
 	call_rcu(&q->rcu_head, blk_free_queue_rcu);
 }
drivers/block/loop.c +1 -1
@@ -1672,8 +1672,8 @@ out:
 
 static void loop_remove(struct loop_device *lo)
 {
-	del_gendisk(lo->lo_disk);
 	blk_cleanup_queue(lo->lo_queue);
+	del_gendisk(lo->lo_disk);
 	blk_mq_free_tag_set(&lo->tag_set);
 	put_disk(lo->lo_disk);
 	kfree(lo);
drivers/md/md.c +2 -2
@@ -4754,12 +4754,12 @@ static void md_free(struct kobject *ko)
 	if (mddev->sysfs_state)
 		sysfs_put(mddev->sysfs_state);
 
+	if (mddev->queue)
+		blk_cleanup_queue(mddev->queue);
 	if (mddev->gendisk) {
 		del_gendisk(mddev->gendisk);
 		put_disk(mddev->gendisk);
 	}
-	if (mddev->queue)
-		blk_cleanup_queue(mddev->queue);
 
 	kfree(mddev);
 }

Patches currently in stable-queue which might be from [email protected] are

queue-4.0/block-destroy-bdi-before-blockdev-is-unregistered.patch --
To unsubscribe from this list: send the line "unsubscribe stable-commits" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author maintainer
#2 lore raw
Quoted 17 lines · Sergey Senozhatsky
Quoted 7 lines · On (05/14/15 19:18), [email protected] wrote:

This is a note to let you know that I've just added the patch titled

block: destroy bdi before blockdev is unregistered.

to the 4.0-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

Quoted 8 lines

The filename of the patch is:
block-destroy-bdi-before-blockdev-is-unregistered.patch
and it can be found in the queue-4.0 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 6 lines

Hello Greg,

jfi, I think this commit will WARN_ON(). fixed by https://lkml.org/lkml/2015/5/8/29

(https://lkml.org/lkml/2015/4/28/568)

That site isn't working for me, what is the git commit id of the fix for this in Linus's tree?

thanks,

greg k-h

#3 lore raw
Quoted 23 lines · On Fri, 15 May 2015 16:16:32 -0700 Greg KH <[email protected]> wrote:
Quoted 22 lines · Sergey Senozhatsky
Quoted 7 lines · On (05/14/15 19:18), [email protected] wrote:

This is a note to let you know that I've just added the patch titled

block: destroy bdi before blockdev is unregistered.

to the 4.0-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

Quoted 8 lines

The filename of the patch is:
block-destroy-bdi-before-blockdev-is-unregistered.patch
and it can be found in the queue-4.0 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Hello Greg,

jfi, I think this commit will WARN_ON(). fixed by https://lkml.org/lkml/2015/5/8/29

Quoted 8 lines
Quoted 1 line

(https://lkml.org/lkml/2015/4/28/568)

That site isn't working for me, what is the git commit id of the fix for this in Linus's tree?

thanks,

greg k-h

http://lkml.kernel.org/r/<[email protected]>

Unfortunately it isn't with Linus yet, or even in -next. Jens hasn't replied.

NeilBrown