Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig

From: Yuyang Du <yuyang.du@intel.com>
To: bsegall@google.com
Cc: Peter Zijlstra <peterz@infradead.org>,
	Morten Rasmussen <morten.rasmussen@arm.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steve Muckle <steve.muckle@linaro.org>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"daniel.lezcano@linaro.org" <daniel.lezcano@linaro.org>,
	"mturquette@baylibre.com" <mturquette@baylibre.com>,
	"rjw@rjwysocki.net" <rjw@rjwysocki.net>,
	Juri Lelli <Juri.Lelli@arm.com>,
	"sgurrappadi@nvidia.com" <sgurrappadi@nvidia.com>,
	"pang.xunlei@zte.com.cn" <pang.xunlei@zte.com.cn>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig
Date: Thu, 24 Sep 2015 08:22:40 +0800	[thread overview]
Message-ID: <20150924002240.GG11102@intel.com> (raw)
In-Reply-To: <xm26zj0d84b3.fsf@sword-of-the-dawn.mtv.corp.google.com>

On Wed, Sep 23, 2015 at 09:54:08AM -0700, bsegall@google.com wrote:
> > This second thought made a mistake (what was wrong with me). load_avg is for sure
> > no greater than load with or without blocked load.
> >
> > With that said, it really does not matter what the following numbers are, 32bit or
> > 64bit machine. What matters is that cfs_rq->load.weight is one that needs to worry
> > whether overflow or not, not the load_avg. It is as simple as that.
> >
> > With that, I think we can and should get rid of the scale_load_down()
> > for load_avg.
> 
> load_avg yes is bounded by load.weight, but on 64-bit load_sum is only
> bounded by load.weight * LOAD_AVG_MAX and is the same size as
> load.weight (as I said below). There's still space for anything
> reasonable though with 10 bits of SLR.

You are absolutely right.

> >> > If NICE_0_LOAD is nice-0's load, and if SCHED_LOAD_SHIFT is to say how to get 
> >> > nice-0's load, I don't understand why you want to separate them.
> >> 
> >> SCHED_LOAD_SHIFT is not how to get nice-0's load, it just happens to
> >> have the same value as NICE_0_SHIFT. (I think anyway, SCHED_LOAD_* is
> >> used in precisely one place other than the newish util_avg, and as I
> >> mentioned it's not remotely clear what compute_imbalance is doing theer)
> >
> > Yes, it is not clear to me either.
> >
> > With the above proposal to get rid of scale_load_down() for load_avg, so I think
> > now we can remove SCHED_LOAD_*, and rename scale_load() to user_to_kernel_load(),
> > and raname scale_load_down() to kernel_to_user_load().
> >
> > Hmm?
> 
> I have no opinion on renaming the scale_load functions, it's certainly
> reasonable, but the scale_load names seem fine too.

Without scale_load_down() in load_avg, it seems they are only used when
reading/writing load between user and kernel. I will ponder more, but
lets see whether others have opinion.