From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751251AbbG0EPc (ORCPT ); Mon, 27 Jul 2015 00:15:32 -0400 Received: from mga02.intel.com ([134.134.136.20]:54115 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750723AbbG0EPb (ORCPT ); Mon, 27 Jul 2015 00:15:31 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,550,1432623600"; d="scan'208";a="530626196" Date: Mon, 27 Jul 2015 04:24:28 +0800 From: Yuyang Du To: Dietmar Eggemann Cc: "mingo@kernel.org" , "peterz@infradead.org" , "linux-kernel@vger.kernel.org" , "pjt@google.com" , "bsegall@google.com" , Morten Rasmussen , "vincent.guittot@linaro.org" , "umgwanakikbuti@gmail.com" , "len.brown@intel.com" , "rafael.j.wysocki@intel.com" , "arjan@linux.intel.com" , "fengguang.wu@intel.com" Subject: Re: [PATCH v10 2/7] sched: Rewrite runnable load and utilization average tracking Message-ID: <20150726202428.GF28512@intel.com> References: <1436918682-4971-1-git-send-email-yuyang.du@intel.com> <1436918682-4971-3-git-send-email-yuyang.du@intel.com> <55B26ABF.2060907@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55B26ABF.2060907@arm.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Dietmar, On Fri, Jul 24, 2015 at 05:41:35PM +0100, Dietmar Eggemann wrote: > Hi Yuyang, > > On 15/07/15 01:04, Yuyang Du wrote: > > [...] > > > @@ -4674,7 +4487,7 @@ static long effective_load(struct task_group *tg, int cpu, long wl, long wg) > > /* > > * w = rw_i + @wl > > */ > > - w = se->my_q->load.weight + wl; > > + w = se->my_q->avg.load_avg + wl; > > > > /* > > * wl = S * s'_i; see (2) > > There is a comment 'Per the above, wl is the new *se->load.weight* > value'. This should be replaced by *se->avg.load_avg*. Also the function > header explains the functionality of effective_load() based on weight > and not sched_avg::load_avg. I think it is already replaced when effective_load is called. About load.weight vs. load_avg, see below. > > @@ -4695,7 +4508,7 @@ static long effective_load(struct task_group *tg, int cpu, long wl, long wg) > > /* > > * wl = dw_i = S * (s'_i - s_i); see (3) > > */ > > - wl -= se->load.weight; > > + wl -= se->avg.load_avg; > > > > /* > > * Recursively apply this logic to all parent groups to compute > > @@ -4769,14 +4582,14 @@ static int wake_affine(struct sched_domain *sd, struct task_struct *p, int sync) > > */ > > if (sync) { > > tg = task_group(current); > > - weight = current->se.load.weight; > > + weight = current->se.avg.load_avg; > > > > this_load += effective_load(tg, this_cpu, -weight, -weight); > > load += effective_load(tg, prev_cpu, 0, -weight); > > } > > > > tg = task_group(p); > > - weight = p->se.load.weight; > > + weight = p->se.avg.load_avg; > > You changed cfs_rq->load.weight to cfs_rq->avg.load_avg and > se->load.weight to se->avg.load_avg in effective_load() and > wake_affine() in v2. > I wasn't able to find explanation why you did this. I mean we still have > to maintain 'struct load_weight' on cfs_rq's and se's representing tg's. Yes, I might not have explained it specifically, but back then, it was simply motivated/reasoned by consistently expressing the load with load_avg. As of now, it is sort of the same, adding as I previously stated, as far as group SE is concerned, we use load_avg, instread of runnable_load_avg or load.weight. As was also suggested by Morten, we need to revisit the bulk of the load balancing code a lot, including rethinking about what to use: load.weight, or runnable_load_avg, or load_avg. I think this patch series is just a starter. Thanks, Yuyang