Alexander Graf
2012-Dec-19 19:40 UTC
[PATCH] virtio-blk: Don't free ida when disk is in use
When a file system is mounted on a virtio-blk disk, we then remove it and then reattach it, the reattached disk gets the same disk name and ids as the hot removed one. This leads to very nasty effects - mostly rendering the newly attached device completely unusable. Trying what happens when I do the same thing with a USB device, I saw that the sd node simply doesn't get free'd when a device gets forcefully removed. Imitate the same behavior for vd devices. This way broken vd devices simply are never free'd and newly attached ones keep working just fine. Signed-off-by: Alexander Graf <agraf at suse.de> --- drivers/block/virtio_blk.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 0bdde8f..07a18e2 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -889,6 +889,7 @@ static void __devexit virtblk_remove(struct virtio_device *vdev) { struct virtio_blk *vblk = vdev->priv; int index = vblk->index; + int refc; /* Prevent config work handler from accessing the device. */ mutex_lock(&vblk->config_lock); @@ -903,11 +904,15 @@ static void __devexit virtblk_remove(struct virtio_device *vdev) flush_work(&vblk->config_work); + refc = atomic_read(&disk_to_dev(vblk->disk)->kobj.kref.refcount); put_disk(vblk->disk); mempool_destroy(vblk->pool); vdev->config->del_vqs(vdev); kfree(vblk); - ida_simple_remove(&vd_index_ida, index); + + /* Only free device id if we don't have any users */ + if (refc == 1) + ida_simple_remove(&vd_index_ida, index); } #ifdef CONFIG_PM -- 1.7.12.4
Rusty Russell
2012-Dec-20 04:15 UTC
[PATCH] virtio-blk: Don't free ida when disk is in use
Alexander Graf <agraf at suse.de> writes:> When a file system is mounted on a virtio-blk disk, we then remove it > and then reattach it, the reattached disk gets the same disk name and > ids as the hot removed one. > > This leads to very nasty effects - mostly rendering the newly attached > device completely unusable. > > Trying what happens when I do the same thing with a USB device, I saw > that the sd node simply doesn't get free'd when a device gets forcefully > removed. > > Imitate the same behavior for vd devices. This way broken vd devices > simply are never free'd and newly attached ones keep working just fine. > > Signed-off-by: Alexander Graf <agraf at suse.de>I think deserves a CC:stable, no? I've put it in my pending queue for *next* merge window for now... Thanks, Rusty.
Michael S. Tsirkin
2012-Dec-20 10:54 UTC
[PATCH] virtio-blk: Don't free ida when disk is in use
On Wed, Dec 19, 2012 at 08:40:15PM +0100, Alexander Graf wrote:> When a file system is mounted on a virtio-blk disk, we then remove it > and then reattach it, the reattached disk gets the same disk name and > ids as the hot removed one. > > This leads to very nasty effects - mostly rendering the newly attached > device completely unusable. > > Trying what happens when I do the same thing with a USB device, I saw > that the sd node simply doesn't get free'd when a device gets forcefully > removed. > > Imitate the same behavior for vd devices. This way broken vd devices > simply are never free'd and newly attached ones keep working just fine. > > Signed-off-by: Alexander Graf <agraf at suse.de> > --- > drivers/block/virtio_blk.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c > index 0bdde8f..07a18e2 100644 > --- a/drivers/block/virtio_blk.c > +++ b/drivers/block/virtio_blk.c > @@ -889,6 +889,7 @@ static void __devexit virtblk_remove(struct virtio_device *vdev) > { > struct virtio_blk *vblk = vdev->priv; > int index = vblk->index; > + int refc; > > /* Prevent config work handler from accessing the device. */ > mutex_lock(&vblk->config_lock); > @@ -903,11 +904,15 @@ static void __devexit virtblk_remove(struct virtio_device *vdev) > > flush_work(&vblk->config_work); > > + refc = atomic_read(&disk_to_dev(vblk->disk)->kobj.kref.refcount); > put_disk(vblk->disk); > mempool_destroy(vblk->pool); > vdev->config->del_vqs(vdev); > kfree(vblk); > - ida_simple_remove(&vd_index_ida, index); > + > + /* Only free device id if we don't have any users */ > + if (refc == 1) > + ida_simple_remove(&vd_index_ida, index); > } > > #ifdef CONFIG_PMNetwork devices take the approach of retrying every second. Donnu if it makes sense here.> -- > 1.7.12.4
Seemingly Similar Threads
- [PATCH] virtio-blk: Don't free ida when disk is in use
- [PATCH v2] virtio-blk: handle block_device_operations callbacks after hot unplug
- [PATCH v2] virtio-blk: handle block_device_operations callbacks after hot unplug
- [PATCH v3] virtio-blk: handle block_device_operations callbacks after hot unplug
- [PATCH v3] virtio-blk: handle block_device_operations callbacks after hot unplug