Message ID | 20170928013512.1058-1-martin.petersen@oracle.com (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
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 <William.Kuzeja@stratus.com> > Reported-by: Ewan D. Milne <emilne@redhat.com> > Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> > --- > 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 0x10000000 /* Attempt to read VPD pages */ > #define BLIST_NO_RSOC 0x20000000 /* don't try to issue RSOC */ > #define BLIST_MAX_1024 0x40000000 /* maximum 1024 sector cdb length */ > +#define BLIST_UNMAP_LIMIT_WS 0x80000000 /* Use UNMAP limit for WRITE SAME */ > > #endif Thanks. Reviewed-by: Ewan D. Milne <emilne@redhat.com>
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 <William.Kuzeja@stratus.com> > Reported-by: Ewan D. Milne <emilne@redhat.com> > Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> > --- > 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 0x10000000 /* Attempt to read VPD > pages */ > #define BLIST_NO_RSOC 0x20000000 /* don't try to > issue RSOC */ > #define BLIST_MAX_1024 0x40000000 /* maximum 1024 > sector cdb length */ > +#define BLIST_UNMAP_LIMIT_WS 0x80000000 /* 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:0x80000000 Device is here [ 16.853083] scsi 4:0:0:50: Direct-Access LIO- ORG thin2 4.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) { max_blocks = sdkp->max_unmap_blocks; printk("RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS10\n"); } else max_blocks = sdkp->max_ws_blocks; max_blocks = min_not_zero(max_blocks, (u32)SD_MAX_WS10_BLOCKS); break; What am I doing wrong to pass the BLIST flags. Thanks Laurence
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:0x80000000 > > What am I doing wrong to pass the BLIST flags. This worked for me: [root@kvm ~]# echo "Linux:scsi_debug:0x80000000" > /proc/scsi/device_info [root@kvm ~]# grep Linux /proc/scsi/device_info 'Linux ' 'scsi_debug ' 0x80000000 [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).
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:0x80000000 > > > > What am I doing wrong to pass the BLIST flags. > > This worked for me: > > [root@kvm ~]# echo "Linux:scsi_debug:0x80000000" > > /proc/scsi/device_info > [root@kvm ~]# grep Linux /proc/scsi/device_info > 'Linux ' 'scsi_debug ' 0x80000000 > [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 <loberman@redhat.com> Sep 29 09:56:11 localhost kernel: scsi 1:0:0:50: Direct-Access LIO- ORG thin2 4.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] 81920000 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
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:0x80000000 > > > > > > What am I doing wrong to pass the BLIST flags. > > > > This worked for me: > > > > [root@kvm ~]# echo "Linux:scsi_debug:0x80000000" > > > /proc/scsi/device_info > > [root@kvm ~]# grep Linux /proc/scsi/device_info > > 'Linux ' 'scsi_debug ' 0x80000000 > > [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 <loberman@redhat.com> > > Sep 29 09:56:11 localhost kernel: scsi 1:0:0:50: Direct- > Access LIO- > ORG thin2 4.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] 81920000 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:0x80000000 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, &scsi_devinfo_proc_fops); if (!p) { error = -ENOMEM; goto out; } #endif /* CONFIG_SCSI_PROC_FS */ out: if (error) scsi_exit_devinfo(); return error; } But I fail to see where we actually copy the string off the kernel line. I intend to add code and test and submit a patch but first wanted to know if its me simply missing something here. Thanks Laurence
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:0x80000000 > > > > > > > > What am I doing wrong to pass the BLIST flags. > > > > > > This worked for me: > > > > > > [root@kvm ~]# echo "Linux:scsi_debug:0x80000000" > > > > /proc/scsi/device_info > > > [root@kvm ~]# grep Linux /proc/scsi/device_info > > > 'Linux ' 'scsi_debug ' 0x80000000 > > > [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 <loberman@redhat.com> > > > > Sep 29 09:56:11 localhost kernel: scsi 1:0:0:50: Direct- > > Access LIO- > > ORG thin2 4.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] 81920000 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:0x80000000 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, > &scsi_devinfo_proc_fops); > if (!p) { > error = -ENOMEM; > goto out; > } > #endif /* CONFIG_SCSI_PROC_FS */ > > out: > if (error) > scsi_exit_devinfo(); > return error; > } > > > But I fail to see where we actually copy the string off the kernel > line. > > I intend to add code and test and submit a patch but first wanted to > know if its me simply missing something here. > > Thanks > Laurence Answering my own post here. As soon as I sent this Ewan emailed me explaining what I was doing wrong. Its working now by using scsi_mod.use_blk_mq=y scsi_mod.dev_flags=LIO-ORG:thin2:0x80000000 [root@segstorage1 ~]# dmesg | grep RHDEBUG [ 1.498639] RHDEBUG:In scsi_init_devinfo scsi_dev_flags=LIO- ORG:thin2:0x80000000 [ 1.499003] RHDEBUG: In scsi_init_devinfo error=0 [ 9.031071] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 9.202887] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 9.246251] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 9.423062] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 9.632754] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 9.781824] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 15.706504] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 16.254131] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 16.373697] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 16.443442] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 16.503806] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 16.582369] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 21.484123] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 21.552131] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 21.692909] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 21.975010] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 22.153413] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 22.685256] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 22.687920] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 22.692079] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 22.697259] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 22.721023] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 22.724256] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 [ 22.728566] RHDEBUG: unmap_limit_for_ws set by kernel flag for case SD_LBP_WS16 Sorry for the noise Thanks Laurence
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 0x10000000 /* Attempt to read VPD pages */ #define BLIST_NO_RSOC 0x20000000 /* don't try to issue RSOC */ #define BLIST_MAX_1024 0x40000000 /* maximum 1024 sector cdb length */ +#define BLIST_UNMAP_LIMIT_WS 0x80000000 /* Use UNMAP limit for WRITE SAME */ #endif
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 <William.Kuzeja@stratus.com> Reported-by: Ewan D. Milne <emilne@redhat.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> --- 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(-)