Messages in this thread | | | Date | Tue, 11 Mar 2014 11:48:49 +0100 (CET) | From | Thomas Gleixner <> | Subject | Re: [PATCH v2] sched: Fix broken setscheduler() |
| |
On Tue, 11 Mar 2014, Juri Lelli wrote: > On Mon, 10 Mar 2014 17:37:31 -0400 > Steven Rostedt <rostedt@goodmis.org> wrote: > > > On Mon, 10 Mar 2014 22:18:56 +0100 (CET) > > Thomas Gleixner <tglx@linutronix.de> wrote: > > > > > > > Lemme look at it tomorrow again with an awake brain. This seems to be > > > some forward porting hickup which needs a closer look. Just look at > > > > Yep, I talked with Sebastian on IRC and that seems to be the case. > > > > > the 3.10-rt version of this: > > > > > > @@ -3825,20 +3826,25 @@ static struct task_struct *find_process_by_pid(pid_t pid) > > > return pid ? find_task_by_vpid(pid) : current; > > > } > > > > > > -/* Actually do priority change: must hold rq lock. */ > > > -static void > > > -__setscheduler(struct rq *rq, struct task_struct *p, int policy, int prio) > > > +static void __setscheduler_params(struct task_struct *p, int policy, int prio) > > > { > > > p->policy = policy; > > > p->rt_priority = prio; > > > p->normal_prio = normal_prio(p); > > > + set_load_weight(p); > > > +} > > > > > > That code has changed significantly probably due to the EDF merge. We > > > need to figure out whether there is more damage due to that. > > > > Yeah, when I looked at the -rt version, it appeared to have my fix > > already. But in reality, the forward port broke it. Here's the problem > > part of the commit: > > > > + set_load_weight(p); > > +} > > > > - p->normal_prio = normal_prio(p); > > - p->prio = rt_mutex_getprio(p); > > > > Your patch never deleted the above two. And it kept them in the > > locations that I placed them in, in my patch.
Correct.
> > Oh, and you have to have also something like this > > @@ -3271,7 +3271,8 @@ static int __sched_setscheduler(struct task_struct *p, > const struct sched_attr *attr, > bool user) > { > - int newprio = MAX_RT_PRIO - 1 - attr->sched_priority; > + int newprio = dl_policy(attr->sched_policy) ? MAX_DL_PRIO - 1 : > + MAX_RT_PRIO - 1 - attr->sched_priority;
Right, you beat me.
> int retval, oldprio, oldpolicy = -1, on_rq, running; > int policy = attr->sched_policy; > unsigned long flags; > > or you can fail to become DL if you are currently boosted by RT, as > attr->sched_priority == 0 for DL tasks. Then rt_mutex_check_prio () > returns 1 (just update params) if setting DL params for a task already > boosted by another DL. But this seems to be ok, as we are already > outside enforcement in this situation. (This is going to be trickier > with proxy exec, though :/).
Steve, can you please send an updated one?
Thanks,
tglx
| |