On Thu 16-06-22 13:23:03, Jan Kara wrote:
> On Thu 16-06-22 12:15:25, Damien Le Moal wrote:
> > On 6/16/22 01:16, Jan Kara wrote:
> > > + if (ioprio_class == IOPRIO_CLASS_NONE)
> > > + bio->bi_ioprio = get_current_ioprio();
> > > }
> > > /**
> >
> > Beside this comment, I am still scratching my head regarding what the user
> > gets with ioprio_get(). If I understood your patches correctly, the user may
> > still see IOPRIO_CLASS_NONE ?
> > For that case, to be in sync with the man page, I thought the returned
> > ioprio should be the effective one based on the task io nice value, that is,
> > the value returned by get_current_ioprio(). Am I missing something... ?
>
> The trouble with returning "effective ioprio" is that with IOPRIO_WHO_PGRP
> or IOPRIO_WHO_USER the effective IO priority may be different for different
> processes considered and it can be also further influenced by blk-ioprio
> settings. But thinking about it now after things have settled I agree that
> what you suggests makes more sense. I'll fix that. Thanks for suggestion!
Oh, now I've remembered why I've done it that way. With IOPRIO_WHO_PROCESS
(which is probably the most used and the best defined variant), we were
returning IOPRIO_CLASS_NONE if the task didn't have set IO priority until
commit e70344c05995 ("block: fix default IO priority handling"). So my
patch was just making behavior of IOPRIO_WHO_PGRP & IOPRIO_WHO_USER
consistent with the behavior of IOPRIO_WHO_PROCESS. I'd be reluctant to
change the behavior of IOPRIO_WHO_PROCESS because that has the biggest
chances for userspace regressions. But perhaps it makes sense to keep
IOPRIO_WHO_PGRP & IOPRIO_WHO_USER inconsistent with IOPRIO_WHO_PROCESS and
just use effective IO priority in those two variants. That looks like the
smallest API change to make things at least somewhat sensible...
Honza
>
> Honza
> --
> Jan Kara <jack@xxxxxxxx>
> SUSE Labs, CR
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
