Re: [PATCH] scsi: sd: provide a new module parameter to set whether SCSI disks support WRITE_SAME_16 by default

[Bart Van Assche <bvanassche@acm.org>]

On 2020-01-19 06:13, AlexChen wrote:
> When the SCSI device is initialized, check whether it supports
> WRITE_SAME_16 or WRITE_SAME_10 in the sd_read_write_same(). If
> the back-end storage device does not support queries, it will not set
> sdkp->ws16 as 1.
> 
> When the WRITE_SAME io is issued through the blkdev_issue_write_same(),
> the WRITE_SAME type is set to WRITE_SAME_10 by default in the
> sd_setup_write_same_cmnd() since of "sdkp->ws16=0". If the storage device
> does not support WRITE_SAME_10, then the SCSI device is set to not support
> WRITE_SAME.
> 
> Currently, some storage devices do not provide queries for
> WRITE_SAME_16/WRITE_SAME_10 support, but they do support WRITE_SAME_16 and
> do not support WRITE_SAME_10. So in order for these devices to use
> WRITE_SAME, we need a new module parameter to set whether SCSI disks
> support WRITE_SAME_16 by default.
> 
> Signed-off-by: AlexChen <alex.chen@huawei.com>
> ---
>  drivers/scsi/sd.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 4f7e7b607..ff368701d 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -104,6 +104,9 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_ZBC);
>  #define SD_MINORS	0
>  #endif
> 
> +static int sd_default_support_ws16;
> +module_param(sd_default_support_ws16, int, 0644);
> +
>  static void sd_config_discard(struct scsi_disk *, unsigned int);
>  static void sd_config_write_same(struct scsi_disk *);
>  static int  sd_revalidate_disk(struct gendisk *);
> @@ -3014,7 +3017,8 @@ static void sd_read_write_same(struct scsi_disk *sdkp, unsigned char *buffer)
>  			sdev->no_write_same = 1;
>  	}
> 
> -	if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, WRITE_SAME_16) == 1)
> +	if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, WRITE_SAME_16) == 1 ||
> +			sd_default_support_ws16)
>  		sdkp->ws16 = 1;
> 
>  	if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, WRITE_SAME) == 1)

Should this be fixed using the quirk mechanism instead of introducing a
new kernel module parameter? Kernel module parameters apply to all SCSI
disk devices irrespective of their vendor and model. The quirk mechanism
can be used to introduce special behavior for certain disk models and
types. See also the output of the following grep command:

$ git grep -nH '& BLIST'

Bart.