Re: [PATCH] block: fix atomic write limits for stacked devices

[John Garry <john.g.garry@oracle.com>]

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?

Thanks,
John