Re: [PATCH v2 4/4] block/mq-deadline: Prevent zoned write reordering due to I/O prioritization

[Damien Le Moal <dlemoal@kernel.org>]

On 12/20/23 09:48, Bart Van Assche wrote:
> On 12/19/23 16:05, Damien Le Moal wrote:
>> On 12/20/23 02:42, Bart Van Assche wrote:
>>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>>> index c11c97afa0bc..668888103a47 100644
>>> --- a/block/blk-mq.c
>>> +++ b/block/blk-mq.c
>>> @@ -2922,6 +2922,13 @@ static bool blk_mq_can_use_cached_rq(struct request *rq, struct blk_plug *plug,
>>>
>>>    static void bio_set_ioprio(struct bio *bio)
>>>    {
>>> +	/*
>>> +	 * Do not set the I/O priority of sequential zoned write bios because
>>> +	 * this could lead to reordering and hence to unaligned write errors.
>>> +	 */
>>> +	if (blk_bio_is_seq_zoned_write(bio))
>>> +		return;
>>
>> That is not acceptable as that will ignore priorities passed for async direct
>> IOs through aio->aio_reqprio. That one is a perfectly acceptable use case and we
>> should not ignore it.
> 
> Hi Damien,
> 
> What you wrote is wrong. bio_set_ioprio() applies the I/O priority set
> by ionice or by the blk-ioprio cgroup policy. The above patch does not
> affect the priorities set via aio_reqprio. aio_reqprio is still copied
> in ki_ioprio and ki_ioprio is still copied into bi_ioprio by the direct
> I/O code.

OK. But your patch will still endup with IO priorities being ignored for
legitimate use cases that do not lead to mixed-priorities. E.g. applications
using directly the raw device and doing writes to a zone without mixing
priorities, either with AIO, ionice or cgroups.

The issue is when a user mixes different IO priorities for writes to the same
zone, and as I said before, since doing that is nonsensical, getting the IOs to
fail is fine by me. The user will then be aware that this should not be done.

f2fs has a problem with that though as that leads to write errors and FS going
read-only (I guess). btrfs will not have this issue because it uses zone append.
Need to check dm-zoned as their may be an issue there.

So what about what I proposed in an earlier email: introduce a bio flag "ignore
ioprio" that causes bio_set_ioprio() to not set any IO priority and have f2fs
set that flag for any zone write BIO it issues ? That will solve your f2fs issue
without messing up good use cases.

-- 
Damien Le Moal
Western Digital Research