Re: [PATCH V2 1/2] scsi: sd: Fix sd_config_write_same()

2017-09-01 Thread Damien Le Moal
Martin,

On 9/1/17 12:36, Martin K. Petersen wrote:
> 
> Damien,
> 
>> +if (sdkp->max_ws_blocks &&
>> +sdkp->physical_block_size > logical_block_size) {
>> +/*
>> + * Reporting a maximum number of blocks that is not aligned
>> + * on the device physical size would cause a large write same
>> + * request to be split into physically unaligned chunks by
>> + * __blkdev_issue_write_zeroes() and __blkdev_issue_write_same()
>> + * even if the caller of these functions took care to align the
>> + * large request. So make sure the maximum reported is aligned
>> + * to the device physical block size. This is only an optional
>> + * optimization for regular disks, but this is mandatory to
>> + * avoid failure of large write same requests directed at
>> + * sequential write required zones of host-managed ZBC disks.
>> + */
>> +sector_t phys_mask =
>> +bytes_to_logical(sdkp->device,
>> + sdkp->physical_block_size) - 1;
>> +
>> +sdkp->max_ws_blocks &= ~phys_mask;
>> +}
>> +
>>  out:
>>  blk_queue_max_write_same_sectors(q, sdkp->max_ws_blocks *
>>   (logical_block_size >> 9));
> 
> ALIGN_DOWN(sdkp->max_ws_blocks, sdkp->physical_block_size)?

Sure. But let's use the same unit then :)

sdkp->max_ws_blocks =
ALIGN_DOWN(sdkp->max_ws_blocks,
   bytes_to_logical(sdkp->device, sdkp->physical_block_size));

Isn't it ?

Do you want me to resend ?

Thank you.

-- 
Damien Le Moal,
Western Digital


Re: [PATCH V2 1/2] scsi: sd: Fix sd_config_write_same()

2017-08-31 Thread Martin K. Petersen

Damien,

> + if (sdkp->max_ws_blocks &&
> + sdkp->physical_block_size > logical_block_size) {
> + /*
> +  * Reporting a maximum number of blocks that is not aligned
> +  * on the device physical size would cause a large write same
> +  * request to be split into physically unaligned chunks by
> +  * __blkdev_issue_write_zeroes() and __blkdev_issue_write_same()
> +  * even if the caller of these functions took care to align the
> +  * large request. So make sure the maximum reported is aligned
> +  * to the device physical block size. This is only an optional
> +  * optimization for regular disks, but this is mandatory to
> +  * avoid failure of large write same requests directed at
> +  * sequential write required zones of host-managed ZBC disks.
> +  */
> + sector_t phys_mask =
> + bytes_to_logical(sdkp->device,
> +  sdkp->physical_block_size) - 1;
> +
> + sdkp->max_ws_blocks &= ~phys_mask;
> + }
> +
>  out:
>   blk_queue_max_write_same_sectors(q, sdkp->max_ws_blocks *
>(logical_block_size >> 9));

ALIGN_DOWN(sdkp->max_ws_blocks, sdkp->physical_block_size)?

-- 
Martin K. Petersen  Oracle Linux Engineering