From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934875AbcATKYe (ORCPT ); Wed, 20 Jan 2016 05:24:34 -0500 Received: from casper.infradead.org ([85.118.1.10]:53595 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934081AbcATKY3 (ORCPT ); Wed, 20 Jan 2016 05:24:29 -0500 Date: Wed, 20 Jan 2016 11:24:26 +0100 From: Peter Zijlstra To: Juri Lelli Cc: mingo@kernel.org, linux-kernel@vger.kernel.org, Thomas Gleixner , Steven Rostedt , Juri Lelli , Andrea Parri Subject: Re: [PATCH] sched: Fix PI handling vs sched_setscheduler() Message-ID: <20160120102425.GI6357@twins.programming.kicks-ass.net> References: <20160119111841.GZ6344@twins.programming.kicks-ass.net> <20160120100839.GL8573@e106622-lin> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160120100839.GL8573@e106622-lin> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 * */