From: KY Srinivasan <kys@microsoft.com>
To: James Bottomley <jbottomley@parallels.com>
CC: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
        "ohering@suse.com" <ohering@suse.com>,
        "hch@infradead.org" <hch@infradead.org>,
        "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>
Subject: RE: [PATCH 1/5] Drivers: scsi: storvsc: Make the scsi timeout a
 module parameter
Thread-Topic: [PATCH 1/5] Drivers: scsi: storvsc: Make the scsi timeout a
 module parameter
Thread-Index: AQHOYKxAJGrjXDxyYEGEh8dfafipzpkkm6WAgAAAcdA=
Date: Mon, 3 Jun 2013 23:25:44 +0000
Message-ID: <154d86b83ab94931a097c32e1c8e4987@SN2PR03MB061.namprd03.prod.outlook.com>
References: <1370301519-27196-1-git-send-email-kys@microsoft.com>
	 <1370301688-27286-1-git-send-email-kys@microsoft.com>
 <1370300581.2013.14.camel@dabdike.int.hansenpartnership.com>
In-Reply-To: <1370300581.2013.14.camel@dabdike.int.hansenpartnership.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2909
Lines: 76


> -----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
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/