From: Parav Pandit parav@nvidia.com Sent: Thursday, May 22, 2025 1:19 PM To: Max Gurtovoy mgurtovoy@nvidia.com; Israel Rukshin israelr@nvidia.com Cc: Parav Pandit parav@nvidia.com; stable@vger.kernel.org; NBU-Contact- Li Rongqing (EXTERNAL) lirongqing@baidu.com Subject: [PATCH v6] virtio_blk: Fix disk deletion hang on device surprise removal
When the PCI device is surprise removed, requests may not complete the device as the VQ is marked as broken. Due to this, the disk deletion hangs.
Fix it by aborting the requests when the VQ is broken.
With this fix now fio completes swiftly. An alternative of IO timeout has been considered, however when the driver knows about unresponsive block device, swiftly clearing them enables users and upper layers to react quickly.
Verified with multiple device unplug iterations with pending requests in virtio used ring and some pending with the device.
Fixes: 43bb40c5b926 ("virtio_pci: Support surprise removal of virtio pci device") Cc: stable@vger.kernel.org Reported-by: Li RongQing lirongqing@baidu.com Closes: https://lore.kernel.org/virtualization/c45dd68698cd47238c55fb73ca9b4741 @baidu.com/ Signed-off-by: Parav Pandit parav@nvidia.com
This is an internal patch, which got CCed to stable by mistake. Please ignore this patch for stable kernels.
It is still under internal review. I am sorry for the noise.
v1->v2: (internal v5->v6):
- Addressed comments from Stephan
- fixed spelling to 'waiting'
v1->v2: (internal v4->v5):
- Addressed comments from MST
- removed the vq broken check in queue_rq(s)
drivers/block/virtio_blk.c | 85 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 7cffea01d868..04f24ec20405 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -1554,6 +1554,89 @@ static int virtblk_probe(struct virtio_device *vdev) return err; }
+static bool virtblk_request_cancel(struct request *rq, void *data) {
- struct virtblk_req *vbr = blk_mq_rq_to_pdu(rq);
- struct virtio_blk *vblk = data;
- struct virtio_blk_vq *vq;
- unsigned long flags;
- vq = &vblk->vqs[rq->mq_hctx->queue_num];
- spin_lock_irqsave(&vq->lock, flags);
- vbr->in_hdr.status = VIRTIO_BLK_S_IOERR;
- if (blk_mq_request_started(rq) && !blk_mq_request_completed(rq))
blk_mq_complete_request(rq);- spin_unlock_irqrestore(&vq->lock, flags);
- return true;
+}
+static void virtblk_broken_device_cleanup(struct virtio_blk *vblk) {
- struct request_queue *q = vblk->disk->queue;
- return;
- if (!virtqueue_is_broken(vblk->vqs[0].vq))
return;- /* Start freezing the queue, so that new requests keeps waiting at the
* door of bio_queue_enter(). We cannot fully freeze the queuebecause
* freezed queue is an empty queue and there are pending requests,so
* only start freezing it.*/- blk_freeze_queue_start(q);
- /* When quiescing completes, all ongoing dispatches have completed
* and no new dispatch will happen towards the driver.* This ensures that later when cancel is attempted, then are not* getting processed by the queue_rq() or queue_rqs() handlers.*/- blk_mq_quiesce_queue(q);
- /*
* Synchronize with any ongoing VQ callbacks, effectively quiescing* the device and preventing it from completing further requests* to the block layer. Any outstanding, incomplete requests will be* completed by virtblk_request_cancel().*/- virtio_synchronize_cbs(vblk->vdev);
- /* At this point, no new requests can enter the queue_rq() and
* completion routine will not complete any new requests either forthe
* broken vq. Hence, it is safe to cancel all requests which are* started.*/- blk_mq_tagset_busy_iter(&vblk->tag_set, virtblk_request_cancel,
vblk);
- blk_mq_tagset_wait_completed_request(&vblk->tag_set);
- /* All pending requests are cleaned up. Time to resume so that disk
* deletion can be smooth. Start the HW queues so that when queue is* unquiesced requests can again enter the driver.*/- blk_mq_start_stopped_hw_queues(q, true);
- /* Unquiescing will trigger dispatching any pending requests to the
* driver which has crossed bio_queue_enter() to the driver.*/- blk_mq_unquiesce_queue(q);
- /* Wait for all pending dispatches to terminate which may have been
* initiated after unquiescing.*/- blk_mq_freeze_queue_wait(q);
- /* Mark the disk dead so that once queue unfreeze, the requests
* waiting at the door of bio_queue_enter() can be aborted right away.*/- blk_mark_disk_dead(vblk->disk);
- /* Unfreeze the queue so that any waiting requests will be aborted. */
- blk_mq_unfreeze_queue_nomemrestore(q);
+}
static void virtblk_remove(struct virtio_device *vdev) { struct virtio_blk *vblk = vdev->priv; @@ -1561,6 +1644,8 @@ static void virtblk_remove(struct virtio_device *vdev) /* Make sure no work handler is accessing the device. */ flush_work(&vblk->config_work);
- virtblk_broken_device_cleanup(vblk);
- del_gendisk(vblk->disk); blk_mq_free_tag_set(&vblk->tag_set);
-- 2.34.1