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@xxxxxxx>
Looks OK to me.
Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> ---
> 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)
--
Damien Le Moal
Western Digital Research
