* Re: [PATCH] sched: make p->prio independent of p->mm
[not found] <20200423040128.6120-1-hdanton@sina.com>
@ 2020-04-23 9:26 ` Peter Zijlstra
2020-04-23 13:44 ` Steven Rostedt
[not found] ` <20200423141609.5224-1-hdanton@sina.com>
0 siblings, 2 replies; 5+ messages in thread
From: Peter Zijlstra @ 2020-04-23 9:26 UTC (permalink / raw)
To: Hillf Danton; +Cc: Steven Rostedt, Mike Galbraith, lkml, Ingo Molnar
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sched: make p->prio independent of p->mm
2020-04-23 9:26 ` [PATCH] sched: make p->prio independent of p->mm Peter Zijlstra
@ 2020-04-23 13:44 ` Steven Rostedt
[not found] ` <20200423141609.5224-1-hdanton@sina.com>
1 sibling, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2020-04-23 13:44 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Hillf Danton, Mike Galbraith, lkml, Ingo Molnar
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.
-- Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sched: make p->prio independent of p->mm
[not found] ` <20200423141609.5224-1-hdanton@sina.com>
@ 2020-04-23 15:01 ` Steven Rostedt
[not found] ` <20200424003028.14800-1-hdanton@sina.com>
1 sibling, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2020-04-23 15:01 UTC (permalink / raw)
To: Hillf Danton; +Cc: Peter Zijlstra, Mike Galbraith, lkml, Ingo Molnar
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sched: make p->prio independent of p->mm
[not found] ` <20200424003028.14800-1-hdanton@sina.com>
@ 2020-04-24 0:41 ` Steven Rostedt
[not found] ` <20200424021231.13676-1-hdanton@sina.com>
1 sibling, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2020-04-24 0:41 UTC (permalink / raw)
To: Hillf Danton; +Cc: Peter Zijlstra, Mike Galbraith, lkml, Ingo Molnar
On Fri, 24 Apr 2020 08:30:28 +0800
Hillf Danton <hdanton@sina.com> wrote:
> > 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;
> >
> Got it, thank you :) Spin in couple of days.
I still think getting rid of MAX_USER_RT_PRIO would be good too.
-- Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sched: make p->prio independent of p->mm
[not found] ` <20200424021231.13676-1-hdanton@sina.com>
@ 2020-04-24 12:43 ` Steven Rostedt
0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2020-04-24 12:43 UTC (permalink / raw)
To: Hillf Danton; +Cc: Peter Zijlstra, Mike Galbraith, lkml, Ingo Molnar
On Fri, 24 Apr 2020 10:12:31 +0800
Hillf Danton <hdanton@sina.com> wrote:
> Yes and if you agree, we send it home during the 5.8 cycle, or not before
> fifo is reclaimed from modules.
>
> In the spin it now looks like
>
> --- a/include/linux/sched/prio.h
> +++ b/include/linux/sched/prio.h
> @@ -12,15 +12,12 @@
> * tasks are in the range MAX_RT_PRIO..MAX_PRIO-1. Priority
> * values are inverted: lower p->prio value means higher priority.
> *
> - * 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. Note:
> - * MAX_RT_PRIO must not be smaller than MAX_USER_RT_PRIO.
> + * Note: MAX_USER_RT_PRIO will be removed as early as 5.8,
> + * don't use it in new code.
> */
>
> -#define MAX_USER_RT_PRIO 100
> -#define MAX_RT_PRIO MAX_USER_RT_PRIO
> +#define MAX_RT_PRIO 100
> +#define MAX_USER_RT_PRIO MAX_RT_PRIO
>
> #define MAX_PRIO (MAX_RT_PRIO + NICE_WIDTH)
> #define DEFAULT_PRIO (MAX_RT_PRIO + NICE_WIDTH / 2)
No one has used it in years, I don't think we need this change. Just delete
it in one go.
Is making p->prio independent from p->mm needed for other changes? If not,
then we can hold off this change until we do so, otherwise, I would keep
your original patch as is, and then remove the extra check when we remove
MAX_USER_RT_PRIO.
-- Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-04-24 12:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200423040128.6120-1-hdanton@sina.com>
2020-04-23 9:26 ` [PATCH] sched: make p->prio independent of p->mm Peter Zijlstra
2020-04-23 13:44 ` Steven Rostedt
[not found] ` <20200423141609.5224-1-hdanton@sina.com>
2020-04-23 15:01 ` Steven Rostedt
[not found] ` <20200424003028.14800-1-hdanton@sina.com>
2020-04-24 0:41 ` Steven Rostedt
[not found] ` <20200424021231.13676-1-hdanton@sina.com>
2020-04-24 12:43 ` Steven Rostedt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).