On 2022/06/02 0:23, Jan Kara wrote:
> On Thu 02-06-22 00:11:29, Damien Le Moal wrote:
>> On 2022/06/01 23:51, Jan Kara wrote:
>>> ioprio_get(2) can be asked to return the best IO priority from several
>>> tasks (IOPRIO_WHO_PGRP, IOPRIO_WHO_USER). Currently the call treats
>>> tasks without set IO priority as having priority
>>> IOPRIO_CLASS_BE/IOPRIO_BE_NORM however this does not really reflect the
>>> IO priority the task will get (which depends on task's nice value) and
>>> with the following fix it will not even match returned IO priority for a
>>> single task. So fix IO priority comparison to treat unset IO priority as
>>> the lowest possible one. This way we will return IOPRIO_CLASS_NONE
>>> priority only if none of the considered tasks has explicitely set IO
>>> priority, otherwise we return the highest set IO priority. This changes
>>> userspace visible behavior but hopefully the results are clearer and
>>> nothing breaks.
>>>
>>> Signed-off-by: Jan Kara <jack@xxxxxxx>
>>> ---
>>> block/ioprio.c | 5 ++---
>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/block/ioprio.c b/block/ioprio.c
>>> index 2fe068fcaad5..62890391fc80 100644
>>> --- a/block/ioprio.c
>>> +++ b/block/ioprio.c
>>> @@ -157,10 +157,9 @@ static int get_task_ioprio(struct task_struct *p)
>>> int ioprio_best(unsigned short aprio, unsigned short bprio)
>>> {
>>> if (!ioprio_valid(aprio))
>>> - aprio = IOPRIO_DEFAULT;
>>> + return bprio;
>>
>> bprio may not be valid...
>
> Yes, bprio may be from IOPRIO_CLASS_NONE as well and IMHO that is a
> desirable return in that case. ioprio_valid() is IMHO a bit of a misnomer
> because IOPRIO_CLASS_NONE is a valid class and can be even set by
> userspace. It actually means, set IO priority based on task's CPU priority.
> But lets first settle on the desired meaning of ioprio in the discussion
> over patch 3/3. How we should behave in this case is a detail we can sort
> out after the general meaning is clear.
Sounds all good to me.
>
> Honza
--
Damien Le Moal
Western Digital Research
