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

[Long Li]

> -----Original Message-----
> From: Johannes Thumshirn [mailto:jthumshirn@xxxxxxx]
> Sent: Wednesday, June 14, 2017 1:25 AM
> To: Long Li <longli@xxxxxxxxxxxxx>; James E.J. Bottomley
> <jejb@xxxxxxxxxxxxxxxxxx>; Martin K. Petersen <martin.petersen@xxxxxxxxxx>;
> linux-scsi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; KY Srinivasan
> <kys@xxxxxxxxxxxxx>; Bart Van Assche <Bart.VanAssche@xxxxxxxxxxx>;
> Christoph Hellwig <hch@xxxxxxxxxxxxx>; Stephen Hemminger
> <sthemmin@xxxxxxxxxxxxx>
> Cc: Long Li <longli@xxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxx>
> >
> > Signed-off-by: Long Li <longli@xxxxxxxxxxxxx>
> > ---
> >  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@xxxxxxx                                +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