Skip to content

Commit

Permalink
block: destroy bdi before blockdev is unregistered.
Browse files Browse the repository at this point in the history
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 c4db59d
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

> [ 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: c4db59d
Reported-by: Azat Khuzhin <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: [email protected] (v4.0)
Signed-off-by: NeilBrown <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
  • Loading branch information
neilbrown authored and axboe committed Apr 27, 2015
1 parent 393a339 commit 6cd18e7
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 5 deletions.
2 changes: 2 additions & 0 deletions block/blk-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,8 @@ void blk_cleanup_queue(struct request_queue *q)
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);
}
Expand Down
2 changes: 0 additions & 2 deletions block/blk-sysfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -522,8 +522,6 @@ static void blk_release_queue(struct kobject *kobj)

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);
}
Expand Down
2 changes: 1 addition & 1 deletion drivers/block/loop.c
Original file line number Diff line number Diff line change
Expand Up @@ -1620,8 +1620,8 @@ static int loop_add(struct loop_device **l, int i)

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);
Expand Down
4 changes: 2 additions & 2 deletions drivers/md/md.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit 6cd18e7

Please sign in to comment.