[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <28fb29cc-1c1b-4b26-a859-c29b6cfa337e@oracle.com>
Date: Wed, 30 Oct 2024 14:03:03 +0000
From: John Garry <john.g.garry@...cle.com>
To: Christoph Hellwig <hch@....de>
Cc: axboe@...nel.dk, song@...nel.org, yukuai3@...wei.com,
linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-raid@...r.kernel.org, martin.petersen@...cle.com
Subject: Re: [PATCH v2 2/5] block: Support atomic writes limits for stacked
devices
On 30/10/2024 13:50, Christoph Hellwig wrote:
>>
>> +static void blk_stack_atomic_writes_limits(struct queue_limits *t, struct queue_limits *b)
> Avoid the overly long line here.
sure
>
>> + if (t->atomic_write_hw_max) {
> Maybe split this branch and the code for when it is not set into
> separate helpers to keep the function to a size where it can be
> easily understood?
I was trying to reduce indentation, but it does read a bit messy now, so
I can try break into a smaller function.
>
>> + /* Check first bottom device limits */
>> + if (!b->atomic_write_hw_boundary)
>> + goto check_unit;
>> + /*
>> + * Ensure atomic write boundary is aligned with chunk sectors. Stacked
>> + * devices store chunk sectors in t->io_min.
>> + */
>> + if (b->atomic_write_hw_boundary > t->io_min &&
>> + b->atomic_write_hw_boundary % t->io_min)
>> + goto unsupported;
>> + else if (t->io_min > b->atomic_write_hw_boundary &&
> No need for the else here.
>
>> + t->io_min % b->atomic_write_hw_boundary)
>> + goto unsupported;
>> +
>> + t->atomic_write_hw_boundary = b->atomic_write_hw_boundary;
>> +
>> +check_unit:
> Maybe instead of the check_unit goto just move the checks between the
> goto above and this into a branch?
I'm not sure, but I can try to avoid using the "goto check_unit" just to
skip code.
>
> Otherwise this looks conceptually fine to me.
ok, thanks!
Powered by blists - more mailing lists