On 28/03/2017 18:48, Bart Van Assche wrote:
>> + if (rq->cmd_flags & REQ_UNMAP) {
>> + switch (sdkp->provisioning_mode) {
>> + case SD_LBP_WS16:
>> + return sd_setup_write_same16_cmnd(cmd, true);
>> + case SD_LBP_WS10:
>> + return sd_setup_write_same10_cmnd(cmd, true);
>> + }
>> + }
>> +
>> if (sdp->no_write_same)
>> return BLKPREP_INVALID;
>> if (sdkp->ws16 || sector > 0xffffffff || nr_sectors > 0xffff)
> Users can change the provisioning mode from user space from SD_LBP_WS16 into
> SD_LBP_WS10 so I'm not sure it's safe to skip the (sdkp->ws16 || sector >
> 0xffffffff || nr_sectors > 0xffff) check if REQ_UNMAP is set.
Yeah, if REQ_UNMAP is set you should probably check sdkp->provisioning_mode
instead of sdkp->ws16, but apart from this it should still go through the
checks below.
Plus, if the provisioning mode is not ws10 or ws16, should
sd_setup_write_zeroes_cmnd:
1) do a WRITE SAME without UNMAP (what Christoph's code does)
2) return BLKPREP_INVALID
3) ignore provisioning mode and do a WRITE SAME with UNMAP
4) do a WRITE SAME without UNMAP for SD_LBP_{ZERO,FULL,DISABLE},
do a WRITE SAME with UNMAP for SD_LBP_{WS10,WS16,UNMAP}.
I'm in favor of (4). The distinction between SD_LBP_UNMAP, SD_LBP_WS10
and SD_LBP_WS16 is as problematic as discard_zeroes_data in my opinion.
Thanks,
Paolo
