On 6/21/22 01:11, Jan Kara wrote:
> Commit e70344c05995 ("block: fix default IO priority handling")
> introduced an inconsistency in get_current_ioprio() that tasks without
> IO context return IOPRIO_DEFAULT priority while tasks with freshly
> allocated IO context will return 0 (IOPRIO_CLASS_NONE/0) IO priority.
> Tasks without IO context used to be rare before 5a9d041ba2f6 ("block:
> move io_context creation into where it's needed") but after this commit
> they became common because now only BFQ IO scheduler setups task's IO
> context. Similar inconsistency is there for get_task_ioprio() so this
> inconsistency is now exposed to userspace and userspace will see
> different IO priority for tasks operating on devices with BFQ compared
> to devices without BFQ. Furthemore the changes done by commit
> e70344c05995 change the behavior when no IO priority is set for BFQ IO
> scheduler which is also documented in ioprio_set(2) manpage:
>
> "If no I/O scheduler has been set for a thread, then by default the I/O
> priority will follow the CPU nice value (setpriority(2)). In Linux
> kernels before version 2.6.24, once an I/O priority had been set using
> ioprio_set(), there was no way to reset the I/O scheduling behavior to
> the default. Since Linux 2.6.24, specifying ioprio as 0 can be used to
> reset to the default I/O scheduling behavior."
>
> So make sure we default to IOPRIO_CLASS_NONE as used to be the case
> before commit e70344c05995. Also cleanup alloc_io_context() to
> explicitely set this IO priority for the allocated IO context to avoid
> future surprises. Note that we tweak ioprio_best() to maintain
> ioprio_get(2) behavior and make this commit easily backportable.
>
> Fixes: e70344c05995 ("block: fix default IO priority handling")
> Signed-off-by: Jan Kara <jack@xxxxxxx>
Looks good to me.
Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> ---
> block/blk-ioc.c | 2 ++
> block/ioprio.c | 4 ++--
> include/linux/ioprio.h | 2 +-
> 3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/block/blk-ioc.c b/block/blk-ioc.c
> index df9cfe4ca532..63fc02042408 100644
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -247,6 +247,8 @@ static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
> INIT_HLIST_HEAD(&ioc->icq_list);
> INIT_WORK(&ioc->release_work, ioc_release_fn);
> #endif
> + ioc->ioprio = IOPRIO_DEFAULT;
> +
> return ioc;
> }
>
> diff --git a/block/ioprio.c b/block/ioprio.c
> index 2fe068fcaad5..2a34cbca18ae 100644
> --- a/block/ioprio.c
> +++ b/block/ioprio.c
> @@ -157,9 +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;
> + aprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM);
> if (!ioprio_valid(bprio))
> - bprio = IOPRIO_DEFAULT;
> + bprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM);
>
> return min(aprio, bprio);
> }
> diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h
> index 3f53bc27a19b..3d088a88f832 100644
> --- a/include/linux/ioprio.h
> +++ b/include/linux/ioprio.h
> @@ -11,7 +11,7 @@
> /*
> * Default IO priority.
> */
> -#define IOPRIO_DEFAULT IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM)
> +#define IOPRIO_DEFAULT IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0)
>
> /*
> * Check that a priority value has a valid class.
--
Damien Le Moal
Western Digital Research
