* Re: [RFC PATCH] scsi: virtio_scsi: bound EH timer resets to avoid unkillable hang
[not found] <20260911163509.684191F000FF@smtp.kernel.org>
@ 2026-09-11 16:44 ` Nguyen Ngoc Thang
2026-09-14 9:17 ` Hannes Reinecke
0 siblings, 1 reply; 10+ messages in thread
From: Nguyen Ngoc Thang @ 2026-09-11 16:44 UTC (permalink / raw)
To: mst, jasowangio, mkp, James.Bottomley
Cc: pbonzini, stefanha, eperezma, virtualization, linux-scsi,
linux-kernel, sashiko-bot
Thank you for the review -- the eh_host_reset_handler finding is correct,
and the underlying mechanism is worse than a stale read.
Once virtscsi_eh_timed_out() lets SCSI EH run to completion on an
unresponsive host, scsi_eh_bus_device_reset() leaves the command in
work_q (device reset fails the same way abort does, via the same bounded
virtscsi_tmf()), and since virtio_scsi implements neither
eh_target_reset_handler, eh_bus_reset_handler nor eh_host_reset_handler,
scsi_eh_target_reset()/scsi_eh_bus_reset()/scsi_eh_host_reset() all fail
immediately (scsi_try_*_reset() return FAILED when the handler pointer is
NULL) and the command falls through to scsi_eh_offline_sdevs(), which
calls scsi_eh_finish_cmd() and frees the tag back to the block layer.
The struct virtio_scsi_cmd for that command lives in scsi_cmd_priv(),
i.e. inside the now-freed request. If a new command is queued on the
same tag, it gets the same address. If the host was only very slow --
not actually dead -- and eventually completes the *original* descriptor,
virtscsi_complete_cmd() will read cmd->sc from that shared address and
call scsi_done() on whatever command currently occupies it, which by
then is the live, unrelated command still in flight on that tag. That's
a double completion on a request the driver itself has not finished,
not merely a read of stale memory.
The original virtscsi_eh_timed_out() comment ("The host guarantees to
respond to each command... Reset the timer unconditionally") reads, in
this light, less like an optimistic assumption and more like the
invariant that kept this reachable at all: as long as EH could never
progress past abort/device-reset into offline+free, the tag-reuse race
had no opening. My patch removes that invariant to fix the wbt hang, but
doesn't supply a replacement, which is the hole you're pointing at.
What I looked at as a replacement and why I'm not sending it as a v2 yet:
Adding eh_host_reset_handler that mirrors virtscsi_freeze()/
virtscsi_restore() (virtscsi_remove_vqs() + virtscsi_init()) was my first
instinct. virtio_reset_device() does call virtio_break_device() +
virtio_synchronize_cbs() before dev->config->reset(), which is meant to
guarantee no vq callback is still in flight when del_vqs() runs -- but
that pairing is compiled in only under CONFIG_VIRTIO_HARDEN_NOTIFICATION
(drivers/virtio/virtio.c:255-264), so the safety property isn't universal.
Separately, a host reset triggered by one wedged command on one LUN would
tear down and reinitialize every virtqueue on the host, silently
abandoning any genuinely in-flight, unrelated I/O on other LUNs/targets
that scsi_error_handler() never queued for recovery in the first place --
freeze/restore can rely on PM having fully quiesced the block queues
first; EH's SHOST_RECOVERY only blocks new submissions, it doesn't drain
what's already dispatched. I don't want to trade the tag-reuse race for
a config-dependent fix plus silent collateral I/O loss without your
input on whether that tradeoff is acceptable or whether there's a
narrower primitive intended for this.
Is virtio_break_device() (independent of host reset, and independent of
CONFIG_VIRTIO_HARDEN_NOTIFICATION) the intended way to make a single
wedged command's descriptor permanently safe to let go of, without
resetting the whole device? It looks close -- virtqueue_get_buf() checks
vq->broken before touching the used ring at all -- but it's one-way (no
unbreak short of recreating the vq), so I'm not certain it's meant to be
used outside device teardown. Has this class of problem (bound a SCSI EH
timeout on a transport that can't cheaply prove a command is gone) come
up for virtio_scsi or virtio_blk before, and is there a pattern I should
be following instead of inventing one?
Happy to do the legwork on whichever direction you point at -- I have a
QEMU virtio-scsi repro harness already wired up for the original hang
(clearing PCI_COMMAND_MASTER mid-write) that I can extend to exercise
the recovery path too.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] scsi: virtio_scsi: bound EH timer resets to avoid unkillable hang
2026-09-11 16:44 ` [RFC PATCH] scsi: virtio_scsi: bound EH timer resets to avoid unkillable hang Nguyen Ngoc Thang
@ 2026-09-14 9:17 ` Hannes Reinecke
2026-09-14 13:31 ` [PATCH v2] scsi: virtio_scsi: implement eh_host_reset_handler Nguyen Ngoc Thang
2026-09-14 13:36 ` [RFC PATCH] scsi: virtio_scsi: bound EH timer resets to avoid unkillable hang Nguyen Ngoc Thang
0 siblings, 2 replies; 10+ messages in thread
From: Hannes Reinecke @ 2026-09-14 9:17 UTC (permalink / raw)
To: Nguyen Ngoc Thang, mst, jasowangio, mkp, James.Bottomley
Cc: pbonzini, stefanha, eperezma, virtualization, linux-scsi,
linux-kernel, sashiko-bot
On 9/11/26 6:44 PM, Nguyen Ngoc Thang wrote:
> Thank you for the review -- the eh_host_reset_handler finding is correct,
> and the underlying mechanism is worse than a stale read.
>
> Once virtscsi_eh_timed_out() lets SCSI EH run to completion on an
> unresponsive host, scsi_eh_bus_device_reset() leaves the command in
> work_q (device reset fails the same way abort does, via the same bounded
> virtscsi_tmf()), and since virtio_scsi implements neither
> eh_target_reset_handler, eh_bus_reset_handler nor eh_host_reset_handler,
> scsi_eh_target_reset()/scsi_eh_bus_reset()/scsi_eh_host_reset() all fail
> immediately (scsi_try_*_reset() return FAILED when the handler pointer is
> NULL) and the command falls through to scsi_eh_offline_sdevs(), which
> calls scsi_eh_finish_cmd() and frees the tag back to the block layer.
>
[ .. ]
>
> Happy to do the legwork on whichever direction you point at -- I have a
> QEMU virtio-scsi repro harness already wired up for the original hang
> (clearing PCI_COMMAND_MASTER mid-write) that I can extend to exercise
> the recovery path too.
>
In short: you can't.
The linux kernel lacks the ability to abort an outstanding I/O from
userspace; userspace (and that includes qemu) _has_ to wait for that
I/O to return.
Otherwise there is no guarantee that the DMA regions of that command
are ever freed, and the kernel can happily scribble over memory which
userspace already assumed to be free. As these DMA regions are mapped
into the qemu guest you essentially allow the kernel to overwrite guest
memory at any time.
Not a good idea.
In general: SCSI host_reset _is_ the bug hammer. If that doesn't work
(for whatever reason) you system is hosed as you can _never_ get the
memory and DMA regions of outstanding commands back to the system.
So the better fix would be to implement a host_reset() callback for
virtio-scsi which would reset the virtio setup, terminating all
outstanding commands and reset the queues.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] scsi: virtio_scsi: implement eh_host_reset_handler
2026-09-14 9:17 ` Hannes Reinecke
@ 2026-09-14 13:31 ` Nguyen Ngoc Thang
2026-11-02 11:48 ` Michael S. Tsirkin
2026-11-02 20:06 ` Stefan Hajnoczi
2026-09-14 13:36 ` [RFC PATCH] scsi: virtio_scsi: bound EH timer resets to avoid unkillable hang Nguyen Ngoc Thang
1 sibling, 2 replies; 10+ messages in thread
From: Nguyen Ngoc Thang @ 2026-09-14 13:31 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Nguyen Ngoc Thang, mst, jasowangio, mkp, James.Bottomley,
pbonzini, stefanha, eperezma, virtualization, linux-scsi,
linux-kernel, sashiko-bot
Once virtscsi_eh_timed_out() lets SCSI EH run on an unresponsive host,
abort and device reset both fail the same way (same dead ctrl vq), and
since virtio_scsi implements neither target nor bus reset, EH falls
through to scsi_eh_offline_sdevs(), which frees the stuck commands' DMA
buffers without any guarantee the device has actually stopped touching
them. If bus mastering (or whatever broke the transport) comes back
later, the device can still write into memory the kernel has already
reused.
Add a real eh_host_reset_handler instead. virtio_reset_device() is the
guarantee scsi_eh_offline_sdevs() was missing: once it returns, the
device will not touch guest memory again, so it is safe to hand
outstanding commands' buffers back. Reuse virtscsi_remove_vqs() +
virtscsi_init() -- the exact sequence already used across
suspend/resume -- to tear the virtqueues down and rebuild them.
Verified with a QEMU virtio-scsi repro (PCI_COMMAND_MASTER cleared
mid-write): EH now runs abort -> device reset -> host reset -> abort ->
device reset -> host reset -> offline, then dd's stuck fsync returns
EIO and D-state drains to 0, in ~244s total. No task remains
uninterruptibly blocked.
Suggested-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
v1 -> v2: implemented eh_host_reset_handler as suggested by Hannes
Reinecke in review of the original RFC, instead of relying solely on
the EH loop falling through to offline.
---
drivers/scsi/virtio_scsi.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index b4f20c487718..9698f4f91cc6 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -833,6 +833,8 @@ static enum scsi_timeout_action virtscsi_eh_timed_out(struct scsi_cmnd *scmnd)
return SCSI_EH_NOT_HANDLED;
}
+static int virtscsi_host_reset(struct scsi_cmnd *sc);
+
static const struct scsi_host_template virtscsi_host_template = {
.module = THIS_MODULE,
.name = "Virtio SCSI HBA",
@@ -846,6 +848,7 @@ static const struct scsi_host_template virtscsi_host_template = {
.eh_abort_handler = virtscsi_abort,
.eh_device_reset_handler = virtscsi_device_reset,
.eh_timed_out = virtscsi_eh_timed_out,
+ .eh_host_reset_handler = virtscsi_host_reset,
.sdev_init = virtscsi_device_alloc,
.dma_boundary = UINT_MAX,
@@ -947,6 +950,27 @@ static int virtscsi_init(struct virtio_device *vdev,
return err;
}
+/*
+ * No bus/target reset in virtio-scsi, so EH lands here after device
+ * reset also times out. A virtio reset guarantees the device won't
+ * touch guest memory again, so stuck commands can be freed safely.
+ * Same teardown/rebuild already used for suspend/resume.
+ */
+static int virtscsi_host_reset(struct scsi_cmnd *sc)
+{
+ struct virtio_scsi *vscsi = shost_priv(sc->device->host);
+ struct virtio_device *vdev = vscsi->vdev;
+
+ scmd_printk(KERN_INFO, sc, "host reset\n");
+
+ virtscsi_remove_vqs(vdev);
+ if (virtscsi_init(vdev, vscsi))
+ return FAILED;
+
+ virtio_device_ready(vdev);
+ return SUCCESS;
+}
+
static int virtscsi_probe(struct virtio_device *vdev)
{
struct Scsi_Host *shost;
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] scsi: virtio_scsi: bound EH timer resets to avoid unkillable hang
2026-09-14 9:17 ` Hannes Reinecke
2026-09-14 13:31 ` [PATCH v2] scsi: virtio_scsi: implement eh_host_reset_handler Nguyen Ngoc Thang
@ 2026-09-14 13:36 ` Nguyen Ngoc Thang
1 sibling, 0 replies; 10+ messages in thread
From: Nguyen Ngoc Thang @ 2026-09-14 13:36 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Nguyen Ngoc Thang, mst, jasowangio, mkp, James.Bottomley,
pbonzini, stefanha, eperezma, virtualization, linux-scsi,
linux-kernel, sashiko-bot
On 9/14/26 11:17 AM, Hannes Reinecke wrote:
> So the better fix would be to implement a host_reset() callback for
> virtio-scsi which would reset the virtio setup, terminating all
> outstanding commands and reset the queues.
Thanks for the suggestion, Hannes -- that's exactly what the patch
below does (also posted separately as [PATCH v2], Message-ID
<20260914133144.15509-1-ngocthang2710.1999@gmail.com>; linking it back
into this thread since the subject changed and a couple of mail
clients won't merge the two threads on their own).
It adds eh_host_reset_handler backed by virtio_reset_device(): once
virtio_reset_device() returns, the device is guaranteed to have
stopped touching guest memory, so it's safe to let
scsi_eh_offline_sdevs() free the stuck commands' DMA buffers
afterwards -- the guarantee that was missing before (falling through
to offline with neither target, bus, nor host reset implemented).
Reuses virtscsi_remove_vqs() + virtscsi_init(), the same
teardown/rebuild sequence already used across suspend/resume.
Verified against the same QEMU virtio-scsi repro (PCI_COMMAND_MASTER
cleared mid-write): EH now runs abort -> device reset -> host reset ->
abort -> device reset -> host reset -> offline, dd's stuck fsync
returns EIO, D-state drains to 0, ~244s total.
Suggested-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
drivers/scsi/virtio_scsi.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index b4f20c487718..9698f4f91cc6 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -833,6 +833,8 @@ static enum scsi_timeout_action virtscsi_eh_timed_out(struct scsi_cmnd *scmnd)
return SCSI_EH_NOT_HANDLED;
}
+static int virtscsi_host_reset(struct scsi_cmnd *sc);
+
static const struct scsi_host_template virtscsi_host_template = {
.module = THIS_MODULE,
.name = "Virtio SCSI HBA",
@@ -846,6 +848,7 @@ static const struct scsi_host_template virtscsi_host_template = {
.eh_abort_handler = virtscsi_abort,
.eh_device_reset_handler = virtscsi_device_reset,
.eh_timed_out = virtscsi_eh_timed_out,
+ .eh_host_reset_handler = virtscsi_host_reset,
.sdev_init = virtscsi_device_alloc,
.dma_boundary = UINT_MAX,
@@ -947,6 +950,27 @@ static int virtscsi_init(struct virtio_device *vdev,
return err;
}
+/*
+ * No bus/target reset in virtio-scsi, so EH lands here after device
+ * reset also times out. A virtio reset guarantees the device won't
+ * touch guest memory again, so stuck commands can be freed safely.
+ * Same teardown/rebuild already used for suspend/resume.
+ */
+static int virtscsi_host_reset(struct scsi_cmnd *sc)
+{
+ struct virtio_scsi *vscsi = shost_priv(sc->device->host);
+ struct virtio_device *vdev = vscsi->vdev;
+
+ scmd_printk(KERN_INFO, sc, "host reset\n");
+
+ virtscsi_remove_vqs(vdev);
+ if (virtscsi_init(vdev, vscsi))
+ return FAILED;
+
+ virtio_device_ready(vdev);
+ return SUCCESS;
+}
+
static int virtscsi_probe(struct virtio_device *vdev)
{
struct Scsi_Host *shost;
--
2.43.0
Thanks,
Thang
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] scsi: virtio_scsi: implement eh_host_reset_handler
2026-09-14 13:31 ` [PATCH v2] scsi: virtio_scsi: implement eh_host_reset_handler Nguyen Ngoc Thang
@ 2026-11-02 11:48 ` Michael S. Tsirkin
2026-11-02 20:06 ` Stefan Hajnoczi
1 sibling, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2026-11-02 11:48 UTC (permalink / raw)
To: Nguyen Ngoc Thang
Cc: Hannes Reinecke, jasowangio, mkp, James.Bottomley, pbonzini,
stefanha, eperezma, virtualization, linux-scsi, linux-kernel,
sashiko-bot
On Mon, Sep 14, 2026 at 08:31:44PM +0700, Nguyen Ngoc Thang wrote:
> Once virtscsi_eh_timed_out() lets SCSI EH run on an unresponsive host,
> abort and device reset both fail the same way (same dead ctrl vq), and
> since virtio_scsi implements neither target nor bus reset, EH falls
> through to scsi_eh_offline_sdevs(), which frees the stuck commands' DMA
> buffers without any guarantee the device has actually stopped touching
> them. If bus mastering (or whatever broke the transport) comes back
> later, the device can still write into memory the kernel has already
> reused.
>
> Add a real eh_host_reset_handler instead. virtio_reset_device() is the
> guarantee scsi_eh_offline_sdevs() was missing: once it returns, the
> device will not touch guest memory again, so it is safe to hand
> outstanding commands' buffers back. Reuse virtscsi_remove_vqs() +
> virtscsi_init() -- the exact sequence already used across
> suspend/resume -- to tear the virtqueues down and rebuild them.
>
> Verified with a QEMU virtio-scsi repro (PCI_COMMAND_MASTER cleared
> mid-write): EH now runs abort -> device reset -> host reset -> abort ->
> device reset -> host reset -> offline, then dd's stuck fsync returns
> EIO and D-state drains to 0, in ~244s total. No task remains
> uninterruptibly blocked.
>
> Suggested-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
This is not posted correctly:
- start a nbew thread do not reply
- against some tree which is not upstream
I papered over it
> ---
> v1 -> v2: implemented eh_host_reset_handler as suggested by Hannes
> Reinecke in review of the original RFC, instead of relying solely on
> the EH loop falling through to offline.
> ---
> drivers/scsi/virtio_scsi.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index b4f20c487718..9698f4f91cc6 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -833,6 +833,8 @@ static enum scsi_timeout_action virtscsi_eh_timed_out(struct scsi_cmnd *scmnd)
> return SCSI_EH_NOT_HANDLED;
> }
>
> +static int virtscsi_host_reset(struct scsi_cmnd *sc);
> +
so move the function here?
> static const struct scsi_host_template virtscsi_host_template = {
> .module = THIS_MODULE,
> .name = "Virtio SCSI HBA",
> @@ -846,6 +848,7 @@ static const struct scsi_host_template virtscsi_host_template = {
> .eh_abort_handler = virtscsi_abort,
> .eh_device_reset_handler = virtscsi_device_reset,
> .eh_timed_out = virtscsi_eh_timed_out,
> + .eh_host_reset_handler = virtscsi_host_reset,
> .sdev_init = virtscsi_device_alloc,
>
> .dma_boundary = UINT_MAX,
> @@ -947,6 +950,27 @@ static int virtscsi_init(struct virtio_device *vdev,
> return err;
> }
>
> +/*
> + * No bus/target reset in virtio-scsi, so EH lands here after device
> + * reset also times out. A virtio reset guarantees the device won't
> + * touch guest memory again, so stuck commands can be freed safely.
> + * Same teardown/rebuild already used for suspend/resume.
> + */
> +static int virtscsi_host_reset(struct scsi_cmnd *sc)
> +{
> + struct virtio_scsi *vscsi = shost_priv(sc->device->host);
> + struct virtio_device *vdev = vscsi->vdev;
> +
> + scmd_printk(KERN_INFO, sc, "host reset\n");
> +
> + virtscsi_remove_vqs(vdev);
> + if (virtscsi_init(vdev, vscsi))
> + return FAILED;
> +
> + virtio_device_ready(vdev);
> + return SUCCESS;
> +}
> +
> static int virtscsi_probe(struct virtio_device *vdev)
> {
> struct Scsi_Host *shost;
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] scsi: virtio_scsi: implement eh_host_reset_handler
2026-09-14 13:31 ` [PATCH v2] scsi: virtio_scsi: implement eh_host_reset_handler Nguyen Ngoc Thang
2026-11-02 11:48 ` Michael S. Tsirkin
@ 2026-11-02 20:06 ` Stefan Hajnoczi
2026-11-03 7:27 ` Hannes Reinecke
1 sibling, 1 reply; 10+ messages in thread
From: Stefan Hajnoczi @ 2026-11-02 20:06 UTC (permalink / raw)
To: Nguyen Ngoc Thang
Cc: Hannes Reinecke, mst, jasowangio, mkp, James.Bottomley, pbonzini,
eperezma, virtualization, linux-scsi, linux-kernel, sashiko-bot,
kwolf
On Mon, Sep 14, 2026 at 08:31:44PM +0700, Nguyen Ngoc Thang wrote:
> Once virtscsi_eh_timed_out() lets SCSI EH run on an unresponsive host,
> abort and device reset both fail the same way (same dead ctrl vq), and
> since virtio_scsi implements neither target nor bus reset, EH falls
> through to scsi_eh_offline_sdevs(), which frees the stuck commands' DMA
> buffers without any guarantee the device has actually stopped touching
> them. If bus mastering (or whatever broke the transport) comes back
> later, the device can still write into memory the kernel has already
> reused.
>
> Add a real eh_host_reset_handler instead. virtio_reset_device() is the
> guarantee scsi_eh_offline_sdevs() was missing: once it returns, the
> device will not touch guest memory again, so it is safe to hand
> outstanding commands' buffers back. Reuse virtscsi_remove_vqs() +
> virtscsi_init() -- the exact sequence already used across
> suspend/resume -- to tear the virtqueues down and rebuild them.
>
> Verified with a QEMU virtio-scsi repro (PCI_COMMAND_MASTER cleared
> mid-write): EH now runs abort -> device reset -> host reset -> abort ->
> device reset -> host reset -> offline, then dd's stuck fsync returns
> EIO and D-state drains to 0, in ~244s total. No task remains
> uninterruptibly blocked.
By the way, there is a known limitation in the QEMU virtio-scsi
implementation where some reset-related operations are implemented
synchronously (can block vCPUs and QEMU activity while waiting for I/O
to drain in some situations) instead of asynchronously. Recently there
has been work in QEMU to make VIRTIO device reset asynchronous, for
example, but it needs to be implemented on a per-device basis and
virtio-scsi hasn't taken advantage of it yet.
Please let me know if you'd like to discuss or contribute. It's
something that is on my radar but not until 2027.
>
> Suggested-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> ---
> v1 -> v2: implemented eh_host_reset_handler as suggested by Hannes
> Reinecke in review of the original RFC, instead of relying solely on
> the EH loop falling through to offline.
> ---
> drivers/scsi/virtio_scsi.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index b4f20c487718..9698f4f91cc6 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -833,6 +833,8 @@ static enum scsi_timeout_action virtscsi_eh_timed_out(struct scsi_cmnd *scmnd)
> return SCSI_EH_NOT_HANDLED;
> }
>
> +static int virtscsi_host_reset(struct scsi_cmnd *sc);
> +
> static const struct scsi_host_template virtscsi_host_template = {
> .module = THIS_MODULE,
> .name = "Virtio SCSI HBA",
> @@ -846,6 +848,7 @@ static const struct scsi_host_template virtscsi_host_template = {
> .eh_abort_handler = virtscsi_abort,
> .eh_device_reset_handler = virtscsi_device_reset,
> .eh_timed_out = virtscsi_eh_timed_out,
> + .eh_host_reset_handler = virtscsi_host_reset,
> .sdev_init = virtscsi_device_alloc,
>
> .dma_boundary = UINT_MAX,
> @@ -947,6 +950,27 @@ static int virtscsi_init(struct virtio_device *vdev,
> return err;
> }
>
> +/*
> + * No bus/target reset in virtio-scsi, so EH lands here after device
The choice to use the VIRTIO device reset in virtscsi_host_reset() looks
good to me, but I would like clarification on this comment. virtio-scsi
does have I_T_NEXUS_RESET and I believe this is the modern equivalent to
the pre-SCSI-6 TARGET RESET. Is it really the case that virtio-scsi
doesn't have target reset? And do you want to go ahead and implement
.eh_target_reset_handler() using I_T_NEXUS_RESET?
> + * reset also times out. A virtio reset guarantees the device won't
> + * touch guest memory again, so stuck commands can be freed safely.
> + * Same teardown/rebuild already used for suspend/resume.
> + */
> +static int virtscsi_host_reset(struct scsi_cmnd *sc)
> +{
> + struct virtio_scsi *vscsi = shost_priv(sc->device->host);
> + struct virtio_device *vdev = vscsi->vdev;
> +
> + scmd_printk(KERN_INFO, sc, "host reset\n");
> +
> + virtscsi_remove_vqs(vdev);
This is synchronous and may take a long time. I noticed that some SCSI
drivers have a timeout that returns FAILED when the low-level host reset
operation takes too long. This can be done here too but would require
adding a timeout to virtio_reset_device() (i.e. see the virtio-pci
implementation that loops until the device register indicates reset is
complete).
> + if (virtscsi_init(vdev, vscsi))
> + return FAILED;
> +
> + virtio_device_ready(vdev);
Missing virtscsi_kick_event_all() like in virtscsi_restore()?
> + return SUCCESS;
> +}
> +
> static int virtscsi_probe(struct virtio_device *vdev)
> {
> struct Scsi_Host *shost;
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] scsi: virtio_scsi: implement eh_host_reset_handler
2026-11-02 20:06 ` Stefan Hajnoczi
@ 2026-11-03 7:27 ` Hannes Reinecke
2026-11-03 13:51 ` Stefan Hajnoczi
0 siblings, 1 reply; 10+ messages in thread
From: Hannes Reinecke @ 2026-11-03 7:27 UTC (permalink / raw)
To: Stefan Hajnoczi, Nguyen Ngoc Thang
Cc: mst, jasowangio, mkp, James.Bottomley, pbonzini, eperezma,
virtualization, linux-scsi, linux-kernel, sashiko-bot, kwolf
On 11/2/26 9:06 PM, Stefan Hajnoczi wrote:
> On Mon, Sep 14, 2026 at 08:31:44PM +0700, Nguyen Ngoc Thang wrote:
>> Once virtscsi_eh_timed_out() lets SCSI EH run on an unresponsive host,
>> abort and device reset both fail the same way (same dead ctrl vq), and
>> since virtio_scsi implements neither target nor bus reset, EH falls
>> through to scsi_eh_offline_sdevs(), which frees the stuck commands' DMA
>> buffers without any guarantee the device has actually stopped touching
>> them. If bus mastering (or whatever broke the transport) comes back
>> later, the device can still write into memory the kernel has already
>> reused.
>>
>> Add a real eh_host_reset_handler instead. virtio_reset_device() is the
>> guarantee scsi_eh_offline_sdevs() was missing: once it returns, the
>> device will not touch guest memory again, so it is safe to hand
>> outstanding commands' buffers back. Reuse virtscsi_remove_vqs() +
>> virtscsi_init() -- the exact sequence already used across
>> suspend/resume -- to tear the virtqueues down and rebuild them.
>>
>> Verified with a QEMU virtio-scsi repro (PCI_COMMAND_MASTER cleared
>> mid-write): EH now runs abort -> device reset -> host reset -> abort ->
>> device reset -> host reset -> offline, then dd's stuck fsync returns
>> EIO and D-state drains to 0, in ~244s total. No task remains
>> uninterruptibly blocked.
>
> By the way, there is a known limitation in the QEMU virtio-scsi
> implementation where some reset-related operations are implemented
> synchronously (can block vCPUs and QEMU activity while waiting for I/O
> to drain in some situations) instead of asynchronously. Recently there
> has been work in QEMU to make VIRTIO device reset asynchronous, for
> example, but it needs to be implemented on a per-device basis and
> virtio-scsi hasn't taken advantage of it yet.
>
> Please let me know if you'd like to discuss or contribute. It's
> something that is on my radar but not until 2027.
>
>>
>> Suggested-by: Hannes Reinecke <hare@suse.de>
>> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
>> ---
>> v1 -> v2: implemented eh_host_reset_handler as suggested by Hannes
>> Reinecke in review of the original RFC, instead of relying solely on
>> the EH loop falling through to offline.
>> ---
>> drivers/scsi/virtio_scsi.c | 24 ++++++++++++++++++++++++
>> 1 file changed, 24 insertions(+)
>>
>> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
>> index b4f20c487718..9698f4f91cc6 100644
>> --- a/drivers/scsi/virtio_scsi.c
>> +++ b/drivers/scsi/virtio_scsi.c
>> @@ -833,6 +833,8 @@ static enum scsi_timeout_action virtscsi_eh_timed_out(struct scsi_cmnd *scmnd)
>> return SCSI_EH_NOT_HANDLED;
>> }
>>
>> +static int virtscsi_host_reset(struct scsi_cmnd *sc);
>> +
>> static const struct scsi_host_template virtscsi_host_template = {
>> .module = THIS_MODULE,
>> .name = "Virtio SCSI HBA",
>> @@ -846,6 +848,7 @@ static const struct scsi_host_template virtscsi_host_template = {
>> .eh_abort_handler = virtscsi_abort,
>> .eh_device_reset_handler = virtscsi_device_reset,
>> .eh_timed_out = virtscsi_eh_timed_out,
>> + .eh_host_reset_handler = virtscsi_host_reset,
>> .sdev_init = virtscsi_device_alloc,
>>
>> .dma_boundary = UINT_MAX,
>> @@ -947,6 +950,27 @@ static int virtscsi_init(struct virtio_device *vdev,
>> return err;
>> }
>>
>> +/*
>> + * No bus/target reset in virtio-scsi, so EH lands here after device
>
> The choice to use the VIRTIO device reset in virtscsi_host_reset() looks
> good to me, but I would like clarification on this comment. virtio-scsi
> does have I_T_NEXUS_RESET and I believe this is the modern equivalent to
> the pre-SCSI-6 TARGET RESET. Is it really the case that virtio-scsi
> doesn't have target reset? And do you want to go ahead and implement
> .eh_target_reset_handler() using I_T_NEXUS_RESET?
>
Would probably not a bad idea to use I_T_NEXUS_RESET here.
Not sure what it does internally, though; _is_ there a nexus
to reset?
>> + * reset also times out. A virtio reset guarantees the device won't
>> + * touch guest memory again, so stuck commands can be freed safely.
>> + * Same teardown/rebuild already used for suspend/resume.
>> + */
>> +static int virtscsi_host_reset(struct scsi_cmnd *sc)
>> +{
>> + struct virtio_scsi *vscsi = shost_priv(sc->device->host);
>> + struct virtio_device *vdev = vscsi->vdev;
>> +
>> + scmd_printk(KERN_INFO, sc, "host reset\n");
>> +
>> + virtscsi_remove_vqs(vdev);
>
> This is synchronous and may take a long time. I noticed that some SCSI
> drivers have a timeout that returns FAILED when the low-level host reset
> operation takes too long. This can be done here too but would require
> adding a timeout to virtio_reset_device() (i.e. see the virtio-pci
> implementation that loops until the device register indicates reset is
> complete).
>
Never, ever, return FAILED from a host reset.
That will induce a non-recoverable error, and you have to reset the
OS to get the virtio devices back.
Not something you'd want to do I guess.
Yes, I know that host reset can take a long time. But normally
host reset would be the last resort, so really when you have to
call it your system has exhausted all other options.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] scsi: virtio_scsi: implement eh_host_reset_handler
2026-11-03 7:27 ` Hannes Reinecke
@ 2026-11-03 13:51 ` Stefan Hajnoczi
2026-11-03 16:26 ` Hannes Reinecke
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Hajnoczi @ 2026-11-03 13:51 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Nguyen Ngoc Thang, mst, jasowangio, mkp, James.Bottomley,
pbonzini, eperezma, virtualization, linux-scsi, linux-kernel,
sashiko-bot, kwolf
On Tue, Nov 03, 2026 at 08:27:52AM +0100, Hannes Reinecke wrote:
> On 11/2/26 9:06 PM, Stefan Hajnoczi wrote:
> > On Mon, Sep 14, 2026 at 08:31:44PM +0700, Nguyen Ngoc Thang wrote:
> > > Once virtscsi_eh_timed_out() lets SCSI EH run on an unresponsive host,
> > > abort and device reset both fail the same way (same dead ctrl vq), and
> > > since virtio_scsi implements neither target nor bus reset, EH falls
> > > through to scsi_eh_offline_sdevs(), which frees the stuck commands' DMA
> > > buffers without any guarantee the device has actually stopped touching
> > > them. If bus mastering (or whatever broke the transport) comes back
> > > later, the device can still write into memory the kernel has already
> > > reused.
> > >
> > > Add a real eh_host_reset_handler instead. virtio_reset_device() is the
> > > guarantee scsi_eh_offline_sdevs() was missing: once it returns, the
> > > device will not touch guest memory again, so it is safe to hand
> > > outstanding commands' buffers back. Reuse virtscsi_remove_vqs() +
> > > virtscsi_init() -- the exact sequence already used across
> > > suspend/resume -- to tear the virtqueues down and rebuild them.
> > >
> > > Verified with a QEMU virtio-scsi repro (PCI_COMMAND_MASTER cleared
> > > mid-write): EH now runs abort -> device reset -> host reset -> abort ->
> > > device reset -> host reset -> offline, then dd's stuck fsync returns
> > > EIO and D-state drains to 0, in ~244s total. No task remains
> > > uninterruptibly blocked.
> >
> > By the way, there is a known limitation in the QEMU virtio-scsi
> > implementation where some reset-related operations are implemented
> > synchronously (can block vCPUs and QEMU activity while waiting for I/O
> > to drain in some situations) instead of asynchronously. Recently there
> > has been work in QEMU to make VIRTIO device reset asynchronous, for
> > example, but it needs to be implemented on a per-device basis and
> > virtio-scsi hasn't taken advantage of it yet.
> >
> > Please let me know if you'd like to discuss or contribute. It's
> > something that is on my radar but not until 2027.
> >
> > >
> > > Suggested-by: Hannes Reinecke <hare@suse.de>
> > > Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> > > ---
> > > v1 -> v2: implemented eh_host_reset_handler as suggested by Hannes
> > > Reinecke in review of the original RFC, instead of relying solely on
> > > the EH loop falling through to offline.
> > > ---
> > > drivers/scsi/virtio_scsi.c | 24 ++++++++++++++++++++++++
> > > 1 file changed, 24 insertions(+)
> > >
> > > diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> > > index b4f20c487718..9698f4f91cc6 100644
> > > --- a/drivers/scsi/virtio_scsi.c
> > > +++ b/drivers/scsi/virtio_scsi.c
> > > @@ -833,6 +833,8 @@ static enum scsi_timeout_action virtscsi_eh_timed_out(struct scsi_cmnd *scmnd)
> > > return SCSI_EH_NOT_HANDLED;
> > > }
> > > +static int virtscsi_host_reset(struct scsi_cmnd *sc);
> > > +
> > > static const struct scsi_host_template virtscsi_host_template = {
> > > .module = THIS_MODULE,
> > > .name = "Virtio SCSI HBA",
> > > @@ -846,6 +848,7 @@ static const struct scsi_host_template virtscsi_host_template = {
> > > .eh_abort_handler = virtscsi_abort,
> > > .eh_device_reset_handler = virtscsi_device_reset,
> > > .eh_timed_out = virtscsi_eh_timed_out,
> > > + .eh_host_reset_handler = virtscsi_host_reset,
> > > .sdev_init = virtscsi_device_alloc,
> > > .dma_boundary = UINT_MAX,
> > > @@ -947,6 +950,27 @@ static int virtscsi_init(struct virtio_device *vdev,
> > > return err;
> > > }
> > > +/*
> > > + * No bus/target reset in virtio-scsi, so EH lands here after device
> >
> > The choice to use the VIRTIO device reset in virtscsi_host_reset() looks
> > good to me, but I would like clarification on this comment. virtio-scsi
> > does have I_T_NEXUS_RESET and I believe this is the modern equivalent to
> > the pre-SCSI-6 TARGET RESET. Is it really the case that virtio-scsi
> > doesn't have target reset? And do you want to go ahead and implement
> > .eh_target_reset_handler() using I_T_NEXUS_RESET?
> >
> Would probably not a bad idea to use I_T_NEXUS_RESET here.
> Not sure what it does internally, though; _is_ there a nexus
> to reset?
Whether there is any benefit to I_T_NEXUS_RESET may depend on the device
implementation. In QEMU:
- LOGICAL_UNIT_RESET resets just 1 LUN
- I_T_NEXUS_RESET resets all LUNs
- VIRTIO device reset also resets the HBA itself
In practice QEMU's I_T_NEXUS_RESET is unlikely to fix things if
LOGICAL_UNIT_RESET wasn't sufficient. Maybe it will help on other device
implementations though.
I think the Linux driver mapping should be:
- .eh_device_reset_handler() -> LOGICAL_UNIT_RESET
- .eh_target_reset_handler() -> I_T_NEXUS_RESET
- .eh_host_reset_handler() -> VIRTIO device reset
> > > + * reset also times out. A virtio reset guarantees the device won't
> > > + * touch guest memory again, so stuck commands can be freed safely.
> > > + * Same teardown/rebuild already used for suspend/resume.
> > > + */
> > > +static int virtscsi_host_reset(struct scsi_cmnd *sc)
> > > +{
> > > + struct virtio_scsi *vscsi = shost_priv(sc->device->host);
> > > + struct virtio_device *vdev = vscsi->vdev;
> > > +
> > > + scmd_printk(KERN_INFO, sc, "host reset\n");
> > > +
> > > + virtscsi_remove_vqs(vdev);
> >
> > This is synchronous and may take a long time. I noticed that some SCSI
> > drivers have a timeout that returns FAILED when the low-level host reset
> > operation takes too long. This can be done here too but would require
> > adding a timeout to virtio_reset_device() (i.e. see the virtio-pci
> > implementation that loops until the device register indicates reset is
> > complete).
> >
> Never, ever, return FAILED from a host reset.
> That will induce a non-recoverable error, and you have to reset the
> OS to get the virtio devices back.
> Not something you'd want to do I guess.
>
> Yes, I know that host reset can take a long time. But normally
> host reset would be the last resort, so really when you have to
> call it your system has exhausted all other options.
Okay.
Stefan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] scsi: virtio_scsi: implement eh_host_reset_handler
2026-11-03 13:51 ` Stefan Hajnoczi
@ 2026-11-03 16:26 ` Hannes Reinecke
2026-11-03 21:43 ` Stefan Hajnoczi
0 siblings, 1 reply; 10+ messages in thread
From: Hannes Reinecke @ 2026-11-03 16:26 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Nguyen Ngoc Thang, mst, jasowangio, mkp, James.Bottomley,
pbonzini, eperezma, virtualization, linux-scsi, linux-kernel,
sashiko-bot, kwolf
On 11/3/26 2:51 PM, Stefan Hajnoczi wrote:
> On Tue, Nov 03, 2026 at 08:27:52AM +0100, Hannes Reinecke wrote:
>> On 11/2/26 9:06 PM, Stefan Hajnoczi wrote:
>>> On Mon, Sep 14, 2026 at 08:31:44PM +0700, Nguyen Ngoc Thang wrote:
>>>> Once virtscsi_eh_timed_out() lets SCSI EH run on an unresponsive host,
>>>> abort and device reset both fail the same way (same dead ctrl vq), and
>>>> since virtio_scsi implements neither target nor bus reset, EH falls
>>>> through to scsi_eh_offline_sdevs(), which frees the stuck commands' DMA
>>>> buffers without any guarantee the device has actually stopped touching
>>>> them. If bus mastering (or whatever broke the transport) comes back
>>>> later, the device can still write into memory the kernel has already
>>>> reused.
>>>>
>>>> Add a real eh_host_reset_handler instead. virtio_reset_device() is the
>>>> guarantee scsi_eh_offline_sdevs() was missing: once it returns, the
>>>> device will not touch guest memory again, so it is safe to hand
>>>> outstanding commands' buffers back. Reuse virtscsi_remove_vqs() +
>>>> virtscsi_init() -- the exact sequence already used across
>>>> suspend/resume -- to tear the virtqueues down and rebuild them.
>>>>
>>>> Verified with a QEMU virtio-scsi repro (PCI_COMMAND_MASTER cleared
>>>> mid-write): EH now runs abort -> device reset -> host reset -> abort ->
>>>> device reset -> host reset -> offline, then dd's stuck fsync returns
>>>> EIO and D-state drains to 0, in ~244s total. No task remains
>>>> uninterruptibly blocked.
>>>
>>> By the way, there is a known limitation in the QEMU virtio-scsi
>>> implementation where some reset-related operations are implemented
>>> synchronously (can block vCPUs and QEMU activity while waiting for I/O
>>> to drain in some situations) instead of asynchronously. Recently there
>>> has been work in QEMU to make VIRTIO device reset asynchronous, for
>>> example, but it needs to be implemented on a per-device basis and
>>> virtio-scsi hasn't taken advantage of it yet.
>>>
>>> Please let me know if you'd like to discuss or contribute. It's
>>> something that is on my radar but not until 2027.
>>>
>>>>
>>>> Suggested-by: Hannes Reinecke <hare@suse.de>
>>>> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
>>>> ---
>>>> v1 -> v2: implemented eh_host_reset_handler as suggested by Hannes
>>>> Reinecke in review of the original RFC, instead of relying solely on
>>>> the EH loop falling through to offline.
>>>> ---
>>>> drivers/scsi/virtio_scsi.c | 24 ++++++++++++++++++++++++
>>>> 1 file changed, 24 insertions(+)
>>>>
>>>> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
>>>> index b4f20c487718..9698f4f91cc6 100644
>>>> --- a/drivers/scsi/virtio_scsi.c
>>>> +++ b/drivers/scsi/virtio_scsi.c
>>>> @@ -833,6 +833,8 @@ static enum scsi_timeout_action virtscsi_eh_timed_out(struct scsi_cmnd *scmnd)
>>>> return SCSI_EH_NOT_HANDLED;
>>>> }
>>>> +static int virtscsi_host_reset(struct scsi_cmnd *sc);
>>>> +
>>>> static const struct scsi_host_template virtscsi_host_template = {
>>>> .module = THIS_MODULE,
>>>> .name = "Virtio SCSI HBA",
>>>> @@ -846,6 +848,7 @@ static const struct scsi_host_template virtscsi_host_template = {
>>>> .eh_abort_handler = virtscsi_abort,
>>>> .eh_device_reset_handler = virtscsi_device_reset,
>>>> .eh_timed_out = virtscsi_eh_timed_out,
>>>> + .eh_host_reset_handler = virtscsi_host_reset,
>>>> .sdev_init = virtscsi_device_alloc,
>>>> .dma_boundary = UINT_MAX,
>>>> @@ -947,6 +950,27 @@ static int virtscsi_init(struct virtio_device *vdev,
>>>> return err;
>>>> }
>>>> +/*
>>>> + * No bus/target reset in virtio-scsi, so EH lands here after device
>>>
>>> The choice to use the VIRTIO device reset in virtscsi_host_reset() looks
>>> good to me, but I would like clarification on this comment. virtio-scsi
>>> does have I_T_NEXUS_RESET and I believe this is the modern equivalent to
>>> the pre-SCSI-6 TARGET RESET. Is it really the case that virtio-scsi
>>> doesn't have target reset? And do you want to go ahead and implement
>>> .eh_target_reset_handler() using I_T_NEXUS_RESET?
>>>
>> Would probably not a bad idea to use I_T_NEXUS_RESET here.
>> Not sure what it does internally, though; _is_ there a nexus
>> to reset?
>
> Whether there is any benefit to I_T_NEXUS_RESET may depend on the device
> implementation. In QEMU:
> - LOGICAL_UNIT_RESET resets just 1 LUN
> - I_T_NEXUS_RESET resets all LUNs
> - VIRTIO device reset also resets the HBA itself
>
> In practice QEMU's I_T_NEXUS_RESET is unlikely to fix things if
> LOGICAL_UNIT_RESET wasn't sufficient. Maybe it will help on other device
> implementations though.
>
> I think the Linux driver mapping should be:
> - .eh_device_reset_handler() -> LOGICAL_UNIT_RESET
> - .eh_target_reset_handler() -> I_T_NEXUS_RESET
> - .eh_host_reset_handler() -> VIRTIO device reset
>
I guess 'target_reset' is pretty much pointless for modern setups.
If lun reset doesn't work it's pretty much guaranteed that target_reset
won't help, either.
(Unless you are on SAS, then it might, as I_T nexus reset actually
resets the SAS link and such does something useful).
Problem is that in most cases LUN reset fails as the _communication_
to the LUN is interrupted, and hence sending another TMF across the
same communication channel won't help, either.
For virtio that would mean that we'd have to reset the virtio queues;
can we even do that in a meaningful fashion?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] scsi: virtio_scsi: implement eh_host_reset_handler
2026-11-03 16:26 ` Hannes Reinecke
@ 2026-11-03 21:43 ` Stefan Hajnoczi
0 siblings, 0 replies; 10+ messages in thread
From: Stefan Hajnoczi @ 2026-11-03 21:43 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Nguyen Ngoc Thang, mst, jasowangio, mkp, James.Bottomley,
pbonzini, eperezma, virtualization, linux-scsi, linux-kernel,
sashiko-bot, kwolf
On Tue, Nov 03, 2026 at 05:26:44PM +0100, Hannes Reinecke wrote:
> On 11/3/26 2:51 PM, Stefan Hajnoczi wrote:
> > On Tue, Nov 03, 2026 at 08:27:52AM +0100, Hannes Reinecke wrote:
> > > On 11/2/26 9:06 PM, Stefan Hajnoczi wrote:
> > > > On Mon, Sep 14, 2026 at 08:31:44PM +0700, Nguyen Ngoc Thang wrote:
> > > > > Once virtscsi_eh_timed_out() lets SCSI EH run on an unresponsive host,
> > > > > abort and device reset both fail the same way (same dead ctrl vq), and
> > > > > since virtio_scsi implements neither target nor bus reset, EH falls
> > > > > through to scsi_eh_offline_sdevs(), which frees the stuck commands' DMA
> > > > > buffers without any guarantee the device has actually stopped touching
> > > > > them. If bus mastering (or whatever broke the transport) comes back
> > > > > later, the device can still write into memory the kernel has already
> > > > > reused.
> > > > >
> > > > > Add a real eh_host_reset_handler instead. virtio_reset_device() is the
> > > > > guarantee scsi_eh_offline_sdevs() was missing: once it returns, the
> > > > > device will not touch guest memory again, so it is safe to hand
> > > > > outstanding commands' buffers back. Reuse virtscsi_remove_vqs() +
> > > > > virtscsi_init() -- the exact sequence already used across
> > > > > suspend/resume -- to tear the virtqueues down and rebuild them.
> > > > >
> > > > > Verified with a QEMU virtio-scsi repro (PCI_COMMAND_MASTER cleared
> > > > > mid-write): EH now runs abort -> device reset -> host reset -> abort ->
> > > > > device reset -> host reset -> offline, then dd's stuck fsync returns
> > > > > EIO and D-state drains to 0, in ~244s total. No task remains
> > > > > uninterruptibly blocked.
> > > >
> > > > By the way, there is a known limitation in the QEMU virtio-scsi
> > > > implementation where some reset-related operations are implemented
> > > > synchronously (can block vCPUs and QEMU activity while waiting for I/O
> > > > to drain in some situations) instead of asynchronously. Recently there
> > > > has been work in QEMU to make VIRTIO device reset asynchronous, for
> > > > example, but it needs to be implemented on a per-device basis and
> > > > virtio-scsi hasn't taken advantage of it yet.
> > > >
> > > > Please let me know if you'd like to discuss or contribute. It's
> > > > something that is on my radar but not until 2027.
> > > >
> > > > >
> > > > > Suggested-by: Hannes Reinecke <hare@suse.de>
> > > > > Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> > > > > ---
> > > > > v1 -> v2: implemented eh_host_reset_handler as suggested by Hannes
> > > > > Reinecke in review of the original RFC, instead of relying solely on
> > > > > the EH loop falling through to offline.
> > > > > ---
> > > > > drivers/scsi/virtio_scsi.c | 24 ++++++++++++++++++++++++
> > > > > 1 file changed, 24 insertions(+)
> > > > >
> > > > > diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> > > > > index b4f20c487718..9698f4f91cc6 100644
> > > > > --- a/drivers/scsi/virtio_scsi.c
> > > > > +++ b/drivers/scsi/virtio_scsi.c
> > > > > @@ -833,6 +833,8 @@ static enum scsi_timeout_action virtscsi_eh_timed_out(struct scsi_cmnd *scmnd)
> > > > > return SCSI_EH_NOT_HANDLED;
> > > > > }
> > > > > +static int virtscsi_host_reset(struct scsi_cmnd *sc);
> > > > > +
> > > > > static const struct scsi_host_template virtscsi_host_template = {
> > > > > .module = THIS_MODULE,
> > > > > .name = "Virtio SCSI HBA",
> > > > > @@ -846,6 +848,7 @@ static const struct scsi_host_template virtscsi_host_template = {
> > > > > .eh_abort_handler = virtscsi_abort,
> > > > > .eh_device_reset_handler = virtscsi_device_reset,
> > > > > .eh_timed_out = virtscsi_eh_timed_out,
> > > > > + .eh_host_reset_handler = virtscsi_host_reset,
> > > > > .sdev_init = virtscsi_device_alloc,
> > > > > .dma_boundary = UINT_MAX,
> > > > > @@ -947,6 +950,27 @@ static int virtscsi_init(struct virtio_device *vdev,
> > > > > return err;
> > > > > }
> > > > > +/*
> > > > > + * No bus/target reset in virtio-scsi, so EH lands here after device
> > > >
> > > > The choice to use the VIRTIO device reset in virtscsi_host_reset() looks
> > > > good to me, but I would like clarification on this comment. virtio-scsi
> > > > does have I_T_NEXUS_RESET and I believe this is the modern equivalent to
> > > > the pre-SCSI-6 TARGET RESET. Is it really the case that virtio-scsi
> > > > doesn't have target reset? And do you want to go ahead and implement
> > > > .eh_target_reset_handler() using I_T_NEXUS_RESET?
> > > >
> > > Would probably not a bad idea to use I_T_NEXUS_RESET here.
> > > Not sure what it does internally, though; _is_ there a nexus
> > > to reset?
> >
> > Whether there is any benefit to I_T_NEXUS_RESET may depend on the device
> > implementation. In QEMU:
> > - LOGICAL_UNIT_RESET resets just 1 LUN
> > - I_T_NEXUS_RESET resets all LUNs
> > - VIRTIO device reset also resets the HBA itself
> >
> > In practice QEMU's I_T_NEXUS_RESET is unlikely to fix things if
> > LOGICAL_UNIT_RESET wasn't sufficient. Maybe it will help on other device
> > implementations though.
> >
> > I think the Linux driver mapping should be:
> > - .eh_device_reset_handler() -> LOGICAL_UNIT_RESET
> > - .eh_target_reset_handler() -> I_T_NEXUS_RESET
> > - .eh_host_reset_handler() -> VIRTIO device reset
> >
> I guess 'target_reset' is pretty much pointless for modern setups.
> If lun reset doesn't work it's pretty much guaranteed that target_reset
> won't help, either.
> (Unless you are on SAS, then it might, as I_T nexus reset actually
> resets the SAS link and such does something useful).
> Problem is that in most cases LUN reset fails as the _communication_
> to the LUN is interrupted, and hence sending another TMF across the
> same communication channel won't help, either.
> For virtio that would mean that we'd have to reset the virtio queues;
> can we even do that in a meaningful fashion?
The virtqueues can only really be reset by VIRTIO device reset
(.eh_host_reset_handler()).
Stefan
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-11-03 21:43 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20260911163509.684191F000FF@smtp.kernel.org>
2026-09-11 16:44 ` [RFC PATCH] scsi: virtio_scsi: bound EH timer resets to avoid unkillable hang Nguyen Ngoc Thang
2026-09-14 9:17 ` Hannes Reinecke
2026-09-14 13:31 ` [PATCH v2] scsi: virtio_scsi: implement eh_host_reset_handler Nguyen Ngoc Thang
2026-11-02 11:48 ` Michael S. Tsirkin
2026-11-02 20:06 ` Stefan Hajnoczi
2026-11-03 7:27 ` Hannes Reinecke
2026-11-03 13:51 ` Stefan Hajnoczi
2026-11-03 16:26 ` Hannes Reinecke
2026-11-03 21:43 ` Stefan Hajnoczi
2026-09-14 13:36 ` [RFC PATCH] scsi: virtio_scsi: bound EH timer resets to avoid unkillable hang Nguyen Ngoc Thang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®