On Thu, 23 Apr 2015 18:10:51 +0200 Christoph Hellwig <hch@xxxxxx> wrote:
> On Thu, Apr 23, 2015 at 06:03:14PM +1000, NeilBrown wrote:
> > On Thu, 23 Apr 2015 09:37:24 +0200 Christoph Hellwig <hch@xxxxxx> wrote:
> >
> > > Plase fix your device name lifetimes.
> >
> > Any chance you could be more explicit?
> >
> > The commit you identified doesn't seem to help much - md and dm are quite
> > different in this area.
> >
> > It seems that it is no longer safe to call 'add_disk' between calling
> > 'del_gendisk' and bdi_destroy being called. How can I find out if I am in
> > that window, or wait for bdi_destroy to be called?
>
> The bdi is only around if the device is open, either through a device
> node, or through a blkdev_get from a file system. If you get duplicate
> names that means you're trying to allocate a new gendisk while the old
> one is still around.
>
> In theory you're fine once the device gets ->release called.
In practice .... the put_disk() shortly after the ->release call in
__blkdev_put() is what ultimately releases the name of the bdi - at least in
the cases where I get a crash.
>
> Except that we can hold sysfs reference to the qeue, eww. So for now
> try to follow the dm model, but I'll need to add a callback to the
> queue called once the request_queue actually is released for this.
I'm pretty sure that the md code is already as close to the "dm model" as it
meaningfully can be.
If I move bdi_destroy out of blk_release_queue (which really think is too
later) and place it in blk_cleanup_queue (which seems a credible place for
it), and then move the blk_cleanup_queue call in md_free up before the
del_gendisk() call (which is probably the right thing to do anyway, though dm
has the same order that md currently has) then I don't get any crashes and
I'm almost convince it is correct...
Thoughts?
Thanks,
NeilBrown
diff --git a/block/blk-core.c b/block/blk-core.c
index 794c3e7f01cf..66406474f0c4 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -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);
}
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index faaf36ade7eb..2b8fd302f677 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -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);
}
diff --git a/drivers/md/md.c b/drivers/md/md.c
index d4f31e195e26..593a02476c78 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -4818,12 +4818,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);
}
Attachment:
pgpWtdSiMHzxL.pgp
Description: OpenPGP digital signature
