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.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
