From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752862AbbJOSeV (ORCPT ); Thu, 15 Oct 2015 14:34:21 -0400 Received: from casper.infradead.org ([85.118.1.10]:38743 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751855AbbJOSeU (ORCPT ); Thu, 15 Oct 2015 14:34:20 -0400 Date: Thu, 15 Oct 2015 20:34:14 +0200 From: Peter Zijlstra To: Byungchul Park Cc: mingo@kernel.org, linux-kernel@vger.kernel.org, yuyang.du@intel.com, pjt@google.com, efault@gmx.de, tglx@linutronix.de Subject: Re: [PATCH v3 1/3] sched: introduce a new migration flag to task_struct Message-ID: <20151015183414.GN3816@twins.programming.kicks-ass.net> References: <1444899676-5852-1-git-send-email-byungchul.park@lge.com> <1444899676-5852-2-git-send-email-byungchul.park@lge.com> <20151015111813.GZ3604@twins.programming.kicks-ass.net> <20151015122644.GA28741@byungchulpark-X58A-UD3R> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151015122644.GA28741@byungchulpark-X58A-UD3R> 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 Thu, Oct 15, 2015 at 09:26:44PM +0900, Byungchul Park wrote: > On Thu, Oct 15, 2015 at 01:18:13PM +0200, Peter Zijlstra wrote: > > On Thu, Oct 15, 2015 at 06:01:14PM +0900, byungchul.park@lge.com wrote: > > > From: Byungchul Park > > > > > > This patch removes a weird coupling between se->avg.last_update_time and > > > the condition checking for migration, and introduce a new migration flag. > > > Now, scheduler can use the flag instead of se->avg.last_update_time to > > > check if migration already happened or not. > > > > Was there a problem with that coupling? This does not explain. > > The reason why i introduce the new flag is that 3/3 patch makes > se->avg.last_update_time non-zero consistently, so we cannot use the > condition "se->avg.last_update_time == 0" to check if migration has > happened. It would have been ever so good to have had that in the Changelog. > > > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h > > > index af6f252..66d0552 100644 > > > --- a/kernel/sched/sched.h > > > +++ b/kernel/sched/sched.h > > > @@ -1158,6 +1158,7 @@ static const u32 prio_to_wmult[40] = { > > > #define ENQUEUE_WAKING 0 > > > #endif > > > #define ENQUEUE_REPLENISH 8 > > > +#define ENQUEUE_MIGRATED 16 > > > > Won't actually apply that.. > > Okay, I got your concern, let me think more.. Ah, so what I means was that that hunk will not apply. I see that what I typed did not express that as clearly as I might have wanted. Current code looks like: #define ENQUEUE_REPLENISH 0x08 #define ENQUEUE_RESTORE 0x10 > By the way, what do you think about the approach of 3/3 patch? Again a wee bit of worry. I like it for 64bits as there its nearly free and does the right thing, but on 32bit we add 2 smp_rmb() invocations (which are not free on ARM etc..). [Note that one could rewrite that function to require but a single rmb].