| Message ID | 20241112125821.1475793-1-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-blk: don't keep queue frozen during system suspend | expand |
On Tue, Nov 12, 2024 at 08:58:21PM +0800, Ming Lei wrote: > Commit 4ce6e2db00de ("virtio-blk: Ensure no requests in virtqueues before > deleting vqs.") replaces queue quiesce with queue freeze in virtio-blk's > PM callbacks. And the motivation is to drain inflight IOs before suspending. > > block layer's queue freeze looks very handy, but it is also easy to cause > deadlock, such as, any attempt to call into bio_queue_enter() may run into > deadlock if the queue is frozen in current context. There are all kinds > of ->suspend() called in suspend context, so keeping queue frozen in the > whole suspend context isn't one good idea. And Marek reported lockdep > warning[1] caused by virtio-blk's freeze queue in virtblk_freeze(). > > [1] https://lore.kernel.org/linux-block/ca16370e-d646-4eee-b9cc-87277c89c43c@samsung.com/ > > Given the motivation is to drain in-flight IOs, it can be done by calling > freeze & unfreeze, meantime restore to previous behavior by keeping queue > quiesced during suspend. > > Cc: Yi Sun <yi.sun@unisoc.com> > Cc: Michael S. Tsirkin <mst@redhat.com> > Cc: Jason Wang <jasowang@redhat.com> > Cc: Stefan Hajnoczi <stefanha@redhat.com> > Cc: virtualization@lists.linux.dev > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Ming Lei <ming.lei@redhat.com> > --- > drivers/block/virtio_blk.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
On Tue, 12 Nov 2024 20:58:21 +0800, Ming Lei wrote: > Commit 4ce6e2db00de ("virtio-blk: Ensure no requests in virtqueues before > deleting vqs.") replaces queue quiesce with queue freeze in virtio-blk's > PM callbacks. And the motivation is to drain inflight IOs before suspending. > > block layer's queue freeze looks very handy, but it is also easy to cause > deadlock, such as, any attempt to call into bio_queue_enter() may run into > deadlock if the queue is frozen in current context. There are all kinds > of ->suspend() called in suspend context, so keeping queue frozen in the > whole suspend context isn't one good idea. And Marek reported lockdep > warning[1] caused by virtio-blk's freeze queue in virtblk_freeze(). > > [...] Applied, thanks! [1/1] virtio-blk: don't keep queue frozen during system suspend commit: 7678abee0867e6b7fb89aa40f6e9f575f755fb37 Best regards,
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 0e99a4714928..25a0a85a19fc 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -1591,9 +1591,12 @@ static void virtblk_remove(struct virtio_device *vdev) static int virtblk_freeze(struct virtio_device *vdev) { struct virtio_blk *vblk = vdev->priv; + struct request_queue *q = vblk->disk->queue; /* Ensure no requests in virtqueues before deleting vqs. */ - blk_mq_freeze_queue(vblk->disk->queue); + blk_mq_freeze_queue(q); + blk_mq_quiesce_queue_nowait(q); + blk_mq_unfreeze_queue(q); /* Ensure we don't receive any more interrupts */ virtio_reset_device(vdev); @@ -1617,8 +1620,8 @@ static int virtblk_restore(struct virtio_device *vdev) return ret; virtio_device_ready(vdev); + blk_mq_unquiesce_queue(vblk->disk->queue); - blk_mq_unfreeze_queue(vblk->disk->queue); return 0; } #endif
Commit 4ce6e2db00de ("virtio-blk: Ensure no requests in virtqueues before deleting vqs.") replaces queue quiesce with queue freeze in virtio-blk's PM callbacks. And the motivation is to drain inflight IOs before suspending. block layer's queue freeze looks very handy, but it is also easy to cause deadlock, such as, any attempt to call into bio_queue_enter() may run into deadlock if the queue is frozen in current context. There are all kinds of ->suspend() called in suspend context, so keeping queue frozen in the whole suspend context isn't one good idea. And Marek reported lockdep warning[1] caused by virtio-blk's freeze queue in virtblk_freeze(). [1] https://lore.kernel.org/linux-block/ca16370e-d646-4eee-b9cc-87277c89c43c@samsung.com/ Given the motivation is to drain in-flight IOs, it can be done by calling freeze & unfreeze, meantime restore to previous behavior by keeping queue quiesced during suspend. Cc: Yi Sun <yi.sun@unisoc.com> Cc: Michael S. Tsirkin <mst@redhat.com> Cc: Jason Wang <jasowang@redhat.com> Cc: Stefan Hajnoczi <stefanha@redhat.com> Cc: virtualization@lists.linux.dev Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/block/virtio_blk.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)