On 6/5/25 2:31 PM, John Garry wrote:
> On 04/06/2025 16:09, Nilay Shroff wrote:
>>> I need to test further, but maybe we can change the check to this:
>>>
>>> if (t->io_min <= SECTOR_SIZE || t->io_min == t->physical_block_size) {
>>> /* No chunk sectors, so use bottom device values directly */
>>> t->atomic_write_hw_unit_max = b->atomic_write_hw_unit_max;
>>> t->atomic_write_hw_unit_min = b->atomic_write_hw_unit_min;
>>> t->atomic_write_hw_max = b->atomic_write_hw_max;
>>> return true;
>>> }
>> How about instead adding a new BLK_FEAT_STRIPED flag and then use it here while
>> setting atomic limits as shown below:
>>
>> diff --git a/block/blk-settings.c b/block/blk-settings.c
>> index a000daafbfb4..bf5d35282d42 100644
>> --- a/block/blk-settings.c
>> +++ b/block/blk-settings.c
>> @@ -598,8 +598,14 @@ static bool blk_stack_atomic_writes_head(struct queue_limits *t,
>> !blk_stack_atomic_writes_boundary_head(t, b))
>> return false;
>> - if (t->io_min <= SECTOR_SIZE) {
>> - /* No chunk sectors, so use bottom device values directly */
>> + if (t->io_min <= SECTOR_SIZE || !(t->features & BLK_FEAT_STRIPED)) {
>> + /*
>> + * If there are no chunk sectors, or if the top device does not
>> + * advertise the STRIPED feature (i.e., it's not a striped
>> + * device like md-raid0 or dm-stripe), then we directly inherit
>> + * the atomic write capabilities from the underlying (bottom)
>> + * device.
>> + */
>> t->atomic_write_hw_unit_max = b->atomic_write_hw_unit_max;
>> t->atomic_write_hw_unit_min = b->atomic_write_hw_unit_min;
>> t->atomic_write_hw_max = b->atomic_write_hw_max;
>>
>> I tested the above change with md-raid0 and dm-strip setup and seems to
>> be working well. What do you think?
>
> I would hope that we don't require this complexity.
>
> I think that this check should be fine:
>
> if (t->io_min <= t->physical_block_size) {
>
> }
>
> But I have found a method to break atomic writes for raid10 on mainline with that - that is if I have physical_block_size > chunk size. This ends up that atomic write unit max comes directly from the bottom device atomic write unit max, and it should be limited by chunk size.
>
> Let me send a patch for that, and we can go from there - ok?
Yeah sure, we need to fix this. Please send your changes and I'd be glad to
help review and test your changes on my setup.
Thanks,
--Nilay
