Messages in this thread | | | Date | Thu, 23 Apr 2020 11:01:12 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH] sched: make p->prio independent of p->mm |
| |
On Thu, 23 Apr 2020 22:16:09 +0800 Hillf Danton <hdanton@sina.com> wrote:
> On Thu, 23 Apr 2020 09:44:03 -0400 Steven Rostedt wrote: > > > > On Thu, 23 Apr 2020 11:26:20 +0200 > > Peter Zijlstra <peterz@infradead.org> wrote: > > > > > On Thu, Apr 23, 2020 at 12:01:28PM +0800, Hillf Danton wrote: > > > > --- a/kernel/sched/core.c > > > > +++ b/kernel/sched/core.c > > > > @@ -4796,13 +4796,19 @@ recheck: > > > > return -EINVAL; > > > > > > > > /* > > > > - * Valid priorities for SCHED_FIFO and SCHED_RR are > > > > - * 1..MAX_USER_RT_PRIO-1, valid priority for SCHED_NORMAL, > > > > - * SCHED_BATCH and SCHED_IDLE is 0. > > > > + * The MAX_USER_RT_PRIO value allows the actual maximum > > > > + * RT priority to be separate from the value exported to > > > > + * user-space. This allows kernel threads to set their > > > > + * priority to a value higher than any user task. > > > > */ > > > > - if ((p->mm && attr->sched_priority > MAX_USER_RT_PRIO-1) || > > > > - (!p->mm && attr->sched_priority > MAX_RT_PRIO-1)) > > > > - return -EINVAL; > > > > + if (p->flags & PF_KTHREAD) { > > > > + if (attr->sched_priority > MAX_RT_PRIO - 1) > > > > + return -EINVAL; > > > > + } else { > > > > + if (attr->sched_priority > MAX_USER_RT_PRIO - 1) > > > > + return -EINVAL; > > > > + } > > > > + > > > > > > Arguably we can do away with the check entirely, MAX_RT_PRIO == > > > MAX_USER_RT_PRIO. > > > > Heh, that was one of my first patches accepted in the mainline kernel! :-) > > > > And the reason we added it, was because there was a small time when the RT > > patch (or my variation of it) had MAX_USER_RT_PRIO and MAX_RT_PRIO different > > values, and would crash in that case here. > > > > d46523ea32a79 ("fix MAX_USER_RT_PRIO and MAX_RT_PRIO") > > > > I would say if we get rid of that check, get rid of the MAX_USER_RT_PRIO > > with it, and make everything use MAX_RT_PRIO. > > BTW the newprio compuation at the beginning of the function looks > questionable if that check is axed without anything added, because > it's then used in the case of pi boost.
I believe Peter meant axing the double check, not the check together.
That is, instead of:
if (p->flags & PF_KTHREAD) { if (attr->sched_priority > MAX_RT_PRIO - 1) return -EINVAL; } else { if (attr->sched_priority > MAX_USER_RT_PRIO - 1) return -EINVAL; }
Just have:
if (attr->sched_priority > MAX_RT_PRIO -1) return -EINVAL;
-- Steve
| |