| Message ID | 20220621102455.13183-5-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: Fix IO priority mess | expand |
On 6/21/22 19:24, 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). > > Fix the code to use the real IO priority task's IO will use. We have to > modify code for ioprio_get(IOPRIO_WHO_PROCESS) to keep returning > IOPRIO_CLASS_NONE priority for tasks without set IO priority as a > special case to maintain userspace visible API. > > Signed-off-by: Jan Kara <jack@suse.cz> Looks OK to me. Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> > --- > block/ioprio.c | 30 +++++++++++++++++++++++------- > 1 file changed, 23 insertions(+), 7 deletions(-) > > diff --git a/block/ioprio.c b/block/ioprio.c > index 8c46f672a0ba..32a456b45804 100644 > --- a/block/ioprio.c > +++ b/block/ioprio.c > @@ -171,10 +171,31 @@ static int get_task_ioprio(struct task_struct *p) > ret = security_task_getioprio(p); > if (ret) > goto out; > - ret = IOPRIO_DEFAULT; > + task_lock(p); > + ret = __get_task_ioprio(p); > + task_unlock(p); > +out: > + return ret; > +} > + > +/* > + * Return raw IO priority value as set by userspace. We use this for > + * ioprio_get(pid, IOPRIO_WHO_PROCESS) so that we keep historical behavior and > + * also so that userspace can distinguish unset IO priority (which just gets > + * overriden based on task's nice value) from IO priority set to some value. > + */ > +static int get_task_raw_ioprio(struct task_struct *p) > +{ > + int ret; > + > + ret = security_task_getioprio(p); > + if (ret) > + goto out; > task_lock(p); > if (p->io_context) > ret = p->io_context->ioprio; > + else > + ret = IOPRIO_DEFAULT; > task_unlock(p); > out: > return ret; > @@ -182,11 +203,6 @@ static int get_task_ioprio(struct task_struct *p) > > static int ioprio_best(unsigned short aprio, unsigned short bprio) > { > - if (!ioprio_valid(aprio)) > - aprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM); > - if (!ioprio_valid(bprio)) > - bprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM); > - > return min(aprio, bprio); > } > > @@ -207,7 +223,7 @@ SYSCALL_DEFINE2(ioprio_get, int, which, int, who) > else > p = find_task_by_vpid(who); > if (p) > - ret = get_task_ioprio(p); > + ret = get_task_raw_ioprio(p); > break; > case IOPRIO_WHO_PGRP: > if (!who)
diff --git a/block/ioprio.c b/block/ioprio.c index 8c46f672a0ba..32a456b45804 100644 --- a/block/ioprio.c +++ b/block/ioprio.c @@ -171,10 +171,31 @@ static int get_task_ioprio(struct task_struct *p) ret = security_task_getioprio(p); if (ret) goto out; - ret = IOPRIO_DEFAULT; + task_lock(p); + ret = __get_task_ioprio(p); + task_unlock(p); +out: + return ret; +} + +/* + * Return raw IO priority value as set by userspace. We use this for + * ioprio_get(pid, IOPRIO_WHO_PROCESS) so that we keep historical behavior and + * also so that userspace can distinguish unset IO priority (which just gets + * overriden based on task's nice value) from IO priority set to some value. + */ +static int get_task_raw_ioprio(struct task_struct *p) +{ + int ret; + + ret = security_task_getioprio(p); + if (ret) + goto out; task_lock(p); if (p->io_context) ret = p->io_context->ioprio; + else + ret = IOPRIO_DEFAULT; task_unlock(p); out: return ret; @@ -182,11 +203,6 @@ static int get_task_ioprio(struct task_struct *p) static int ioprio_best(unsigned short aprio, unsigned short bprio) { - if (!ioprio_valid(aprio)) - aprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM); - if (!ioprio_valid(bprio)) - bprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM); - return min(aprio, bprio); } @@ -207,7 +223,7 @@ SYSCALL_DEFINE2(ioprio_get, int, which, int, who) else p = find_task_by_vpid(who); if (p) - ret = get_task_ioprio(p); + ret = get_task_raw_ioprio(p); break; case IOPRIO_WHO_PGRP: if (!who)
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). Fix the code to use the real IO priority task's IO will use. We have to modify code for ioprio_get(IOPRIO_WHO_PROCESS) to keep returning IOPRIO_CLASS_NONE priority for tasks without set IO priority as a special case to maintain userspace visible API. Signed-off-by: Jan Kara <jack@suse.cz> --- block/ioprio.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-)