search for: del_gendisk

From: Greg KH <gregkh at linuxfoundation.org> 3.5-stable review patch. If anyone has any objections, please let me know. ------------------ From: Asias He <asias at redhat.com> commit 02e2b124943648fba0a2ccee5c3656a5653e0151 upstream. del_gendisk() might not return due to failing to remove the /sys/block/vda/serial sysfs entry when another thread (udev) is trying to read it. virtblk_remove() vdev->config->reset() : guest will not kick us through interrupt del_gendisk() device_del() kobject_del(): got stuck, sysfs...

From: Greg KH <gregkh at linuxfoundation.org> 3.5-stable review patch. If anyone has any objections, please let me know. ------------------ From: Asias He <asias at redhat.com> commit 02e2b124943648fba0a2ccee5c3656a5653e0151 upstream. del_gendisk() might not return due to failing to remove the /sys/block/vda/serial sysfs entry when another thread (udev) is trying to read it. virtblk_remove() vdev->config->reset() : guest will not kick us through interrupt del_gendisk() device_del() kobject_del(): got stuck, sysfs...

3.4-stable review patch. If anyone has any objections, please let me know. ------------------ From: Asias He <asias at redhat.com> commit 02e2b124943648fba0a2ccee5c3656a5653e0151 upstream. del_gendisk() might not return due to failing to remove the /sys/block/vda/serial sysfs entry when another thread (udev) is trying to read it. virtblk_remove() vdev->config->reset() : guest will not kick us through interrupt del_gendisk() device_del() kobject_del(): got stuck, sysfs...

3.4-stable review patch. If anyone has any objections, please let me know. ------------------ From: Asias He <asias at redhat.com> commit 02e2b124943648fba0a2ccee5c3656a5653e0151 upstream. del_gendisk() might not return due to failing to remove the /sys/block/vda/serial sysfs entry when another thread (udev) is trying to read it. virtblk_remove() vdev->config->reset() : guest will not kick us through interrupt del_gendisk() device_del() kobject_del(): got stuck, sysfs...

...ugged. Several different errors occur when a block device with mounted file-system(s) is hot-unplugged. Asynchronous writeback functions, as well as page cache read or write operations end up in never ending wait situations. Hang situations occur during device removal when virtblk_remove() invokes del_gendisk() to synch dirty inode pages (invalidate_partition()). The below patches [4,5,6,7] also exploit a 'broken' virtqueue in order to trigger IO errors as well as to prevent final hanging IO operations. Heinz Graalfs (7): virtio_ring: add new functions virtqueue{_set_broken()/_is_broken()}...

...ugged. Several different errors occur when a block device with mounted file-system(s) is hot-unplugged. Asynchronous writeback functions, as well as page cache read or write operations end up in never ending wait situations. Hang situations occur during device removal when virtblk_remove() invokes del_gendisk() to synch dirty inode pages (invalidate_partition()). The below patches [4,5,6,7] also exploit a 'broken' virtqueue in order to trigger IO errors as well as to prevent final hanging IO operations. Heinz Graalfs (7): virtio_ring: add new functions virtqueue{_set_broken()/_is_broken()}...

Deleting the disk and partitions in virtblk_remove() via del_gendisk() causes never ending waits when trying to synch dirty inode pages. A dummy virtqueue_notify() in virtblk_remove() is used to detect a host notification error, latter occurs when block device was hot-unplugged. When the dummy host kick failed blk_cleanup_queue() should be invoked prior to del_gend...

...; + struct virtblk_req *vbr; /* Prevent config work handler from accessing the device. */ mutex_lock(&vblk->config_lock); vblk->config_enable = false; mutex_unlock(&vblk->config_lock); + /* Abort all request on the queue. */ + blk_abort_queue(vblk->disk->queue); + del_gendisk(vblk->disk); + /* Stop all the virtqueues. */ vdev->config->reset(vdev); - flush_work(&vblk->config_work); - del_gendisk(vblk->disk); + /* Abort request dispatched to driver. */ + while ((vbr = virtqueue_detach_unused_buf(vblk->vq))) { + blk_abort_request(vbr->req)...

...; + struct virtblk_req *vbr; /* Prevent config work handler from accessing the device. */ mutex_lock(&vblk->config_lock); vblk->config_enable = false; mutex_unlock(&vblk->config_lock); + /* Abort all request on the queue. */ + blk_abort_queue(vblk->disk->queue); + del_gendisk(vblk->disk); + /* Stop all the virtqueues. */ vdev->config->reset(vdev); - flush_work(&vblk->config_work); - del_gendisk(vblk->disk); + /* Abort request dispatched to driver. */ + while ((vbr = virtqueue_detach_unused_buf(vblk->vq))) { + blk_abort_request(vbr->req)...

This patch set fixes the race when hot-unplug stressed disk. Asias He (3): virtio-blk: Call del_gendisk() before disable guest kick virtio-blk: Reset device after blk_cleanup_queue() virtio-blk: Use block layer provided spinlock drivers/block/virtio_blk.c | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) -- 1.7.10.2

This patch set fixes the race when hot-unplug stressed disk. Asias He (3): virtio-blk: Call del_gendisk() before disable guest kick virtio-blk: Reset device after blk_cleanup_queue() virtio-blk: Use block layer provided spinlock drivers/block/virtio_blk.c | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) -- 1.7.10.2

