linux-kernel - Re: [PATCH] sched: Fix PI handling vs sched

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160120102425.GI6357@twins.programming.kicks-ass.net>
Date:	Wed, 20 Jan 2016 11:24:26 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Juri Lelli <juri.lelli@....com>
Cc:	mingo@...nel.org, linux-kernel@...r.kernel.org,
	Thomas Gleixner <tglx@...utronix.de>,
	Steven Rostedt <rostedt@...dmis.org>,
	Juri Lelli <juri.lelli@...il.com>,
	Andrea Parri <parri.andrea@...il.com>
Subject: Re: [PATCH] sched: Fix PI handling vs sched_setscheduler()

On Wed, Jan 20, 2016 at 10:08:39AM +0000, Juri Lelli wrote:
> Hi,
> 
> On 19/01/16 12:18, Peter Zijlstra wrote:
> 
> [...]
> 
> > @@ -4097,15 +4099,14 @@ static int __sched_setscheduler(struct t
> >  	if (running)
> >  		p->sched_class->set_curr_task(rq);
> >  	if (queued) {
> > -		int enqueue_flags = ENQUEUE_RESTORE;
> >  		/*
> >  		 * We enqueue to tail when the priority of a task is
> >  		 * increased (user space view).
> >  		 */
> > -		if (oldprio <= p->prio)
> > -			enqueue_flags |= ENQUEUE_HEAD;
> > +		if (oldprio < p->prio)
> > +			queue_flags |= ENQUEUE_HEAD;
> 
> Was this condition broken before or it needs to be changed now with this
> patch?

I could not see how we could get there before, seeing how the == case is
handled above.

> > --- a/kernel/sched/sched.h
> > +++ b/kernel/sched/sched.h
> > @@ -1130,18 +1130,35 @@ static inline void finish_lock_switch(st
> >  extern const int sched_prio_to_weight[40];
> >  extern const u32 sched_prio_to_wmult[40];
> >  
> > +/*
> > + * {de,en}queue flags:
> > + *
> > + * SAVE/RESTORE - an otherwise spurious dequeue/enqueue, done to ensure tasks
> > + *                are in a known state which allows modification. Such pairs
> > + *                should preserve as much state as possible.
> > + *
> > + * MOVE - paired with SAVE/RESTORE, explicitly does not preserve the location
> > + *        in the runqueue.
> > + *
> > + * ENQUEUE_HEAD - place at front of runqueue (tail if not specificed)
> > + *
> > + */
> 
> Do we want to document all the flags while we are at it? :)

Probably.. does the below work? I think I bailed on replenish because I
could not come up with a coherent short description, does the below make
any sense?


--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1140,7 +1140,9 @@ extern const u32 sched_prio_to_wmult[40]
  * MOVE - paired with SAVE/RESTORE, explicitly does not preserve the location
  *        in the runqueue.
  *
- * ENQUEUE_HEAD - place at front of runqueue (tail if not specificed)
+ * ENQUEUE_HEAD      - place at front of runqueue (tail if not specificed)
+ * ENQUEUE_REPLENISH - push the CBS slot forward
+ * ENQUEUE_WAKING    - sched_class::task_waking was called
  *
  */