sda abort with virtio-scsi

Commit Message

Hannes Reinecke Feb. 4, 2016, 1:54 p.m. UTC

On 02/04/2016 02:41 PM, Jim Minter wrote:
> FWIW, I've now done:
> 
> echo 300 >/sys/block/sda/device/timeout
> 
> Not entirely sure whether it would help or not, but so far I haven't
> had a recurrence.
> 
Can you try with the attached patch?
Should work as well, but you shouldn't need to tweak anything.

Cheers,

Hannes

Comments

Paolo Bonzini Feb. 4, 2016, 3:03 p.m. UTC | #1

> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index 595af1a..f712db5 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -543,6 +543,33 @@ static int virtscsi_abort(struct scsi_cmnd *sc)
>  	return virtscsi_tmf(vscsi, cmd);
>  }
>  
> +static enum blk_eh_timer_return virtscsi_timed_out(struct scsi_cmnd *sc)
> +{
> +	struct virtio_scsi *vscsi = shost_priv(sc->device->host);
> +	struct virtio_scsi_cmd *cmd;
> +
> +	scmd_printk(KERN_INFO, sc, "timeout\n");
> +	cmd = mempool_alloc(virtscsi_cmd_pool, GFP_NOIO);
> +	if (!cmd)
> +		return FAILED;

Should this be BLK_EH_NOT_HANDLED?  Apart from this, the patch looks
good.  And of course this doesn't affect Jim, who can test this patch
anyway.

Thanks!

Paolo

> +
> +	memset(cmd, 0, sizeof(*cmd));
> +	cmd->sc = sc;
> +	cmd->req.tmf = (struct virtio_scsi_ctrl_tmf_req){
> +		.type = VIRTIO_SCSI_T_TMF,
> +		.subtype = VIRTIO_SCSI_T_TMF_QUERY_TASK,
> +		.lun[0] = 1,
> +		.lun[1] = sc->device->id,
> +		.lun[2] = (sc->device->lun >> 8) | 0x40,
> +		.lun[3] = sc->device->lun & 0xff,
> +		.tag = (unsigned long)sc,
> +	};
> +	if (virtscsi_tmf(vscsi, cmd) == FAILED)
> +		return BLK_EH_NOT_HANDLED;
> +
> +	return BLK_EH_RESET_TIMER;
> +}
> +
>  static struct scsi_host_template virtscsi_host_template = {
>  	.module = THIS_MODULE,
>  	.name = "Virtio SCSI HBA",
> @@ -551,6 +578,7 @@ static struct scsi_host_template virtscsi_host_template = {
>  	.this_id = -1,
>  	.eh_abort_handler = virtscsi_abort,
>  	.eh_device_reset_handler = virtscsi_device_reset,
> +	.eh_timed_out = virtscsi_timed_out,
>  
>  	.can_queue = 1024,
>  	.dma_boundary = UINT_MAX,
> -- 1.8.5.6
>

Hannes Reinecke Feb. 4, 2016, 3:11 p.m. UTC | #2

On 02/04/2016 04:03 PM, Paolo Bonzini wrote:
>> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
>> index 595af1a..f712db5 100644
>> --- a/drivers/scsi/virtio_scsi.c
>> +++ b/drivers/scsi/virtio_scsi.c
>> @@ -543,6 +543,33 @@ static int virtscsi_abort(struct scsi_cmnd *sc)
>>  	return virtscsi_tmf(vscsi, cmd);
>>  }
>>  
>> +static enum blk_eh_timer_return virtscsi_timed_out(struct scsi_cmnd *sc)
>> +{
>> +	struct virtio_scsi *vscsi = shost_priv(sc->device->host);
>> +	struct virtio_scsi_cmd *cmd;
>> +
>> +	scmd_printk(KERN_INFO, sc, "timeout\n");
>> +	cmd = mempool_alloc(virtscsi_cmd_pool, GFP_NOIO);
>> +	if (!cmd)
>> +		return FAILED;
> 
> Should this be BLK_EH_NOT_HANDLED?  Apart from this, the patch looks
> good.  And of course this doesn't affect Jim, who can test this patch
> anyway.
> 
Why, but of course.

I'll be sending the official patch to linux-scsi.

Cheers,

Hannes

Patch

From 330b4f056957eed60cb3cd1bfa1486ba7db96d8d Mon Sep 17 00:00:00 2001
From: Hannes Reinecke <[e-mail]>
Date: Wed, 18 Nov 2015 10:33:13 +0100
Subject: [PATCH] virtio_scsi: Implement eh_timed_out callback

If a command timeout occurs on the host the SCSI error handling
is entered, which typically takes quite some time until to error
is resolved and the command can be returned to the guest.
This error handling typically overflows any timeout the guest
might have been set on that command, causing the guest to abort
the command.
However, command aborts from the guest is tricky, and, in most
cases, downright impossible.
So instead this patch implements an eh_timed_out callback to
virtio_scsi, which will issue a QUERY TASK TMF. If that succeeds
the command is still active on the host, and the guest command
timeout is reset.

References: bsc#936530

Signed-off-by: Hannes Reinecke <[e-mail]>
---
 drivers/scsi/virtio_scsi.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 595af1a..f712db5 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -543,6 +543,33 @@  static int virtscsi_abort(struct scsi_cmnd *sc)
 	return virtscsi_tmf(vscsi, cmd);
 }
 
+static enum blk_eh_timer_return virtscsi_timed_out(struct scsi_cmnd *sc)
+{
+	struct virtio_scsi *vscsi = shost_priv(sc->device->host);
+	struct virtio_scsi_cmd *cmd;
+
+	scmd_printk(KERN_INFO, sc, "timeout\n");
+	cmd = mempool_alloc(virtscsi_cmd_pool, GFP_NOIO);
+	if (!cmd)
+		return FAILED;
+
+	memset(cmd, 0, sizeof(*cmd));
+	cmd->sc = sc;
+	cmd->req.tmf = (struct virtio_scsi_ctrl_tmf_req){
+		.type = VIRTIO_SCSI_T_TMF,
+		.subtype = VIRTIO_SCSI_T_TMF_QUERY_TASK,
+		.lun[0] = 1,
+		.lun[1] = sc->device->id,
+		.lun[2] = (sc->device->lun >> 8) | 0x40,
+		.lun[3] = sc->device->lun & 0xff,
+		.tag = (unsigned long)sc,
+	};
+	if (virtscsi_tmf(vscsi, cmd) == FAILED)
+		return BLK_EH_NOT_HANDLED;
+
+	return BLK_EH_RESET_TIMER;
+}
+
 static struct scsi_host_template virtscsi_host_template = {
 	.module = THIS_MODULE,
 	.name = "Virtio SCSI HBA",
@@ -551,6 +578,7 @@  static struct scsi_host_template virtscsi_host_template = {
 	.this_id = -1,
 	.eh_abort_handler = virtscsi_abort,
 	.eh_device_reset_handler = virtscsi_device_reset,
+	.eh_timed_out = virtscsi_timed_out,
 
 	.can_queue = 1024,
 	.dma_boundary = UINT_MAX,
-- 
1.8.5.6