On 12/9/23 03:40, Bart Van Assche wrote:
> On 12/7/23 17:37, Damien Le Moal wrote:
>> On 12/8/23 09:03, Bart Van Assche wrote:
>>> +static bool dd_use_io_priority(struct deadline_data *dd, enum req_op op)
>>> +{
>>> + return dd->prio_aging_expire != 0 && !op_needs_zoned_write_locking(op);
>>> +}
>>
>> Hard NACK on this. The reason is that this will disable IO priority also for
>> sensible use cases that use libaio/io_uring with direct IOs, with an application
>> that does the right thing for writes, namely assigning the same priority for all
>> writes to a zone. There are some use cases like this in production.
>>
>> I do understand that there is a problem when IO priorities come from cgroups and
>> the user go through a file system. But that should be handled by the file
>> system. That is, for f2fs, all writes going to the same zone should have the
>> same priority. Otherwise, priority inversion issues will lead to non sequential
>> write patterns.
>>
>> Ideally, we should indeed have a generic solution for the cgroup case. But it
>> seems that for now, the simplest thing to do is to not allow priorities through
>> cgroups for writes to zoned devices, unless cgroups is made more intellignet
>> about it and manage bio priorities per zone to avoid priority inversion within a
>> zone.
>
> Hi Damien,
>
> My understanding is that blkcg_set_ioprio() is called from inside submit_bio()
> and hence that the reported issue cannot be solved by modifying F2FS. How about
> modifying the blk-ioprio policy such that it ignores zoned writes?
I do not see a better solution than that at the moment. So yes, let's do that.
But please add a big comment in the code explaining why we ignore zoned writes.
--
Damien Le Moal
Western Digital Research
