On 24/04/20 05:36, Hillf Danton wrote:
> Set newprio the same way as normal_prio() does after checking schedule
> policy and to MAX_PRIO -1 by default.
>
> What is also added is boundary checks for RT and fair priorities.
>
> Cc: Mike Galbraith <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: Mel Gorman <[email protected]>
> Cc: Vincent Guittot <[email protected]>
> Cc: Phil Auld <[email protected]>
> Cc: Valentin Schneider <[email protected]>
> Cc: Dietmar Eggemann <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Signed-off-by: Hillf Danton <[email protected]>
> ---
>
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -4768,8 +4768,7 @@ static int __sched_setscheduler(struct t
> const struct sched_attr *attr,
> bool user, bool pi)
> {
> - int newprio = dl_policy(attr->sched_policy) ? MAX_DL_PRIO - 1 :
> - MAX_RT_PRIO - 1 - attr->sched_priority;
> + int newprio;
> int retval, oldprio, oldpolicy = -1, queued, running;
> int new_effective_prio, policy = attr->sched_policy;
> const struct sched_class *prev_class;
> @@ -4800,6 +4799,26 @@ recheck:
> return -EINVAL;
>
> /*
> + * compute newprio after checking policy, see normal_prio();
> + * it's used in pi boost below
> + */
> + if (dl_policy(policy)) {
> + newprio = MAX_DL_PRIO - 1;
> + }
> + else if (rt_policy(policy)) {
> + if (attr->sched_priority > MAX_RT_PRIO - 1)
> + return -EINVAL;
> + newprio = MAX_RT_PRIO - 1 - attr->sched_priority;
> + }
> + else if (fair_policy(policy)) {
> + if (attr->sched_nice < MIN_NICE ||
> + attr->sched_nice > MAX_NICE)
> + return -EINVAL;
We can't hit this with the syscall route, since we (silently) clamp those
values in sched_copy_attr(). setpriority() does the same. There's this
comment in sched_copy_attr() that asks whether we should clamp or return an
error; seems like the current consensus is on clamping, but then we might
want to get rid of that comment :)
> + newprio = NICE_TO_PRIO(attr->sched_nice);
This is new, however AFAICT it doesn't change anything for CFS (or about to
be) tasks since what matters is calling check_class_changed() further
down.
> + } else
> + newprio = MAX_PRIO - 1;
> +
> + /*
> * Allow unprivileged RT tasks to decrease priority:
> */
> if (user && !capable(CAP_SYS_NICE)) {
