RE: [PATCH 1/5] Drivers: scsi: storvsc: Make the scsi timeout a module parameter

KY Srinivasan
2013-06-03T23:26:10Z

> -----Original Message-----
> From: James Bottomley [mailto:jbottomley@parallels.com]
> Sent: Monday, June 03, 2013 7:03 PM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com; hch@infradead.org; linux-
> scsi@vger.kernel.org
> Subject: Re: [PATCH 1/5] Drivers: scsi: storvsc: Make the scsi timeout a module
> parameter
>
> On Mon, 2013-06-03 at 16:21 -0700, K. Y. Srinivasan wrote:
> > The standard scsi timeout is not appropriate in some of the environments
> where
> > Hyper-V is deployed. Set this timeout appropriately for all devices managed
> > by this driver. Further make this a module parameter.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> > drivers/scsi/storvsc_drv.c | 9 +++++++++
> > 1 files changed, 9 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > index 16a3a0c..8d29a95 100644
> > --- a/drivers/scsi/storvsc_drv.c
> > +++ b/drivers/scsi/storvsc_drv.c
> > @@ -221,6 +221,13 @@ static int storvsc_ringbuffer_size = (20 * PAGE_SIZE);
> > module_param(storvsc_ringbuffer_size, int, S_IRUGO);
> > MODULE_PARM_DESC(storvsc_ringbuffer_size, "Ring buffer size (bytes)");
> >
> > +/*
> > + * Timeout in seconds for all devices managed by this driver.
> > + */
> > +static int storvsc_timeout = 180;
> > +module_param(storvsc_timeout, uint, (S_IRUGO | S_IWUSR));
> > +MODULE_PARM_DESC(storvsc_timeout, "Device timeout (seconds)");
> > +
> > #define STORVSC_MAX_IO_REQUESTS 128
> >
> > /*
> > @@ -1204,6 +1211,8 @@ static int storvsc_device_configure(struct scsi_device
> *sdevice)
> >
> > blk_queue_bounce_limit(sdevice->request_queue, BLK_BOUNCE_ANY);
> >
> > + blk_queue_rq_timeout(sdevice->request_queue, (storvsc_timeout *
> HZ));
>
> Why does this need to be a module parameter? It's already a sysfs one
> in the scsi_device class? Three minutes is also a bit large. The
> default is 30s with huge cache arrays recommending upping this to
> 60s ... you're three times this.

James,
This number was arrived at based on some testing that was done on the cloud. On our cloud, we have a 120 second
timeouts that trigger broader VM level recovery and in cases where there is storage access issues
(which is when we would hit this timeout), it will be better to defer to the fabric level recovery than attempt
Scsi level recovery/retry. The default value chosen for devices managed by storvsc should be just fine,
I made it a module parameter to have more flexibility.

Regards,

K. Y
>
> James
>
>