Re: [PATCH] scsi: sd: Implement blacklist option for WRITE SAME w/ UNMAP

2017-10-17 Thread Laurence Oberman 
On Tue, 2017-10-17 at 10:26 -0400, Laurence Oberman wrote:
> On Fri, 2017-09-29 at 10:01 -0400, Laurence Oberman wrote:
> > On Fri, 2017-09-29 at 09:21 -0400, Martin K. Petersen wrote:
> > > Laurence,
> > > 
> > > > I am testing this but its not being picked up so I want to know
> > > > if
> > > > I
> > > > have the kernel command line wrong here.
> > > > 
> > > > scsi_dev_flags=LIO-ORG:thin2:0x8000
> > > > 
> > > > What am I doing wrong to pass the BLIST flags.
> > > 
> > > This worked for me:
> > > 
> > > [root@kvm ~]# echo "Linux:scsi_debug:0x8000" >
> > > /proc/scsi/device_info
> > > [root@kvm ~]# grep Linux /proc/scsi/device_info 
> > > 'Linux   ' 'scsi_debug  ' 0x8000
> > > [root@kvm ~]# modprobe scsi_debug unmap_max_blocks=10
> > > unmap_max_desc=1 write_same_length=20 lbpws=1
> > > [root@kvm ~]# lsblk -D
> > > NAME DISC-ALN DISC-GRAN DISC-MAX DISC-ZERO
> > > sda 0  512B   5K 0
> > > 
> > > (With the caveat that I tweaked scsi_debug to report the UNMAP
> > > parameters despite lbpu being 0).
> > > 
> > 
> > OK, Thanks, that is working now and I pick up the correct size now.
> > Its going to be very useful for these corner case array
> > inconsistencies.
> > 
> > Tested-by: Laurence Oberman 
> > 
> > Sep 29 09:56:11 localhost kernel: scsi 1:0:0:50: Direct-
> > Access LIO-
> > ORG  thin24.0  PQ: 0 ANSI: 5
> > Sep 29 09:56:11 localhost kernel: scsi 1:0:0:50: alua: supports
> > implicit and explicit TPGS
> > Sep 29 09:56:11 localhost kernel: scsi 1:0:0:50: alua: device
> > naa.6001405f7aa27ca453f4381a00f22ea6 port group 0 rel port 2
> > Sep 29 09:56:11 localhost kernel: sd 1:0:0:50: Attached scsi
> > generic
> > sg64 type 0
> > Sep 29 09:56:11 localhost kernel: RHDEBUG: unmap_limit_for_ws set
> > by
> > kernel flag for case SD_LBP_WS16
> > Sep 29 09:56:11 localhost kernel: sd 1:0:0:50: [sdbl] 8192 512-
> > byte 
> > logical blocks: (41.9 GB/39.1 GiB)
> > Sep 29 09:56:11 localhost kernel: sd 1:0:0:50: [sdbl] Write Protect
> > is
> > off
> > Sep 29 09:56:11 localhost kernel: sd 1:0:0:50: [sdbl] Write cache:
> > enabled, read cache: enabled, supports DPO and FUA
> > Sep 29 09:56:11 localhost kernel: sd 1:0:0:50: alua: transition
> > timeout
> > set to 60 seconds
> 
> 
> Hi Martin
> 
> We have the code accepted for the patch above and its working fine
> with
> echo after boot as you already know.
> 
> However I am still fighting with trying to pass this on the kernel
> command line. We need to be able to do this as a dynamic method for
> adding devices to the list so that the list is populated prior to the
> device scan.
> 
> Using scsi_dev_flags=LIO-ORG:thin2:0x8000 on the kernel line is
> ignored and not filled in.
> Its apparent to me that we have no longer have code to actually copy
> the string from the kernel line after boot.
> 
> I ran some tests and added a couple of printk;s to see if we have any
> capture and its NULL.
> 
> So when did this stop working, is what I am now chasing
> 
> [1.524595] RHDEBUG:In scsi_init_devinfo scsi_dev_flags=
> [1.524705] RHDEBUG: In scsi_init_devinfo error=0
> 
> We have this in  drivers/scsi/scsi_devinfo.c
> 
> module_param_string(dev_flags, scsi_dev_flags,
> sizeof(scsi_dev_flags),
> 0);
> MODULE_PARM_DESC(dev_flags,
> 
>  "Given scsi_dev_flags=vendor:model:flags[,v:m:f] add
> black/white"
>  " list entries for vendor and model with an integer value of
> flags"
>  " to the scsi device info list");
> 
> and we have:
> 
> /**
>  * scsi_init_devinfo - set up the dynamic device list.
>  *
>  * Description:
>  *  Add command line entries from scsi_dev_flags, then add
>  *  scsi_static_device_list entries to the scsi device info list.
>  */
> int __init scsi_init_devinfo(void)
> {
> #ifdef CONFIG_SCSI_PROC_FS
> struct proc_dir_entry *p;
> #endif
> int error, i;
> 
> printk("RHDEBUG:In scsi_init_devinfo
> scsi_dev_flags=%s\n",scsi_dev_flags);
> 
> error = scsi_dev_info_add_list(SCSI_DEVINFO_GLOBAL, NULL);
> printk("RHDEBUG: In scsi_init_devinfo error=%d\n",error);
> if (error) {
> printk("RHDEBUG: In scsi_init_devinfo, calling
> scsi_dev_info_add_list returning with error=%d\n",error);
> return error;
> }
> 
> error = scsi_dev_info_list_add_str(scsi_dev_flags);
> if (error) {
> printk("RHDEBUG: In scsi_init_devinfo, calling
> scsi_info_list_add returning with error=%d\n",error);
> goto out;
> }
> 
> for (i = 0; scsi_static_device_list[i].vendor; i++) {
> error = scsi_dev_info_list_add(1 /* compatibile */,
> scsi_static_device_list[i].vendor,
> scsi_static_device_list[i].model,
> NULL,
> scsi_static_device_list[i].flags);
> if (error)
>    

Re: [PATCH] scsi: sd: Implement blacklist option for WRITE SAME w/ UNMAP

2017-10-17 Thread Laurence Oberman 
On Fri, 2017-09-29 at 10:01 -0400, Laurence Oberman wrote:
> On Fri, 2017-09-29 at 09:21 -0400, Martin K. Petersen wrote:
> > Laurence,
> > 
> > > I am testing this but its not being picked up so I want to know
> > > if
> > > I
> > > have the kernel command line wrong here.
> > > 
> > > scsi_dev_flags=LIO-ORG:thin2:0x8000
> > > 
> > > What am I doing wrong to pass the BLIST flags.
> > 
> > This worked for me:
> > 
> > [root@kvm ~]# echo "Linux:scsi_debug:0x8000" >
> > /proc/scsi/device_info
> > [root@kvm ~]# grep Linux /proc/scsi/device_info 
> > 'Linux   ' 'scsi_debug  ' 0x8000
> > [root@kvm ~]# modprobe scsi_debug unmap_max_blocks=10
> > unmap_max_desc=1 write_same_length=20 lbpws=1
> > [root@kvm ~]# lsblk -D
> > NAME DISC-ALN DISC-GRAN DISC-MAX DISC-ZERO
> > sda 0  512B   5K 0
> > 
> > (With the caveat that I tweaked scsi_debug to report the UNMAP
> > parameters despite lbpu being 0).
> > 
> 
> OK, Thanks, that is working now and I pick up the correct size now.
> Its going to be very useful for these corner case array
> inconsistencies.
> 
> Tested-by: Laurence Oberman 
> 
> Sep 29 09:56:11 localhost kernel: scsi 1:0:0:50: Direct-
> Access LIO-
> ORG  thin24.0  PQ: 0 ANSI: 5
> Sep 29 09:56:11 localhost kernel: scsi 1:0:0:50: alua: supports
> implicit and explicit TPGS
> Sep 29 09:56:11 localhost kernel: scsi 1:0:0:50: alua: device
> naa.6001405f7aa27ca453f4381a00f22ea6 port group 0 rel port 2
> Sep 29 09:56:11 localhost kernel: sd 1:0:0:50: Attached scsi generic
> sg64 type 0
> Sep 29 09:56:11 localhost kernel: RHDEBUG: unmap_limit_for_ws set by
> kernel flag for case SD_LBP_WS16
> Sep 29 09:56:11 localhost kernel: sd 1:0:0:50: [sdbl] 8192 512-
> byte 
> logical blocks: (41.9 GB/39.1 GiB)
> Sep 29 09:56:11 localhost kernel: sd 1:0:0:50: [sdbl] Write Protect
> is
> off
> Sep 29 09:56:11 localhost kernel: sd 1:0:0:50: [sdbl] Write cache:
> enabled, read cache: enabled, supports DPO and FUA
> Sep 29 09:56:11 localhost kernel: sd 1:0:0:50: alua: transition
> timeout
> set to 60 seconds


Hi Martin

We have the code accepted for the patch above and its working fine with
echo after boot as you already know.

However I am still fighting with trying to pass this on the kernel
command line. We need to be able to do this as a dynamic method for
adding devices to the list so that the list is populated prior to the
device scan.

Using scsi_dev_flags=LIO-ORG:thin2:0x8000 on the kernel line is
ignored and not filled in.
Its apparent to me that we have no longer have code to actually copy
the string from the kernel line after boot.

I ran some tests and added a couple of printk;s to see if we have any
capture and its NULL.

So when did this stop working, is what I am now chasing

[1.524595] RHDEBUG:In scsi_init_devinfo scsi_dev_flags=
[1.524705] RHDEBUG: In scsi_init_devinfo error=0

We have this in  drivers/scsi/scsi_devinfo.c

module_param_string(dev_flags, scsi_dev_flags, sizeof(scsi_dev_flags),
0);
MODULE_PARM_DESC(dev_flags,

 "Given scsi_dev_flags=vendor:model:flags[,v:m:f] add
black/white"
 " list entries for vendor and model with an integer value of
flags"
 " to the scsi device info list");

and we have:

/**
 * scsi_init_devinfo - set up the dynamic device list.
 *
 * Description:
 *  Add command line entries from scsi_dev_flags, then add
 *  scsi_static_device_list entries to the scsi device info list.
 */
int __init scsi_init_devinfo(void)
{
#ifdef CONFIG_SCSI_PROC_FS
struct proc_dir_entry *p;
#endif
int error, i;

printk("RHDEBUG:In scsi_init_devinfo
scsi_dev_flags=%s\n",scsi_dev_flags);

error = scsi_dev_info_add_list(SCSI_DEVINFO_GLOBAL, NULL);
printk("RHDEBUG: In scsi_init_devinfo error=%d\n",error);
if (error) {
printk("RHDEBUG: In scsi_init_devinfo, calling
scsi_dev_info_add_list returning with error=%d\n",error);
return error;
}

error = scsi_dev_info_list_add_str(scsi_dev_flags);
if (error) {
printk("RHDEBUG: In scsi_init_devinfo, calling
scsi_info_list_add returning with error=%d\n",error);
goto out;
}

for (i = 0; scsi_static_device_list[i].vendor; i++) {
error = scsi_dev_info_list_add(1 /* compatibile */,
scsi_static_device_list[i].vendor,
scsi_static_device_list[i].model,
NULL,
scsi_static_device_list[i].flags);
if (error)
goto out;
}

#ifdef CONFIG_SCSI_PROC_FS
p = proc_create("scsi/device_info", 0, NULL,
_devinfo_proc_fops);
if (!p) {
error = -ENOMEM;
goto out;
}
#endif /* CONFIG_SCSI_PROC_FS */

 out:
if (error)
scsi_exit_devinfo();
return

Re: [PATCH] scsi: sd: Implement blacklist option for WRITE SAME w/ UNMAP

2017-09-29 Thread Laurence Oberman 
On Fri, 2017-09-29 at 09:21 -0400, Martin K. Petersen wrote:
> Laurence,
> 
> > I am testing this but its not being picked up so I want to know if
> > I
> > have the kernel command line wrong here.
> > 
> > scsi_dev_flags=LIO-ORG:thin2:0x8000
> > 
> > What am I doing wrong to pass the BLIST flags.
> 
> This worked for me:
> 
> [root@kvm ~]# echo "Linux:scsi_debug:0x8000" >
> /proc/scsi/device_info
> [root@kvm ~]# grep Linux /proc/scsi/device_info 
> 'Linux   ' 'scsi_debug  ' 0x8000
> [root@kvm ~]# modprobe scsi_debug unmap_max_blocks=10
> unmap_max_desc=1 write_same_length=20 lbpws=1
> [root@kvm ~]# lsblk -D
> NAME DISC-ALN DISC-GRAN DISC-MAX DISC-ZERO
> sda 0  512B   5K 0
> 
> (With the caveat that I tweaked scsi_debug to report the UNMAP
> parameters despite lbpu being 0).
> 

OK, Thanks, that is working now and I pick up the correct size now.
Its going to be very useful for these corner case array
inconsistencies.

Tested-by: Laurence Oberman 

Sep 29 09:56:11 localhost kernel: scsi 1:0:0:50: Direct-Access LIO-
ORG  thin24.0  PQ: 0 ANSI: 5
Sep 29 09:56:11 localhost kernel: scsi 1:0:0:50: alua: supports
implicit and explicit TPGS
Sep 29 09:56:11 localhost kernel: scsi 1:0:0:50: alua: device
naa.6001405f7aa27ca453f4381a00f22ea6 port group 0 rel port 2
Sep 29 09:56:11 localhost kernel: sd 1:0:0:50: Attached scsi generic
sg64 type 0
Sep 29 09:56:11 localhost kernel: RHDEBUG: unmap_limit_for_ws set by
kernel flag for case SD_LBP_WS16
Sep 29 09:56:11 localhost kernel: sd 1:0:0:50: [sdbl] 8192 512-byte 
logical blocks: (41.9 GB/39.1 GiB)
Sep 29 09:56:11 localhost kernel: sd 1:0:0:50: [sdbl] Write Protect is
off
Sep 29 09:56:11 localhost kernel: sd 1:0:0:50: [sdbl] Write cache:
enabled, read cache: enabled, supports DPO and FUA
Sep 29 09:56:11 localhost kernel: sd 1:0:0:50: alua: transition timeout
set to 60 seconds

Re: [PATCH] scsi: sd: Implement blacklist option for WRITE SAME w/ UNMAP

2017-09-29 Thread Martin K. Petersen 

Laurence,

> I am testing this but its not being picked up so I want to know if I
> have the kernel command line wrong here.
>
> scsi_dev_flags=LIO-ORG:thin2:0x8000
>
> What am I doing wrong to pass the BLIST flags.

This worked for me:

[root@kvm ~]# echo "Linux:scsi_debug:0x8000" > /proc/scsi/device_info
[root@kvm ~]# grep Linux /proc/scsi/device_info 
'Linux   ' 'scsi_debug  ' 0x8000
[root@kvm ~]# modprobe scsi_debug unmap_max_blocks=10 unmap_max_desc=1 
write_same_length=20 lbpws=1
[root@kvm ~]# lsblk -D
NAME DISC-ALN DISC-GRAN DISC-MAX DISC-ZERO
sda 0  512B   5K 0

(With the caveat that I tweaked scsi_debug to report the UNMAP
parameters despite lbpu being 0).

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: sd: Implement blacklist option for WRITE SAME w/ UNMAP

2017-09-29 Thread Laurence Oberman 
On Wed, 2017-09-27 at 21:35 -0400, Martin K. Petersen wrote:
> SBC-4 states:
> 
>   "A MAXIMUM UNMAP LBA COUNT field set to a non-zero value indicates
> the
>    maximum number of LBAs that may be unmapped by an UNMAP command"
> 
>   "A MAXIMUM WRITE SAME LENGTH field set to a non-zero value
> indicates
>    the maximum number of contiguous logical blocks that the device
> server
>    allows to be unmapped or written in a single WRITE SAME command."
> 
> Despite the spec being clear on the topic, some devices incorrectly
> expect WRITE SAME commands with the UNMAP bit set to be limited to
> the
> value reported in MAXIMUM UNMAP LBA COUNT in the Block Limits VPD.
> 
> Implement a blacklist option that can be used to accommodate devices
> with this behavior.
> 
> Reported-by: Bill Kuzeja 
> Reported-by: Ewan D. Milne 
> Signed-off-by: Martin K. Petersen 
> ---
>  drivers/scsi/scsi_scan.c|  3 +++
>  drivers/scsi/sd.c   | 16 
>  include/scsi/scsi_device.h  |  1 +
>  include/scsi/scsi_devinfo.h |  1 +
>  4 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index e7818afeda2b..15590a063ad9 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -956,6 +956,9 @@ static int scsi_add_lun(struct scsi_device *sdev,
> unsigned char *inq_result,
>   if (*bflags & BLIST_NO_DIF)
>   sdev->no_dif = 1;
>  
> + if (*bflags & BLIST_UNMAP_LIMIT_WS)
> + sdev->unmap_limit_for_ws = 1;
> +
>   sdev->eh_timeout = SCSI_DEFAULT_EH_TIMEOUT;
>  
>   if (*bflags & BLIST_TRY_VPD_PAGES)
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index b18ba3235900..347be7580181 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -715,13 +715,21 @@ static void sd_config_discard(struct scsi_disk
> *sdkp, unsigned int mode)
>   break;
>  
>   case SD_LBP_WS16:
> - max_blocks = min_not_zero(sdkp->max_ws_blocks,
> -   (u32)SD_MAX_WS16_BLOCKS);
> + if (sdkp->device->unmap_limit_for_ws)
> + max_blocks = sdkp->max_unmap_blocks;
> + else
> + max_blocks = sdkp->max_ws_blocks;
> +
> + max_blocks = min_not_zero(max_blocks,
> (u32)SD_MAX_WS16_BLOCKS);
>   break;
>  
>   case SD_LBP_WS10:
> - max_blocks = min_not_zero(sdkp->max_ws_blocks,
> -   (u32)SD_MAX_WS10_BLOCKS);
> + if (sdkp->device->unmap_limit_for_ws)
> + max_blocks = sdkp->max_unmap_blocks;
> + else
> + max_blocks = sdkp->max_ws_blocks;
> +
> + max_blocks = min_not_zero(max_blocks,
> (u32)SD_MAX_WS10_BLOCKS);
>   break;
>  
>   case SD_LBP_ZERO:
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 82e93ee94708..67c5a9f223f7 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -192,6 +192,7 @@ struct scsi_device {
>   unsigned no_dif:1;  /* T10 PI (DIF) should be disabled
> */
>   unsigned broken_fua:1;  /* Don't set FUA bit
> */
>   unsigned lun_in_cdb:1;  /* Store LUN bits in
> CDB[1] */
> + unsigned unmap_limit_for_ws:1;  /* Use the UNMAP limit
> for WRITE SAME */
>  
>   atomic_t disk_events_disable_depth; /* disable depth for
> disk events */
>  
> diff --git a/include/scsi/scsi_devinfo.h
> b/include/scsi/scsi_devinfo.h
> index 9592570e092a..36b03013d629 100644
> --- a/include/scsi/scsi_devinfo.h
> +++ b/include/scsi/scsi_devinfo.h
> @@ -29,5 +29,6 @@
>  #define BLIST_TRY_VPD_PAGES  0x1000 /* Attempt to read VPD
> pages */
>  #define BLIST_NO_RSOC0x2000 /* don't try to
> issue RSOC */
>  #define BLIST_MAX_1024   0x4000 /* maximum 1024
> sector cdb length */
> +#define BLIST_UNMAP_LIMIT_WS 0x8000 /* Use UNMAP limit
> for WRITE SAME */
>  
>  #endif

Hello Martin
I am testing this but its not being picked up so I want to know if I
have the kernel command line wrong here.

scsi_dev_flags=LIO-ORG:thin2:0x8000

Device is here
[   16.853083] scsi 4:0:0:50: Direct-Access LIO-
ORG  thin24.0  PQ: 0 ANSI: 5

Have a couple of printk's in now to see if I see the flags and they
dont trigger

case SD_LBP_WS16:
if (sdkp->device->unmap_limit_for_ws) {
max_blocks = sdkp->max_unmap_blocks;
printk("RHDEBUG: unmap_limit_for_ws set by
kernel flag for case SD_LBP_WS16\n");
}
else
max_blocks = sdkp->max_ws_blocks;

max_blocks = min_not_zero(max_blocks,
(u32)SD_MAX_WS16_BLOCKS);
break;

case SD_LBP_WS10:
if (sdkp->device->unmap_limit_for_ws) {
   

Re: [PATCH] scsi: sd: Implement blacklist option for WRITE SAME w/ UNMAP

2017-09-28 Thread Ewan D. Milne 
On Wed, 2017-09-27 at 21:35 -0400, Martin K. Petersen wrote:
> SBC-4 states:
> 
>   "A MAXIMUM UNMAP LBA COUNT field set to a non-zero value indicates the
>maximum number of LBAs that may be unmapped by an UNMAP command"
> 
>   "A MAXIMUM WRITE SAME LENGTH field set to a non-zero value indicates
>the maximum number of contiguous logical blocks that the device server
>allows to be unmapped or written in a single WRITE SAME command."
> 
> Despite the spec being clear on the topic, some devices incorrectly
> expect WRITE SAME commands with the UNMAP bit set to be limited to the
> value reported in MAXIMUM UNMAP LBA COUNT in the Block Limits VPD.
> 
> Implement a blacklist option that can be used to accommodate devices
> with this behavior.
> 
> Reported-by: Bill Kuzeja 
> Reported-by: Ewan D. Milne 
> Signed-off-by: Martin K. Petersen 
> ---
>  drivers/scsi/scsi_scan.c|  3 +++
>  drivers/scsi/sd.c   | 16 
>  include/scsi/scsi_device.h  |  1 +
>  include/scsi/scsi_devinfo.h |  1 +
>  4 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index e7818afeda2b..15590a063ad9 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -956,6 +956,9 @@ static int scsi_add_lun(struct scsi_device *sdev, 
> unsigned char *inq_result,
>   if (*bflags & BLIST_NO_DIF)
>   sdev->no_dif = 1;
>  
> + if (*bflags & BLIST_UNMAP_LIMIT_WS)
> + sdev->unmap_limit_for_ws = 1;
> +
>   sdev->eh_timeout = SCSI_DEFAULT_EH_TIMEOUT;
>  
>   if (*bflags & BLIST_TRY_VPD_PAGES)
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index b18ba3235900..347be7580181 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -715,13 +715,21 @@ static void sd_config_discard(struct scsi_disk *sdkp, 
> unsigned int mode)
>   break;
>  
>   case SD_LBP_WS16:
> - max_blocks = min_not_zero(sdkp->max_ws_blocks,
> -   (u32)SD_MAX_WS16_BLOCKS);
> + if (sdkp->device->unmap_limit_for_ws)
> + max_blocks = sdkp->max_unmap_blocks;
> + else
> + max_blocks = sdkp->max_ws_blocks;
> +
> + max_blocks = min_not_zero(max_blocks, (u32)SD_MAX_WS16_BLOCKS);
>   break;
>  
>   case SD_LBP_WS10:
> - max_blocks = min_not_zero(sdkp->max_ws_blocks,
> -   (u32)SD_MAX_WS10_BLOCKS);
> + if (sdkp->device->unmap_limit_for_ws)
> + max_blocks = sdkp->max_unmap_blocks;
> + else
> + max_blocks = sdkp->max_ws_blocks;
> +
> + max_blocks = min_not_zero(max_blocks, (u32)SD_MAX_WS10_BLOCKS);
>   break;
>  
>   case SD_LBP_ZERO:
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 82e93ee94708..67c5a9f223f7 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -192,6 +192,7 @@ struct scsi_device {
>   unsigned no_dif:1;  /* T10 PI (DIF) should be disabled */
>   unsigned broken_fua:1;  /* Don't set FUA bit */
>   unsigned lun_in_cdb:1;  /* Store LUN bits in CDB[1] */
> + unsigned unmap_limit_for_ws:1;  /* Use the UNMAP limit for WRITE SAME */
>  
>   atomic_t disk_events_disable_depth; /* disable depth for disk events */
>  
> diff --git a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
> index 9592570e092a..36b03013d629 100644
> --- a/include/scsi/scsi_devinfo.h
> +++ b/include/scsi/scsi_devinfo.h
> @@ -29,5 +29,6 @@
>  #define BLIST_TRY_VPD_PAGES  0x1000 /* Attempt to read VPD pages */
>  #define BLIST_NO_RSOC0x2000 /* don't try to issue RSOC */
>  #define BLIST_MAX_1024   0x4000 /* maximum 1024 sector cdb 
> length */
> +#define BLIST_UNMAP_LIMIT_WS 0x8000 /* Use UNMAP limit for WRITE SAME */
>  
>  #endif

Thanks.

Reviewed-by: Ewan D. Milne

[PATCH] scsi: sd: Implement blacklist option for WRITE SAME w/ UNMAP

2017-09-27 Thread Martin K. Petersen 
SBC-4 states:

  "A MAXIMUM UNMAP LBA COUNT field set to a non-zero value indicates the
   maximum number of LBAs that may be unmapped by an UNMAP command"

  "A MAXIMUM WRITE SAME LENGTH field set to a non-zero value indicates
   the maximum number of contiguous logical blocks that the device server
   allows to be unmapped or written in a single WRITE SAME command."

Despite the spec being clear on the topic, some devices incorrectly
expect WRITE SAME commands with the UNMAP bit set to be limited to the
value reported in MAXIMUM UNMAP LBA COUNT in the Block Limits VPD.

Implement a blacklist option that can be used to accommodate devices
with this behavior.

Reported-by: Bill Kuzeja 
Reported-by: Ewan D. Milne 
Signed-off-by: Martin K. Petersen 
---
 drivers/scsi/scsi_scan.c|  3 +++
 drivers/scsi/sd.c   | 16 
 include/scsi/scsi_device.h  |  1 +
 include/scsi/scsi_devinfo.h |  1 +
 4 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index e7818afeda2b..15590a063ad9 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -956,6 +956,9 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned 
char *inq_result,
if (*bflags & BLIST_NO_DIF)
sdev->no_dif = 1;
 
+   if (*bflags & BLIST_UNMAP_LIMIT_WS)
+   sdev->unmap_limit_for_ws = 1;
+
sdev->eh_timeout = SCSI_DEFAULT_EH_TIMEOUT;
 
if (*bflags & BLIST_TRY_VPD_PAGES)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index b18ba3235900..347be7580181 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -715,13 +715,21 @@ static void sd_config_discard(struct scsi_disk *sdkp, 
unsigned int mode)
break;
 
case SD_LBP_WS16:
-   max_blocks = min_not_zero(sdkp->max_ws_blocks,
- (u32)SD_MAX_WS16_BLOCKS);
+   if (sdkp->device->unmap_limit_for_ws)
+   max_blocks = sdkp->max_unmap_blocks;
+   else
+   max_blocks = sdkp->max_ws_blocks;
+
+   max_blocks = min_not_zero(max_blocks, (u32)SD_MAX_WS16_BLOCKS);
break;
 
case SD_LBP_WS10:
-   max_blocks = min_not_zero(sdkp->max_ws_blocks,
- (u32)SD_MAX_WS10_BLOCKS);
+   if (sdkp->device->unmap_limit_for_ws)
+   max_blocks = sdkp->max_unmap_blocks;
+   else
+   max_blocks = sdkp->max_ws_blocks;
+
+   max_blocks = min_not_zero(max_blocks, (u32)SD_MAX_WS10_BLOCKS);
break;
 
case SD_LBP_ZERO:
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 82e93ee94708..67c5a9f223f7 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -192,6 +192,7 @@ struct scsi_device {
unsigned no_dif:1;  /* T10 PI (DIF) should be disabled */
unsigned broken_fua:1;  /* Don't set FUA bit */
unsigned lun_in_cdb:1;  /* Store LUN bits in CDB[1] */
+   unsigned unmap_limit_for_ws:1;  /* Use the UNMAP limit for WRITE SAME */
 
atomic_t disk_events_disable_depth; /* disable depth for disk events */
 
diff --git a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
index 9592570e092a..36b03013d629 100644
--- a/include/scsi/scsi_devinfo.h
+++ b/include/scsi/scsi_devinfo.h
@@ -29,5 +29,6 @@
 #define BLIST_TRY_VPD_PAGES0x1000 /* Attempt to read VPD pages */
 #define BLIST_NO_RSOC  0x2000 /* don't try to issue RSOC */
 #define BLIST_MAX_1024 0x4000 /* maximum 1024 sector cdb length */
+#define BLIST_UNMAP_LIMIT_WS   0x8000 /* Use UNMAP limit for WRITE SAME */
 
 #endif
-- 
2.14.1