...when trying to figure out whether the error handling in > > virtio_blk was correct: > > > > Shouldn't you try to cleanup/rewind so that any structures are in a > > sane state after failure? The caller doesn't know where device_add_disk > > failed, and calling del_gendisk unconditionally like virtio_blk does is > > probably not the right thing to do (at the very least, I don't think > > unregistering a device that has not been registered is likely to work). > > > > Yes, I think all callers need to be reviewed before device_add_disk do t...

...when trying to figure out whether the error handling in > > virtio_blk was correct: > > > > Shouldn't you try to cleanup/rewind so that any structures are in a > > sane state after failure? The caller doesn't know where device_add_disk > > failed, and calling del_gendisk unconditionally like virtio_blk does is > > probably not the right thing to do (at the very least, I don't think > > unregistering a device that has not been registered is likely to work). > > > > Yes, I think all callers need to be reviewed before device_add_disk do t...

...disk); > + return retval; > } Noticed this when trying to figure out whether the error handling in virtio_blk was correct: Shouldn't you try to cleanup/rewind so that any structures are in a sane state after failure? The caller doesn't know where device_add_disk failed, and calling del_gendisk unconditionally like virtio_blk does is probably not the right thing to do (at the very least, I don't think unregistering a device that has not been registered is likely to work).

...disk); > + return retval; > } Noticed this when trying to figure out whether the error handling in virtio_blk was correct: Shouldn't you try to cleanup/rewind so that any structures are in a sane state after failure? The caller doesn't know where device_add_disk failed, and calling del_gendisk unconditionally like virtio_blk does is probably not the right thing to do (at the very least, I don't think unregistering a device that has not been registered is likely to work).

Hi, here is an updated patch-set to my v2 RFC virtio: add new notify() callback to virtio_driver This RFC introduces a new virtio_device entry 'surprize_removal' instead of a new 'notify' callback in struct virtio_driver. When an active virtio block device is hot-unplugged from a KVM guest, affected guest user applications are not aware of any errors that occur due to the lost

Hi, here is an updated patch-set to my v2 RFC virtio: add new notify() callback to virtio_driver This RFC introduces a new virtio_device entry 'surprize_removal' instead of a new 'notify' callback in struct virtio_driver. When an active virtio block device is hot-unplugged from a KVM guest, affected guest user applications are not aware of any errors that occur due to the lost

...backend driver virtio_blk. > > Question: I guess remove callback is invoked eventually? > Could you please clarify why isn't this sufficient? > yes, the remove callback is invoked lateron, and it could be done there. However, it should be done conditionally, and prior to invoking del_gendisk() (which triggers final I/O). We would still have the need for such notification information. The remove callback is also invoked when a device is set offline, and in that case we don't want a queue to reject further requests. The way it is done right doesn't affect the remove callback....

...backend driver virtio_blk. > > Question: I guess remove callback is invoked eventually? > Could you please clarify why isn't this sufficient? > yes, the remove callback is invoked lateron, and it could be done there. However, it should be done conditionally, and prior to invoking del_gendisk() (which triggers final I/O). We would still have the need for such notification information. The remove callback is also invoked when a device is set offline, and in that case we don't want a queue to reject further requests. The way it is done right doesn't affect the remove callback....

Code is added to the remove callback to verify if a device was lost. In case of a device loss further request queueing should be prevented by setting appropriate queue flags prior to invoking del_gendisk(). Blocking of request queueing leads to appropriate I/O errors when data are tried to be synched. Trying to synch data to a lost block device doesn't make too much sense. If the current remove callback was triggered due to an unregister driver, and the surprize_removal is not already set (alt...