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

[Laurence Oberman <loberman@redhat.com>]

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