From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S935097AbcATKdz (ORCPT <rfc822;w@1wt.eu>);
	Wed, 20 Jan 2016 05:33:55 -0500
Received: from foss.arm.com ([217.140.101.70]:44436 "EHLO foss.arm.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S934951AbcATKdw (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Wed, 20 Jan 2016 05:33:52 -0500
Date: Wed, 20 Jan 2016 10:34:21 +0000
From: Juri Lelli <juri.lelli@arm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: mingo@kernel.org, linux-kernel@vger.kernel.org,
        Thomas Gleixner <tglx@linutronix.de>,
        Steven Rostedt <rostedt@goodmis.org>,
        Juri Lelli <juri.lelli@gmail.com>,
        Andrea Parri <parri.andrea@gmail.com>
Subject: Re: [PATCH] sched: Fix PI handling vs sched_setscheduler()
Message-ID: <20160120103421.GQ8573@e106622-lin>
References: <20160119111841.GZ6344@twins.programming.kicks-ass.net>
 <20160120100839.GL8573@e106622-lin>
 <20160120102425.GI6357@twins.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160120102425.GI6357@twins.programming.kicks-ass.net>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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
>   *
>   */
>  
>