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]

Date:	Wed, 20 Jan 2016 10:34:21 +0000
From:	Juri Lelli <juri.lelli@....com>
To:	Peter Zijlstra <peterz@...radead.org>
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 20/01/16 11:24, Peter Zijlstra wrote:
> 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_REPLENISH - CBS (replenish runtime and postpone deadline)

Maybe? :)

> + * ENQUEUE_WAKING    - sched_class::task_waking was called
>   *
>   */
>  
>