On 2020/07/16 23:35, Christoph Hellwig wrote:
> On Thu, Jul 16, 2020 at 07:09:33PM +0900, Johannes Thumshirn wrote:
>> Max append sectors needs to be aligned to physical block size, otherwise
>> we can end up in a situation where it's off by 1-3 sectors which would
>> cause short writes with asynchronous zone append submissions from an FS.
>
> Huh? The physical block size is purely a hint.
For ZBC/ZAC SMR drives, all writes must be aligned to the physical sector size.
However, sd/sd_zbc does not change max_hw_sectors_kb to ensure alignment to 4K
on 512e disks. There is also nullblk which uses the default max_hw_sectors_kb to
255 x 512B sectors, which is not 4K aligned if the nullb device is created with
4K block size.
So we can fix each driver separately to ensure that we end up with a sensible
max_hw_sectors_kb and by extension a sensible value for max_zone_append_sectors,
or we could force this alignment in blk_queue_max_hw_sectors().
Right now, without any fix, there are setups that end up having weird values
that the user have to sort through and may have a hard time making sense of.
>
>>
>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
>> ---
>> block/blk-settings.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/block/blk-settings.c b/block/blk-settings.c
>> index 9a2c23cd9700..d75c4cc34a7a 100644
>> --- a/block/blk-settings.c
>> +++ b/block/blk-settings.c
>> @@ -231,6 +231,7 @@ EXPORT_SYMBOL(blk_queue_max_write_zeroes_sectors);
>> void blk_queue_max_zone_append_sectors(struct request_queue *q,
>> unsigned int max_zone_append_sectors)
>> {
>> + unsigned int phys = queue_physical_block_size(q);
>> unsigned int max_sectors;
>>
>> if (WARN_ON(!blk_queue_is_zoned(q)))
>> @@ -246,6 +247,13 @@ void blk_queue_max_zone_append_sectors(struct request_queue *q,
>> */
>> WARN_ON(!max_sectors);
>>
>> + /*
>> + * Max append sectors needs to be aligned to physical block size,
>> + * otherwise we can end up in a situation where it's off by 1-3 sectors
>> + * which would cause short writes with asynchronous zone append
>> + * submissions from an FS.
>> + */
>> + max_sectors = ALIGN_DOWN(max_sectors << 9, phys) >> 9;
>> q->limits.max_zone_append_sectors = max_sectors;
>> }
>> EXPORT_SYMBOL_GPL(blk_queue_max_zone_append_sectors);
>> --
>> 2.26.2
>>
> ---end quoted text---
>
--
Damien Le Moal
Western Digital Research
