From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751434AbbIJEFA (ORCPT ); Thu, 10 Sep 2015 00:05:00 -0400 Received: from mga03.intel.com ([134.134.136.65]:8442 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750727AbbIJEE4 (ORCPT ); Thu, 10 Sep 2015 00:04:56 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,501,1437462000"; d="scan'208";a="641959039" Date: Thu, 10 Sep 2015 04:15:20 +0800 From: Yuyang Du To: Dietmar Eggemann Cc: Vincent Guittot , Steve Muckle , Morten Rasmussen , "peterz@infradead.org" , "mingo@redhat.com" , "daniel.lezcano@linaro.org" , "mturquette@baylibre.com" , "rjw@rjwysocki.net" , Juri Lelli , "sgurrappadi@nvidia.com" , "pang.xunlei@zte.com.cn" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig Message-ID: <20150909201519.GB21833@intel.com> References: <1439569394-11974-1-git-send-email-morten.rasmussen@arm.com> <1439569394-11974-6-git-send-email-morten.rasmussen@arm.com> <55E8DD00.2030706@linaro.org> <55EDAF43.30500@arm.com> <55EDDD5A.70904@arm.com> <55EED99E.2040100@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55EED99E.2040100@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 On Tue, Sep 08, 2015 at 01:50:38PM +0100, Dietmar Eggemann wrote: > > It's both a unit and a SCALE/SHIFT problem, SCHED_LOAD_SHIFT and > > SCHED_CAPACITY_SHIFT are defined separately so we must be sure to > > scale the value in the right range. In the case of cpu_usage which > > returns sa->util_avg , it's the capacity range not the load range. > > Still don't understand why it's a unit problem. IMHO LOAD/UTIL and > CAPACITY have no unit. To be more accurate, probably, LOAD can be thought of as having unit, but UTIL has no unit. Anyway, those are my definitions: 1) unit, only for LOAD, and SCHED_LOAD_X is the unit (but SCHED_LOAD_RESOLUTION make it also some 2, see below) 2) range, aka, resolution or fix-point percentage (as Ben said) 3) timing ratio, LOAD_AVG_MAX etc, unralated with SCHED_LOAD_X > >> I always thought that scale_load_down() takes care of that. > > > > AFAIU, scale_load_down is a way to increase the resolution of the > > load not to move from load to capacity > > I tried to figure out why we have this issue when comparing UTIL w/ > CAPACITY and not LOAD w/ CAPACITY: > > Both are initialized like that: > > sa->load_avg = scale_load_down(se->load.weight); > sa->load_sum = sa->load_avg * LOAD_AVG_MAX; > sa->util_avg = scale_load_down(SCHED_LOAD_SCALE); > sa->util_sum = LOAD_AVG_MAX; > > and we use 'se->on_rq * scale_load_down(se->load.weight)' as 'unsigned > long weight' argument to call __update_load_avg() making sure the > scaling differences between LOAD and CAPACITY are respected while > updating sa->load_sum (and sa->load_avg). Yes, because we used SCHED_LOAD_X as both unit and range for LOAD. > OTAH, we don't apply a scale_load_down for sa->util_[sum/avg] only a '<< > SCHED_LOAD_SHIFT) / LOAD_AVG_MAX' on sa->util_avg. > So changing '<< SCHED_LOAD_SHIFT' to '* > scale_load_down(SCHED_LOAD_SCALE)' would be the logical thing to do. Actually, for UTIL, we only need range, so don't conflate with LOAD, what about we get all these clarified by redefining SCHED_LOAD_RESOLUTION as the resolution/range generic macro for LOAD, UTIL, and CAPACITY: #define SCHED_RESOLUTION_SHIFT 10 #define SCHED_RESOLUTION_SCALE (1L << SCHED_RESOLUTION_SHIFT) #if 0 /* BITS_PER_LONG > 32 -- currently broken: it increases power usage under light load */ # define scale_load(w) ((w) << SCHED_RESOLUTION_SHIFT) # define scale_load_down(w) ((w) >> SCHED_RESOLUTION_SHIFT) # define SCHED_LOAD_SHIFT (10 + SCHED_RESOLUTION_SHIFT) #else # define scale_load(w) (w) # define scale_load_down(w) (w) # define SCHED_LOAD_SHIFT (10) #endif #define SCHED_LOAD_SCALE (1L << SCHED_LOAD_SHIFT) For UTIL, e.g., it will be initiated as: sa->util_avg = SCHED_RESOLUTION_SCALE; And for capacity, we just use SCHED_RESOLUTION_SHIFT (so SCHED_CAPACITY_SHIFT is not needed). Thanks, Yuyang