[PATCH v2] storvsc: use default I/O timeout handler for FC devices

[PATCH v2] storvsc: use default I/O timeout handler for FC devices

[Long Li]

From: Long Li <longli@microsoft.com>

FC disks issue I/O directly to the host storage port driver, this is
diffirent to VHD disks where I/O is virtualized and timeout is handled
by the host VSP (Virtualization Service Provider).

FC disks are usually setup in a multipath system, and they don't want to
reset timer on I/O timeout. Timeout is detected by multipath as a good 
time to failover and recover.

Patch v2 includes suggestions from Bart Van Assche
<Bart.VanAssche@sandisk.com>

Signed-off-by: Long Li <longli@microsoft.com>
---
 drivers/scsi/storvsc_drv.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 8d955db..3cc8d67 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1495,6 +1495,10 @@ static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd)
  */
 static enum blk_eh_timer_return storvsc_eh_timed_out(struct scsi_cmnd *scmnd)
 {
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	if (scmnd->device->host->transportt == fc_transport_template)
+		return fc_eh_timed_out(scmnd);
+#endif
 	return BLK_EH_RESET_TIMER;
 }
 
-- 
2.7.4

Re: [PATCH v2] storvsc: use default I/O timeout handler for FC devices

[Johannes Thumshirn]

On 06/13/2017 11:34 PM, Long Li wrote:
> From: Long Li <longli@microsoft.com>
> 
> FC disks issue I/O directly to the host storage port driver, this is
> diffirent to VHD disks where I/O is virtualized and timeout is handled
> by the host VSP (Virtualization Service Provider).
> 
> FC disks are usually setup in a multipath system, and they don't want to
> reset timer on I/O timeout. Timeout is detected by multipath as a good 
> time to failover and recover.
> 
> Patch v2 includes suggestions from Bart Van Assche
> <Bart.VanAssche@sandisk.com>
> 
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
>  drivers/scsi/storvsc_drv.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 8d955db..3cc8d67 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1495,6 +1495,10 @@ static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd)
>   */
>  static enum blk_eh_timer_return storvsc_eh_timed_out(struct scsi_cmnd *scmnd)
>  {
> +#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
> +	if (scmnd->device->host->transportt == fc_transport_template)
> +		return fc_eh_timed_out(scmnd);
> +#endif

Can you please change the #if IS_ENABLED() to
if(IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
	&& scmnd->device->host->transportt == fc_transport_template)

That way we have better compiler coverage.

Thanks,
	Johannes

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

RE: [PATCH v2] storvsc: use default I/O timeout handler for FC devices

[Long Li]

> -----Original Message-----
> From: Johannes Thumshirn [mailto:jthumshirn@suse.de]
> Sent: Wednesday, June 14, 2017 1:25 AM
> To: Long Li <longli@microsoft.com>; James E.J. Bottomley
> <jejb@linux.vnet.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>;
> linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; KY Srinivasan
> <kys@microsoft.com>; Bart Van Assche <Bart.VanAssche@sandisk.com>;
> Christoph Hellwig <hch@infradead.org>; Stephen Hemminger
> <sthemmin@microsoft.com>
> Cc: Long Li <longli@microsoft.com>
> Subject: Re: [PATCH v2] storvsc: use default I/O timeout handler for FC devices
> 
> On 06/13/2017 11:34 PM, Long Li wrote:
> > From: Long Li <longli@microsoft.com>
> >
> > FC disks issue I/O directly to the host storage port driver, this is
> > diffirent to VHD disks where I/O is virtualized and timeout is handled
> > by the host VSP (Virtualization Service Provider).
> >
> > FC disks are usually setup in a multipath system, and they don't want
> > to reset timer on I/O timeout. Timeout is detected by multipath as a
> > good time to failover and recover.
> >
> > Patch v2 includes suggestions from Bart Van Assche
> > <Bart.VanAssche@sandisk.com>
> >
> > Signed-off-by: Long Li <longli@microsoft.com>
> > ---
> >  drivers/scsi/storvsc_drv.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > index 8d955db..3cc8d67 100644
> > --- a/drivers/scsi/storvsc_drv.c
> > +++ b/drivers/scsi/storvsc_drv.c
> > @@ -1495,6 +1495,10 @@ static int storvsc_host_reset_handler(struct
> scsi_cmnd *scmnd)
> >   */
> >  static enum blk_eh_timer_return storvsc_eh_timed_out(struct scsi_cmnd
> > *scmnd)  {
> > +#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
> > +	if (scmnd->device->host->transportt == fc_transport_template)
> > +		return fc_eh_timed_out(scmnd);
> > +#endif
> 
> Can you please change the #if IS_ENABLED() to
> if(IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
> 	&& scmnd->device->host->transportt == fc_transport_template)
> 
> That way we have better compiler coverage.

Thank you for pointing this out.

The coding style is kept consistent with the rest of the FC related code in storvsc. E.g. the definition of fc_transport_template (and many other places):

#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
static struct scsi_transport_template *fc_transport_template;
#endif

I suggest we make another patch to fix this in all the places. This patch is mainly for fixing FC timeout.

Long

> 
> Thanks,
> 	Johannes
> 
> --
> Johannes Thumshirn                                          Storage
> jthumshirn@suse.de                                +49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg)
> Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850