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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <MWHPR03MB3136B62CABFE986250B2547FCEC30@MWHPR03MB3136.namprd03.prod.outlook.com>
Date:   Wed, 14 Jun 2017 19:19:58 +0000
From:   Long Li <longli@...rosoft.com>
To:     Johannes Thumshirn <jthumshirn@...e.de>,
        "James E.J. Bottomley" <jejb@...ux.vnet.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        KY Srinivasan <kys@...rosoft.com>,
        Bart Van Assche <Bart.VanAssche@...disk.com>,
        "Christoph Hellwig" <hch@...radead.org>,
        Stephen Hemminger <sthemmin@...rosoft.com>
Subject: RE: [PATCH v2] storvsc: use default I/O timeout handler for FC
 devices



> -----Original Message-----
> From: Johannes Thumshirn [mailto:jthumshirn@...e.de]
> Sent: Wednesday, June 14, 2017 1:25 AM
> To: Long Li <longli@...rosoft.com>; James E.J. Bottomley
> <jejb@...ux.vnet.ibm.com>; Martin K. Petersen <martin.petersen@...cle.com>;
> linux-scsi@...r.kernel.org; linux-kernel@...r.kernel.org; KY Srinivasan
> <kys@...rosoft.com>; Bart Van Assche <Bart.VanAssche@...disk.com>;
> Christoph Hellwig <hch@...radead.org>; Stephen Hemminger
> <sthemmin@...rosoft.com>
> Cc: Long Li <longli@...rosoft.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@...rosoft.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@...disk.com>
> >
> > Signed-off-by: Long Li <longli@...rosoft.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@...e.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