Re: [PATCH 5/8] sched/cpufreq: pass sched class into cpufreq_update_util

[Peter Zijlstra]

On Wed, Mar 16, 2016 at 01:39:10PM +0100, Rafael J. Wysocki wrote:
> On Wed, Mar 16, 2016 at 9:53 AM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> > On Wed, Mar 16, 2016 at 09:29:59AM +0100, Vincent Guittot wrote:
> >> I wonder if it's really worth passing per sched_class request to
> >> sched_util ? sched_util is about selecting a frequency based on the
> >> utilization of the CPU, it only needs a value that reflect the whole
> >> utilization. Can't we sum  (or whatever the formula we want to apply)
> >> utilizations before calling cpufreq_update_util
> >
> > So I've thought the same; but I'm conflicted, its a shame to compute
> > anything if the call then doesn't do anything with it.
> >
> > And keeping a structure of all the various numbers to pass in also has
> > cost of yet another cacheline to touch.
> 
> In principle we can use high-order bits of util and max to encode the
> information on where they come from.
> 
> Of course, that translates to additional ifs in the governor, but I
> guess they are unavoidable anyway.

Another thing we can do, for as long as we have the indirect function
call anyway, is stuff extra pointers in that same cacheline we pull the
function from.

Something like the below; there's room for 8 pointers (including the
function pointer) in a cacheline.

That would allow the callback to fetch whatever data it feels is
required (could be all of it).

We could also put a u64 *now = &rq->clock in, which would leave another
4 pointers for DL/RT support.

And since we're then back to 1-2 arguments on the function, we can add a
flags/mask field to indicate what changed (and if the function
throttles, it can keep a mask of all that changed since last time it
actually did something, or allow punching through the throttle if our
minimum guarantee changes or whatnot).

(this would of course require we allocate struct update_util_data with
the proper alignment thingies etc..)

Then again, maybe this is somewhat overboard :-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index ba49c9efd0b2..d34d75c5cc93 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -3236,8 +3236,10 @@ static inline unsigned long rlimit_max(unsigned int limit)
 
 #ifdef CONFIG_CPU_FREQ
 struct update_util_data {
-	void (*func)(struct update_util_data *data,
-		     u64 time, unsigned long util, unsigned long max);
+	unsigned long *cfs_util_avg;
+	unsigned long *cfs_util_max;
+
+	void (*func)(struct update_util_data *data, u64 time);
 };
 
 void cpufreq_set_update_util_data(int cpu, struct update_util_data *data);
diff --git a/kernel/sched/cpufreq.c b/kernel/sched/cpufreq.c
index 928c4ba32f68..de5b20b11de3 100644
--- a/kernel/sched/cpufreq.c
+++ b/kernel/sched/cpufreq.c
@@ -32,6 +32,9 @@ void cpufreq_set_update_util_data(int cpu, struct update_util_data *data)
 	if (WARN_ON(data && !data->func))
 		return;
 
+	data->cfs_util_avg = &cpu_rq(cpu)->cfs.avg.util_avg;
+	data->cfs_util_max = &cpu_rq(cpu)->cpu_capacity_orig;
+
 	rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
 }
 EXPORT_SYMBOL_GPL(cpufreq_set_update_util_data);