[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b1a04eb5b007455f9b57467b9c87ddf5@SN2PR03MB061.namprd03.prod.outlook.com>
Date: Thu, 3 Oct 2013 14:13:13 +0000
From: KY Srinivasan <kys@...rosoft.com>
To: "Nicholas A. Bellinger" <nab@...ux-iscsi.org>
CC: Geert Uytterhoeven <geert@...ux-m68k.org>,
Mike Christie <michaelc@...wisc.edu>,
Jack Wang <xjtuwjp@...il.com>,
Greg KH <gregkh@...uxfoundation.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
"ohering@...e.com" <ohering@...e.com>,
"jbottomley@...allels.com" <jbottomley@...allels.com>,
"hch@...radead.org" <hch@...radead.org>,
"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>
Subject: RE: Drivers: scsi: FLUSH timeout
> -----Original Message-----
> From: Nicholas A. Bellinger [mailto:nab@...ux-iscsi.org]
> Sent: Thursday, October 03, 2013 5:09 AM
> To: KY Srinivasan
> Cc: Geert Uytterhoeven; Mike Christie; Jack Wang; Greg KH; linux-
> kernel@...r.kernel.org; devel@...uxdriverproject.org; ohering@...e.com;
> jbottomley@...allels.com; hch@...radead.org; linux-scsi@...r.kernel.org
> Subject: Re: Drivers: scsi: FLUSH timeout
>
> On Wed, 2013-10-02 at 18:29 +0000, KY Srinivasan wrote:
> >
> > > -----Original Message-----
> > > From: geert.uytterhoeven@...il.com
> [mailto:geert.uytterhoeven@...il.com]
> > > On Behalf Of Geert Uytterhoeven
> > > Sent: Wednesday, September 25, 2013 1:40 AM
> > > To: KY Srinivasan
> > > Cc: Mike Christie; Jack Wang; Greg KH; linux-kernel@...r.kernel.org;
> > > devel@...uxdriverproject.org; ohering@...e.com;
> jbottomley@...allels.com;
> > > hch@...radead.org; linux-scsi@...r.kernel.org
> > > Subject: Re: Drivers: scsi: FLUSH timeout
> > >
> > > On Tue, Sep 24, 2013 at 11:53 PM, KY Srinivasan <kys@...rosoft.com> wrote:
> > > > I am not sure how that magic number was arrived at (the 60HZ number). We
> > > want this to be little higher -
> > >
> > > "60 * HZ" means "60 seconds".
> > >
> > > > would there be any issues raising this to say 180 seconds. This is the value
> we
> > > currently have for I/O
> > > > timeout.
> > >
> > > So you want to replace it by "180 * HZ", which is ... another magic number?
> >
> > Ideally, I want this to be adjustable like the way we can change the I/O timeout.
> > Since that has been attempted earlier and rejected (not clear what the reasons
> were),
> > I was suggesting that we pick a larger number. James, let me know how I should
> proceed here.
> >
>
> I think the objection was to making a module parameter for doing this
> globally for all struct scsi_disk, and not the idea of making it
> adjustable on an individual basis per-say..
>
> What about adding a /sys/class/scsi_disk/$HCTL/flush_timeout..?
>
> Here's a quick (untested) patch to that effect.
Thanks Nicholas. This is precisely what I was hoping we could agree on.
James, if this is acceptable, we will go ahead and test this patch.
Regards,
K. Y
>
> --nab
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index e62d17d..f7a8c5b 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -460,6 +460,38 @@ max_write_same_blocks_store(struct device *dev,
> struct device_attribute *attr,
> }
> static DEVICE_ATTR_RW(max_write_same_blocks);
>
> +static ssize_t
> +flush_timeout_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct scsi_disk *sdkp = to_scsi_disk(dev);
> +
> + return snprintf(buf, 20, "%u\n", sdkp->flush_timeout);
> +}
> +
> +static ssize_t
> +flush_timeout_store(struct device *dev,
> + struct device_attribute *attr, const char *buf,
> + size_t count)
> +{
> + struct scsi_disk *sdkp = to_scsi_disk(dev);
> + unsigned int flush_timeout;
> + int err;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EACCES;
> +
> + err = kstrtouint(buf, 10, &flush_timeout);
> +
> + if (flush_timeout > SD_FLUSH_TIMEOUT_MAX)
> + return -EINVAL;
> +
> + sdkp->flush_timeout = flush_timeout;
> +
> + return err ? err : count;
> +}
> +static DEVICE_ATTR_RW(flush_timeout);
> +
> static struct attribute *sd_disk_attrs[] = {
> &dev_attr_cache_type.attr,
> &dev_attr_FUA.attr,
> @@ -472,6 +504,7 @@ static struct attribute *sd_disk_attrs[] = {
> &dev_attr_provisioning_mode.attr,
> &dev_attr_max_write_same_blocks.attr,
> &dev_attr_max_medium_access_timeouts.attr,
> + &dev_attr_flush_timeout.attr,
> NULL,
> };
> ATTRIBUTE_GROUPS(sd_disk);
> @@ -829,7 +862,9 @@ static int sd_setup_write_same_cmnd(struct scsi_device
> *sdp, struct request *rq)
>
> static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct request *rq)
> {
> - rq->timeout = SD_FLUSH_TIMEOUT;
> + struct scsi_disk *sdkp = scsi_disk(rq->rq_disk);
> +
> + rq->timeout = sdkp->flush_timeout;
> rq->retries = SD_MAX_RETRIES;
> rq->cmd[0] = SYNCHRONIZE_CACHE;
> rq->cmd_len = 10;
> @@ -2841,6 +2876,7 @@ static void sd_probe_async(void *data, async_cookie_t
> cookie)
> sdkp->ATO = 0;
> sdkp->first_scan = 1;
> sdkp->max_medium_access_timeouts = SD_MAX_MEDIUM_TIMEOUTS;
> + sdkp->flush_timeout = SD_FLUSH_TIMEOUT;
>
> sd_revalidate_disk(gd);
>
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index 7a049de..ac7cd0f 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -14,6 +14,7 @@
> #define SD_TIMEOUT (30 * HZ)
> #define SD_MOD_TIMEOUT (75 * HZ)
> #define SD_FLUSH_TIMEOUT (60 * HZ)
> +#define SD_FLUSH_TIMEOUT_MAX (180 * HZ)
> #define SD_WRITE_SAME_TIMEOUT (120 * HZ)
>
> /*
> @@ -68,6 +69,7 @@ struct scsi_disk {
> unsigned int physical_block_size;
> unsigned int max_medium_access_timeouts;
> unsigned int medium_access_timed_out;
> + unsigned int flush_timeout;
> u8 media_present;
> u8 write_prot;
> u8 protection_type;/* Data Integrity Field */
Powered by blists - more mailing lists