Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1347699 > unrolled thread

[PATCH 0/6] cpufreq: schedutil governor

Started by"Rafael J. Wysocki" <rjw@rjwysocki.net>
First post2016-03-02 03:30 +0100
Last post2016-03-04 04:40 +0100
Articles 20 on this page of 40 — 8 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/6] cpufreq: schedutil governor "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-02 03:30 +0100
    [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-02 03:30 +0100
      Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Vincent Guittot <vincent.guittot@linaro.org> - 2016-03-02 18:20 +0100
        Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data "Rafael J. Wysocki" <rafael@kernel.org> - 2016-03-02 19:00 +0100
          Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data "Rafael J. Wysocki" <rafael@kernel.org> - 2016-03-02 23:50 +0100
            Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 13:30 +0100
              Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Juri Lelli <juri.lelli@arm.com> - 2016-03-03 13:40 +0100
              Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data "Rafael J. Wysocki" <rafael@kernel.org> - 2016-03-03 17:30 +0100
                Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 17:40 +0100
                  Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 17:50 +0100
                    Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data "Rafael J. Wysocki" <rafael@kernel.org> - 2016-03-04 02:20 +0100
                  Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 18:00 +0100
                    Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Juri Lelli <juri.lelli@arm.com> - 2016-03-03 18:20 +0100
                  Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Juri Lelli <juri.lelli@arm.com> - 2016-03-03 18:00 +0100
            Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Vincent Guittot <vincent.guittot@linaro.org> - 2016-03-03 15:10 +0100
              Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 16:40 +0100
                Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 17:30 +0100
                  Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 17:50 +0100
                  Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Dietmar Eggemann <dietmar.eggemann@arm.com> - 2016-03-03 18:30 +0100
                    Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 19:30 +0100
                      Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Dietmar Eggemann <dietmar.eggemann@arm.com> - 2016-03-03 20:20 +0100
                  Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data "Rafael J. Wysocki" <rafael@kernel.org> - 2016-03-03 20:00 +0100
          Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Vincent Guittot <vincent.guittot@linaro.org> - 2016-03-03 14:10 +0100
            Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Steve Muckle <steve.muckle@linaro.org> - 2016-03-03 21:10 +0100
              Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data "Rafael J. Wysocki" <rafael@kernel.org> - 2016-03-03 21:30 +0100
                Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Steve Muckle <steve.muckle@linaro.org> - 2016-03-03 22:40 +0100
    [PATCH v2 10/10] cpufreq: schedutil: New governor based on scheduler utilization data "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
    [PATCH v2 6/10] cpufreq: Support for fast frequency switching "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
    [PATCH v2 0/10] cpufreq: schedutil governor "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
      [PATCH v2 1/10] cpufreq: Reduce cpufreq_update_util() overhead a bit "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
      [PATCH v2 5/10] cpufreq: Move governor attribute set headers to cpufreq.h "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
        Re: [PATCH v2 5/10] cpufreq: Move governor attribute set headers to  cpufreq.h Viresh Kumar <viresh.kumar@linaro.org> - 2016-03-04 07:00 +0100
      [PATCH v2 2/10][Resend] cpufreq: acpi-cpufreq: Make read and write operations more efficient "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
      [PATCH v2 3/10] cpufreq: governor: New data type for management part of dbs_data "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
        Re: [PATCH v2 3/10] cpufreq: governor: New data type for management  part of dbs_data Viresh Kumar <viresh.kumar@linaro.org> - 2016-03-04 07:00 +0100
      [PATCH v2 7/10] cpufreq: Rework the scheduler hooks for triggering updates "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
      [PATCH v2 4/10] cpufreq: governor: Move abstract gov_attr_set code to seperate file "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
        Re: [PATCH v2 4/10] cpufreq: governor: Move abstract gov_attr_set  code to seperate file Viresh Kumar <viresh.kumar@linaro.org> - 2016-03-04 07:00 +0100
      [PATCH v2 8/10] cpufreq: Move scheduler-related code to the sched directory "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
      [PATCH v2 9/10] cpufreq: sched: Re-introduce cpufreq_update_util() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100

Page 1 of 2  [1] 2  Next page →


#1347699 — [PATCH 0/6] cpufreq: schedutil governor

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-03-02 03:30 +0100
Subject[PATCH 0/6] cpufreq: schedutil governor
Message-ID<r84I2-18B-15@gated-at.bofh.it>
Hi,

My previous intro message still applies somewhat, so here's a link:

http://marc.info/?l=linux-pm&m=145609673008122&w=2

The executive summary of the motivation is that I wanted to do two things:
use the utilization data from the scheduler (it's passed to the governor
as aguments of update callbacks anyway) and make it possible to set
CPU frequency without involving process context (fast frequency switching).

Both have been prototyped in the previous RFCs:

https://patchwork.kernel.org/patch/8426691/
https://patchwork.kernel.org/patch/8426741/

but in the meantime I found a couple of issues in there.

First off, the common governor code relied on by the previous version reset
the sample delay to 0 in order to force an immediate frequency update.  That
doesn't work with the new governor, though, because it computes the frequency
to set in a cpufreq_update_util() callback and (when fast switching is not
used) passes that to a work item which sets the frequency and then restores
the sample delay.  Thus if sysfs changes the sample delay to 0 when work_in_progress
is in effect, it will be overwritten by the work item and so discarded.

When using fast switching, the previous version would update the sample delay
from a scheduler path, but that (on a 32-bit system) might clash with an
update from sysfs leading to a result that's completely off.  That value would
be less than the correct sample delay (I think), so in practice that shouldn't
matter that much, but still it's not nice.

The above means that schedutil cannot really share as much code as I thought it
could with "ondemand" and "conservative".

Moreover, I wanted to have a "rate_limit" tunable (instead of the sampling rate
which doesn't mean what the name suggests in schedutil), but that would be the
only one used by schedutil, so I ended up having to define a new struct to point
to from struct dbs_data just to hold that single value and I would need to
define ->init() and ->exit() callbacks for the governor for that reason (and
the common tunables in struct dbs_data wouldn't be used).

Not to mention the fact that the majority of the common governor code is not
really used by schedutil anyway.

Taking the above into account, I decided to decouple schedutil from the other
governors, but I wanted to avoid duplicating some of the tunables manipulation
code.  Hence patches [3-4/6] taking that code into a separate file so schedutil
can use it too without pulling the rest of the common "ondemand" and
"conservative" code along with it.

Patch [5/6] adds support for fast switching to the core and the ACPI driver,
but doesn't hook it up to anything useful.  That is done in the last patch
that actually adds the new governor.

That depends on two patches I sent previously, [1/6] that makes
cpufreq_update_util() use RCU-sched (one change from the previous version
as requested by Peter) and [2/6] that reworks acpi-cpufreq so the fast
switching (added later in patch [5/6]) can work with all of the frequency
setting methods the driver may use.

Comments welcome.

Thanks,
Rafael

[toc] | [next] | [standalone]


#1347700 — [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-03-02 03:30 +0100
Subject[PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r84I4-18B-75@gated-at.bofh.it>
In reply to#1347699
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Add a new cpufreq scaling governor, called "schedutil", that uses
scheduler-provided CPU utilization information as input for making
its decisions.

Doing that is possible after commit fe7034338ba0 (cpufreq: Add
mechanism for registering utilization update callbacks) that
introduced cpufreq_update_util() called by the scheduler on
utilization changes (from CFS) and RT/DL task status updates.
In particular, CPU frequency scaling decisions may be based on
the the utilization data passed to cpufreq_update_util() by CFS.

The new governor is relatively simple.

The frequency selection formula used by it is essentially the same
as the one used by the "ondemand" governor, although it doesn't use
the additional up_threshold parameter, but instead of computing the
load as the "non-idle CPU time" to "total CPU time" ratio, it takes
the utilization data provided by CFS as input.  More specifically,
it represents "load" as the util/max ratio, where util and max
are the utilization and CPU capacity coming from CFS.

All of the computations are carried out in the utilization update
handlers provided by the new governor.  One of those handlers is
used for cpufreq policies shared between multiple CPUs and the other
one is for policies with one CPU only (and therefore it doesn't need
to use any extra synchronization means).

The governor supports fast frequency switching if that is supported
by the cpufreq driver in use and possible for the given policy.
In the fast switching case, all operations of the governor take
place in its utilization update handlers.  If fast switching cannot
be used, the frequency switch operations are carried out with the
help of a work item which only calls __cpufreq_driver_target()
(under a mutex) to trigger a frequency update (to a value already
computed beforehand in one of the utilization update handlers).

Currently, the governor treats all of the RT and DL tasks as
"unknown utilization" and sets the frequency to the allowed
maximum when updated from the RT or DL sched classes.  That
heavy-handed approach should be replaced with something more
subtle and specifically targeted at RT and DL tasks.

The governor shares some tunables management code with the
"ondemand" and "conservative" governors and uses some common
definitions from cpufreq_governor.h, but apart from that it
is stand-alone.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

In addition to the changes mentioned in the intro message [0/6] this also
tweaks the frequency selection formula in a couple of ways.

First off, it uses min and max frequencies correctly (the formula from
"ondemand" is applied to cpuinfo.min/max_freq like the original and
policy->min/max are applied to the result later).

Second, RELATION_L is used most of the time except for the bottom 1/4
of the available frequency range (but also note that DL tasks are
treated in the same way as RT ones, meaning f_max is always used for
them).

Finally, the condition for discarding idle policy CPUs was modified
to also work if the rate limit is below the scheduling rate.

The code in sugov_init/exit/stop() and the irq_work handler look
very similar to the analogous code in cpufreq_governor.c, but it
is different enough that trying to avoid that duplication was not
practical.

Thanks,
Rafael

---
 drivers/cpufreq/Kconfig             |   26 +
 drivers/cpufreq/Makefile            |    1 
 drivers/cpufreq/cpufreq_schedutil.c |  501 ++++++++++++++++++++++++++++++++++++
 3 files changed, 528 insertions(+)

Index: linux-pm/drivers/cpufreq/cpufreq_schedutil.c
===================================================================
--- /dev/null
+++ linux-pm/drivers/cpufreq/cpufreq_schedutil.c
@@ -0,0 +1,501 @@
+/*
+ * CPUFreq governor based on scheduler-provided CPU utilization data.
+ *
+ * Copyright (C) 2016, Intel Corporation
+ * Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/percpu-defs.h>
+#include <linux/slab.h>
+
+#include "cpufreq_governor.h"
+
+struct sugov_tunables {
+	struct gov_tunables gt;
+	unsigned int rate_limit_us;
+};
+
+struct sugov_policy {
+	struct cpufreq_policy *policy;
+
+	struct sugov_tunables *tunables;
+	struct list_head tunables_hook;
+
+	raw_spinlock_t update_lock;  /* For shared policies */
+	u64 last_freq_update_time;
+	s64 freq_update_delay_ns;
+	unsigned int next_freq;
+
+	/* The next fields are only needed if fast switch cannot be used. */
+	unsigned int relation;
+	struct irq_work irq_work;
+	struct work_struct work;
+	struct mutex work_lock;
+	bool work_in_progress;
+
+	bool need_freq_update;
+};
+
+struct sugov_cpu {
+	struct update_util_data update_util;
+	struct sugov_policy *sg_policy;
+
+	/* The fields below are only needed when sharing a policy. */
+	unsigned long util;
+	unsigned long max;
+	u64 last_update;
+};
+
+static DEFINE_PER_CPU(struct sugov_cpu, sugov_cpu);
+
+/************************ Governor internals ***********************/
+
+static bool sugov_should_update_freq(struct sugov_policy *sg_policy, u64 time)
+{
+	u64 delta_ns;
+
+	if (sg_policy->work_in_progress)
+		return false;
+
+	if (unlikely(sg_policy->need_freq_update)) {
+		sg_policy->need_freq_update = false;
+		return true;
+	}
+
+	delta_ns = time - sg_policy->last_freq_update_time;
+	return (s64)delta_ns >= sg_policy->freq_update_delay_ns;
+}
+
+static void sugov_update_commit(struct sugov_policy *sg_policy, u64 time,
+				unsigned long util, unsigned long max,
+				unsigned int next_freq)
+{
+	struct cpufreq_policy *policy = sg_policy->policy;
+	unsigned int rel;
+
+	if (next_freq > policy->max)
+		next_freq = policy->max;
+	else if (next_freq < policy->min)
+		next_freq = policy->min;
+
+	sg_policy->last_freq_update_time = time;
+	if (sg_policy->next_freq == next_freq)
+		return;
+
+	sg_policy->next_freq = next_freq;
+	/*
+	 * If utilization is less than max / 4, use RELATION_C to allow the
+	 * minimum frequency to be selected more often in case the distance from
+	 * it to the next available frequency in the table is significant.
+	 */
+	rel = util < (max >> 2) ? CPUFREQ_RELATION_C : CPUFREQ_RELATION_L;
+	if (policy->fast_switch_possible) {
+		cpufreq_driver_fast_switch(policy, next_freq, rel);
+	} else {
+		sg_policy->relation = rel;
+		sg_policy->work_in_progress = true;
+		irq_work_queue(&sg_policy->irq_work);
+	}
+}
+
+static void sugov_update_single(struct update_util_data *data, u64 time,
+				unsigned long util, unsigned long max)
+{
+	struct sugov_cpu *sg_cpu = container_of(data, struct sugov_cpu, update_util);
+	struct sugov_policy *sg_policy = sg_cpu->sg_policy;
+	unsigned int min_f, max_f, next_f;
+
+	if (!sugov_should_update_freq(sg_policy, time))
+		return;
+
+	min_f = sg_policy->policy->cpuinfo.min_freq;
+	max_f = sg_policy->policy->cpuinfo.max_freq;
+	next_f = util > max ? max_f : min_f + util * (max_f - min_f) / max;
+
+	sugov_update_commit(sg_policy, time, util, max, next_f);
+}
+
+static unsigned int sugov_next_freq(struct sugov_policy *sg_policy,
+				    unsigned long util, unsigned long max)
+{
+	struct cpufreq_policy *policy = sg_policy->policy;
+	unsigned int min_f = policy->cpuinfo.min_freq;
+	unsigned int max_f = policy->cpuinfo.max_freq;
+	u64 last_freq_update_time = sg_policy->last_freq_update_time;
+	unsigned int j;
+
+	if (util > max)
+		return max_f;
+
+	for_each_cpu(j, policy->cpus) {
+		struct sugov_cpu *j_sg_cpu;
+		unsigned long j_util, j_max;
+		u64 delta_ns;
+
+		if (j == smp_processor_id())
+			continue;
+
+		j_sg_cpu = &per_cpu(sugov_cpu, j);
+		/*
+		 * If the CPU utilization was last updated before the previous
+		 * frequency update and the time elapsed between the last update
+		 * of the CPU utilization and the last frequency update is long
+		 * enough, don't take the CPU into account as it probably is
+		 * idle now.
+		 */
+		delta_ns = last_freq_update_time - j_sg_cpu->last_update;
+		if ((s64)delta_ns > NSEC_PER_SEC / HZ)
+			continue;
+
+		j_util = j_sg_cpu->util;
+		j_max = j_sg_cpu->max;
+		if (j_util > j_max)
+			return max_f;
+
+		if (j_util * max > j_max * util) {
+			util = j_util;
+			max = j_max;
+		}
+	}
+
+	return min_f + util * (max_f - min_f) / max;
+}
+
+static void sugov_update_shared(struct update_util_data *data, u64 time,
+				unsigned long util, unsigned long max)
+{
+	struct sugov_cpu *sg_cpu = container_of(data, struct sugov_cpu, update_util);
+	struct sugov_policy *sg_policy = sg_cpu->sg_policy;
+	unsigned int next_f;
+
+	raw_spin_lock(&sg_policy->update_lock);
+
+	sg_cpu->util = util;
+	sg_cpu->max = max;
+	sg_cpu->last_update = time;
+
+	if (sugov_should_update_freq(sg_policy, time)) {
+		next_f = sugov_next_freq(sg_policy, util, max);
+		sugov_update_commit(sg_policy, time, util, max, next_f);
+	}
+
+	raw_spin_unlock(&sg_policy->update_lock);
+}
+
+static void sugov_work(struct work_struct *work)
+{
+	struct sugov_policy *sg_policy = container_of(work, struct sugov_policy, work);
+
+	mutex_lock(&sg_policy->work_lock);
+	__cpufreq_driver_target(sg_policy->policy, sg_policy->next_freq,
+				sg_policy->relation);
+	mutex_unlock(&sg_policy->work_lock);
+
+	sg_policy->work_in_progress = false;
+}
+
+static void sugov_irq_work(struct irq_work *irq_work)
+{
+	struct sugov_policy *sg_policy;
+
+	sg_policy = container_of(irq_work, struct sugov_policy, irq_work);
+	schedule_work(&sg_policy->work);
+}
+
+/************************** sysfs interface ************************/
+
+static struct sugov_tunables *global_tunables;
+static DEFINE_MUTEX(global_tunables_lock);
+
+static inline struct sugov_tunables *to_sugov_tunables(struct gov_tunables *gt)
+{
+	return container_of(gt, struct sugov_tunables, gt);
+}
+
+static ssize_t rate_limit_us_show(struct gov_tunables *gt, char *buf)
+{
+	struct sugov_tunables *tunables = to_sugov_tunables(gt);
+
+	return sprintf(buf, "%u\n", tunables->rate_limit_us);
+}
+
+static ssize_t rate_limit_us_store(struct gov_tunables *gt, const char *buf,
+				   size_t count)
+{
+	struct sugov_tunables *tunables = to_sugov_tunables(gt);
+	struct sugov_policy *sg_policy;
+	unsigned int rate_limit_us;
+	int ret;
+
+	ret = sscanf(buf, "%u", &rate_limit_us);
+	if (ret != 1)
+		return -EINVAL;
+
+	tunables->rate_limit_us = rate_limit_us;
+
+	list_for_each_entry(sg_policy, &gt->policy_list, tunables_hook)
+		sg_policy->freq_update_delay_ns = rate_limit_us * NSEC_PER_USEC;
+
+	return count;
+}
+
+static struct governor_attr rate_limit_us = __ATTR_RW(rate_limit_us);
+
+static struct attribute *sugov_attributes[] = {
+	&rate_limit_us.attr,
+	NULL
+};
+
+static struct kobj_type sugov_tunables_ktype = {
+	.default_attrs = sugov_attributes,
+	.sysfs_ops = &governor_sysfs_ops,
+};
+
+/********************** cpufreq governor interface *********************/
+
+static struct cpufreq_governor schedutil_gov;
+
+static struct sugov_policy *sugov_policy_alloc(struct cpufreq_policy *policy)
+{
+	struct sugov_policy *sg_policy;
+
+	sg_policy = kzalloc(sizeof(*sg_policy), GFP_KERNEL);
+	if (!sg_policy)
+		return NULL;
+
+	sg_policy->policy = policy;
+	init_irq_work(&sg_policy->irq_work, sugov_irq_work);
+	INIT_WORK(&sg_policy->work, sugov_work);
+	mutex_init(&sg_policy->work_lock);
+	raw_spin_lock_init(&sg_policy->update_lock);
+	return sg_policy;
+}
+
+static void sugov_policy_free(struct sugov_policy *sg_policy)
+{
+	mutex_destroy(&sg_policy->work_lock);
+	kfree(sg_policy);
+}
+
+static struct sugov_tunables *sugov_tunables_alloc(struct sugov_policy *sg_policy)
+{
+	struct sugov_tunables *tunables;
+
+	tunables = kzalloc(sizeof(*tunables), GFP_KERNEL);
+	if (tunables)
+		gov_tunables_init(&tunables->gt, &sg_policy->tunables_hook);
+
+	return tunables;
+}
+
+static void sugov_tunables_free(struct sugov_tunables *tunables)
+{
+	if (!have_governor_per_policy())
+		global_tunables = NULL;
+
+	kfree(tunables);
+}
+
+static int sugov_init(struct cpufreq_policy *policy)
+{
+	struct sugov_policy *sg_policy;
+	struct sugov_tunables *tunables;
+	unsigned int lat;
+	int ret = 0;
+
+	/* State should be equivalent to EXIT */
+	if (policy->governor_data)
+		return -EBUSY;
+
+	sg_policy = sugov_policy_alloc(policy);
+	if (!sg_policy)
+		return -ENOMEM;
+
+	mutex_lock(&global_tunables_lock);
+
+	if (global_tunables) {
+		if (WARN_ON(have_governor_per_policy())) {
+			ret = -EINVAL;
+			goto free_sg_policy;
+		}
+		policy->governor_data = sg_policy;
+		sg_policy->tunables = global_tunables;
+
+		gov_tunables_get(&global_tunables->gt, &sg_policy->tunables_hook);
+		goto out;
+	}
+
+	tunables = sugov_tunables_alloc(sg_policy);
+	if (!tunables) {
+		ret = -ENOMEM;
+		goto free_sg_policy;
+	}
+
+	tunables->rate_limit_us = LATENCY_MULTIPLIER;
+	lat = policy->cpuinfo.transition_latency / NSEC_PER_USEC;
+	if (lat)
+		tunables->rate_limit_us *= lat;
+
+	if (!have_governor_per_policy())
+		global_tunables = tunables;
+
+	policy->governor_data = sg_policy;
+	sg_policy->tunables = tunables;
+
+	ret = kobject_init_and_add(&tunables->gt.kobj, &sugov_tunables_ktype,
+				   get_governor_parent_kobj(policy), "%s",
+				   schedutil_gov.name);
+	if (!ret)
+		goto out;
+
+	/* Failure, so roll back. */
+	policy->governor_data = NULL;
+	sugov_tunables_free(tunables);
+
+ free_sg_policy:
+	pr_err("cpufreq: schedutil governor initialization failed (error %d)\n", ret);
+	sugov_policy_free(sg_policy);
+
+ out:
+	mutex_unlock(&global_tunables_lock);
+	return ret;
+}
+
+static int sugov_exit(struct cpufreq_policy *policy)
+{
+	struct sugov_policy *sg_policy = policy->governor_data;
+	struct sugov_tunables *tunables = sg_policy->tunables;
+	unsigned int count;
+
+	mutex_lock(&global_tunables_lock);
+
+	count = gov_tunables_put(&tunables->gt, &sg_policy->tunables_hook);
+	policy->governor_data = NULL;
+	if (!count)
+		sugov_tunables_free(tunables);
+
+	mutex_unlock(&global_tunables_lock);
+
+	sugov_policy_free(sg_policy);
+	return 0;
+}
+
+static int sugov_start(struct cpufreq_policy *policy)
+{
+	struct sugov_policy *sg_policy = policy->governor_data;
+	unsigned int cpu;
+
+	sg_policy->freq_update_delay_ns = sg_policy->tunables->rate_limit_us * NSEC_PER_USEC;
+	sg_policy->last_freq_update_time = 0;
+	sg_policy->next_freq = UINT_MAX;
+	sg_policy->work_in_progress = false;
+	sg_policy->need_freq_update = false;
+
+	for_each_cpu(cpu, policy->cpus) {
+		struct sugov_cpu *sg_cpu = &per_cpu(sugov_cpu, cpu);
+
+		sg_cpu->sg_policy = sg_policy;
+		if (policy_is_shared(policy)) {
+			sg_cpu->util = ULONG_MAX;
+			sg_cpu->max = 0;
+			sg_cpu->last_update = 0;
+			sg_cpu->update_util.func = sugov_update_shared;
+		} else {
+			sg_cpu->update_util.func = sugov_update_single;
+		}
+		cpufreq_set_update_util_data(cpu, &sg_cpu->update_util);
+	}
+	return 0;
+}
+
+static int sugov_stop(struct cpufreq_policy *policy)
+{
+	struct sugov_policy *sg_policy = policy->governor_data;
+	unsigned int cpu;
+
+	for_each_cpu(cpu, policy->cpus)
+		cpufreq_set_update_util_data(cpu, NULL);
+
+	synchronize_sched();
+
+	irq_work_sync(&sg_policy->irq_work);
+	cancel_work_sync(&sg_policy->work);
+	return 0;
+}
+
+static int sugov_limits(struct cpufreq_policy *policy)
+{
+	struct sugov_policy *sg_policy = policy->governor_data;
+
+	if (!policy->fast_switch_possible) {
+		mutex_lock(&sg_policy->work_lock);
+
+		if (policy->max < policy->cur)
+			__cpufreq_driver_target(policy, policy->max,
+						CPUFREQ_RELATION_H);
+		else if (policy->min > policy->cur)
+			__cpufreq_driver_target(policy, policy->min,
+						CPUFREQ_RELATION_L);
+
+		mutex_unlock(&sg_policy->work_lock);
+	}
+
+	sg_policy->need_freq_update = true;
+	return 0;
+}
+
+int sugov_governor(struct cpufreq_policy *policy, unsigned int event)
+{
+	if (event == CPUFREQ_GOV_POLICY_INIT) {
+		return sugov_init(policy);
+	} else if (policy->governor_data) {
+		switch (event) {
+		case CPUFREQ_GOV_POLICY_EXIT:
+			return sugov_exit(policy);
+		case CPUFREQ_GOV_START:
+			return sugov_start(policy);
+		case CPUFREQ_GOV_STOP:
+			return sugov_stop(policy);
+		case CPUFREQ_GOV_LIMITS:
+			return sugov_limits(policy);
+		}
+	}
+	return -EINVAL;
+}
+
+static struct cpufreq_governor schedutil_gov = {
+	.name = "schedutil",
+	.governor = sugov_governor,
+	.max_transition_latency	= TRANSITION_LATENCY_LIMIT,
+	.owner = THIS_MODULE,
+};
+
+static int __init sugov_module_init(void)
+{
+	return cpufreq_register_governor(&schedutil_gov);
+}
+
+static void __exit sugov_module_exit(void)
+{
+	cpufreq_unregister_governor(&schedutil_gov);
+}
+
+MODULE_AUTHOR("Rafael J. Wysocki <rafael.j.wysocki@intel.com>");
+MODULE_DESCRIPTION("Utilization-based CPU frequency selection");
+MODULE_LICENSE("GPL");
+
+#ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_SCHEDUTIL
+struct cpufreq_governor *cpufreq_default_governor(void)
+{
+	return &schedutil_gov;
+}
+
+fs_initcall(sugov_module_init);
+#else
+module_init(sugov_module_init);
+#endif
+module_exit(sugov_module_exit);
Index: linux-pm/drivers/cpufreq/Kconfig
===================================================================
--- linux-pm.orig/drivers/cpufreq/Kconfig
+++ linux-pm/drivers/cpufreq/Kconfig
@@ -107,6 +107,16 @@ config CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
 	  Be aware that not all cpufreq drivers support the conservative
 	  governor. If unsure have a look at the help section of the
 	  driver. Fallback governor will be the performance governor.
+
+config CPU_FREQ_DEFAULT_GOV_SCHEDUTIL
+	bool "schedutil"
+	select CPU_FREQ_GOV_SCHEDUTIL
+	select CPU_FREQ_GOV_PERFORMANCE
+	help
+	  Use the 'schedutil' CPUFreq governor by default. If unsure,
+	  have a look at the help section of that governor. The fallback
+	  governor will be 'performance'.
+
 endchoice
 
 config CPU_FREQ_GOV_PERFORMANCE
@@ -188,6 +198,22 @@ config CPU_FREQ_GOV_CONSERVATIVE
 
 	  If in doubt, say N.
 
+config CPU_FREQ_GOV_SCHEDUTIL
+	tristate "'schedutil' cpufreq policy governor"
+	depends on CPU_FREQ
+	select CPU_FREQ_GOV_TUNABLES
+	select IRQ_WORK
+	help
+	  The frequency selection formula used by this governor is analogous
+	  to the one used by 'ondemand', but instead of computing CPU load
+	  as the "non-idle CPU time" to "total CPU time" ratio, it uses CPU
+	  utilization data provided by the scheduler as input.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cpufreq_schedutil.
+
+	  If in doubt, say N.
+
 comment "CPU frequency scaling drivers"
 
 config CPUFREQ_DT
Index: linux-pm/drivers/cpufreq/Makefile
===================================================================
--- linux-pm.orig/drivers/cpufreq/Makefile
+++ linux-pm/drivers/cpufreq/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_CPU_FREQ_GOV_POWERSAVE)	+=
 obj-$(CONFIG_CPU_FREQ_GOV_USERSPACE)	+= cpufreq_userspace.o
 obj-$(CONFIG_CPU_FREQ_GOV_ONDEMAND)	+= cpufreq_ondemand.o
 obj-$(CONFIG_CPU_FREQ_GOV_CONSERVATIVE)	+= cpufreq_conservative.o
+obj-$(CONFIG_CPU_FREQ_GOV_SCHEDUTIL)	+= cpufreq_schedutil.o
 obj-$(CONFIG_CPU_FREQ_GOV_COMMON)		+= cpufreq_governor.o
 obj-$(CONFIG_CPU_FREQ_GOV_TUNABLES)	+= cpufreq_governor_tunables.o
 

[toc] | [prev] | [next] | [standalone]


#1348342 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromVincent Guittot <vincent.guittot@linaro.org>
Date2016-03-02 18:20 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8iBj-2eO-11@gated-at.bofh.it>
In reply to#1347700
Hi Rafael,


On 2 March 2016 at 03:27, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Add a new cpufreq scaling governor, called "schedutil", that uses
> scheduler-provided CPU utilization information as input for making
> its decisions.
>
> Doing that is possible after commit fe7034338ba0 (cpufreq: Add
> mechanism for registering utilization update callbacks) that
> introduced cpufreq_update_util() called by the scheduler on
> utilization changes (from CFS) and RT/DL task status updates.
> In particular, CPU frequency scaling decisions may be based on
> the the utilization data passed to cpufreq_update_util() by CFS.
>
> The new governor is relatively simple.
>
> The frequency selection formula used by it is essentially the same
> as the one used by the "ondemand" governor, although it doesn't use
> the additional up_threshold parameter, but instead of computing the
> load as the "non-idle CPU time" to "total CPU time" ratio, it takes
> the utilization data provided by CFS as input.  More specifically,
> it represents "load" as the util/max ratio, where util and max
> are the utilization and CPU capacity coming from CFS.
>

[snip]

> +
> +static void sugov_update_commit(struct sugov_policy *sg_policy, u64 time,
> +                               unsigned long util, unsigned long max,
> +                               unsigned int next_freq)
> +{
> +       struct cpufreq_policy *policy = sg_policy->policy;
> +       unsigned int rel;
> +
> +       if (next_freq > policy->max)
> +               next_freq = policy->max;
> +       else if (next_freq < policy->min)
> +               next_freq = policy->min;
> +
> +       sg_policy->last_freq_update_time = time;
> +       if (sg_policy->next_freq == next_freq)
> +               return;
> +
> +       sg_policy->next_freq = next_freq;
> +       /*
> +        * If utilization is less than max / 4, use RELATION_C to allow the
> +        * minimum frequency to be selected more often in case the distance from
> +        * it to the next available frequency in the table is significant.
> +        */
> +       rel = util < (max >> 2) ? CPUFREQ_RELATION_C : CPUFREQ_RELATION_L;
> +       if (policy->fast_switch_possible) {
> +               cpufreq_driver_fast_switch(policy, next_freq, rel);
> +       } else {
> +               sg_policy->relation = rel;
> +               sg_policy->work_in_progress = true;
> +               irq_work_queue(&sg_policy->irq_work);
> +       }
> +}
> +
> +static void sugov_update_single(struct update_util_data *data, u64 time,
> +                               unsigned long util, unsigned long max)
> +{
> +       struct sugov_cpu *sg_cpu = container_of(data, struct sugov_cpu, update_util);
> +       struct sugov_policy *sg_policy = sg_cpu->sg_policy;
> +       unsigned int min_f, max_f, next_f;
> +
> +       if (!sugov_should_update_freq(sg_policy, time))
> +               return;
> +
> +       min_f = sg_policy->policy->cpuinfo.min_freq;
> +       max_f = sg_policy->policy->cpuinfo.max_freq;
> +       next_f = util > max ? max_f : min_f + util * (max_f - min_f) / max;

I think it has been pointed out in another email's thread but you
should change the way the next_f is computed. util reflects the
utilization of a CPU from 0 to its max compute capacity whereas
ondemand was using the load at the current frequency during the last
time window. I have understood that you want to keep same formula than
ondemand as a starting point but you use a different input to
calculate the next frequency so i don't see the rational of keeping
this formula. Saying that, even the simple formula next_f = util > max
? max_f : util * (max_f) / max will not work properly if the frequency
invariance is enable because the utilization becomes capped by the
current compute capacity so next_f will never be higher than current
freq (unless a task move on the rq).  That was one reason of using a
threshold in sched-freq proposal (and there are on going dev to try to
solve this limitation).
IIIUC, frequency invariance is not enable on your platform so you have
not seen the problem but you have probably see that selection of your
next_f was not really stable. Without frequency invariance, the
utilization will be overestimated when running at lower frequency so
the governor will probably select a frequency that is higher than
necessary but then the utilization will decrease at this higher
frequency so the governor will probably decrease the frequency and so
on until you found the right frequency that will generate the right
utilisation value

Regards,
Vincent

> +
> +       sugov_update_commit(sg_policy, time, util, max, next_f);
> +}
> +
> +static unsigned int sugov_next_freq(struct sugov_policy *sg_policy,
> +                                   unsigned long util, unsigned long max)
> +{
> +       struct cpufreq_policy *policy = sg_policy->policy;
> +       unsigned int min_f = policy->cpuinfo.min_freq;
> +       unsigned int max_f = policy->cpuinfo.max_freq;
> +       u64 last_freq_update_time = sg_policy->last_freq_update_time;
> +       unsigned int j;
> +
> +       if (util > max)
> +               return max_f;
> +
> +       for_each_cpu(j, policy->cpus) {
> +               struct sugov_cpu *j_sg_cpu;
> +               unsigned long j_util, j_max;
> +               u64 delta_ns;
> +
> +               if (j == smp_processor_id())
> +                       continue;
> +
> +               j_sg_cpu = &per_cpu(sugov_cpu, j);
> +               /*
> +                * If the CPU utilization was last updated before the previous
> +                * frequency update and the time elapsed between the last update
> +                * of the CPU utilization and the last frequency update is long
> +                * enough, don't take the CPU into account as it probably is
> +                * idle now.
> +                */
> +               delta_ns = last_freq_update_time - j_sg_cpu->last_update;
> +               if ((s64)delta_ns > NSEC_PER_SEC / HZ)
> +                       continue;
> +
> +               j_util = j_sg_cpu->util;
> +               j_max = j_sg_cpu->max;
> +               if (j_util > j_max)
> +                       return max_f;
> +
> +               if (j_util * max > j_max * util) {
> +                       util = j_util;
> +                       max = j_max;
> +               }
> +       }
> +
> +       return min_f + util * (max_f - min_f) / max;
> +}
> +

[snip]

[toc] | [prev] | [next] | [standalone]


#1348366 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2016-03-02 19:00 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8je3-2XE-17@gated-at.bofh.it>
In reply to#1348342
On Wed, Mar 2, 2016 at 6:10 PM, Vincent Guittot
<vincent.guittot@linaro.org> wrote:
> Hi Rafael,
>
>
> On 2 March 2016 at 03:27, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> Add a new cpufreq scaling governor, called "schedutil", that uses
>> scheduler-provided CPU utilization information as input for making
>> its decisions.
>>
>> Doing that is possible after commit fe7034338ba0 (cpufreq: Add
>> mechanism for registering utilization update callbacks) that
>> introduced cpufreq_update_util() called by the scheduler on
>> utilization changes (from CFS) and RT/DL task status updates.
>> In particular, CPU frequency scaling decisions may be based on
>> the the utilization data passed to cpufreq_update_util() by CFS.
>>
>> The new governor is relatively simple.
>>
>> The frequency selection formula used by it is essentially the same
>> as the one used by the "ondemand" governor, although it doesn't use
>> the additional up_threshold parameter, but instead of computing the
>> load as the "non-idle CPU time" to "total CPU time" ratio, it takes
>> the utilization data provided by CFS as input.  More specifically,
>> it represents "load" as the util/max ratio, where util and max
>> are the utilization and CPU capacity coming from CFS.
>>
>
> [snip]
>
>> +
>> +static void sugov_update_commit(struct sugov_policy *sg_policy, u64 time,
>> +                               unsigned long util, unsigned long max,
>> +                               unsigned int next_freq)
>> +{
>> +       struct cpufreq_policy *policy = sg_policy->policy;
>> +       unsigned int rel;
>> +
>> +       if (next_freq > policy->max)
>> +               next_freq = policy->max;
>> +       else if (next_freq < policy->min)
>> +               next_freq = policy->min;
>> +
>> +       sg_policy->last_freq_update_time = time;
>> +       if (sg_policy->next_freq == next_freq)
>> +               return;
>> +
>> +       sg_policy->next_freq = next_freq;
>> +       /*
>> +        * If utilization is less than max / 4, use RELATION_C to allow the
>> +        * minimum frequency to be selected more often in case the distance from
>> +        * it to the next available frequency in the table is significant.
>> +        */
>> +       rel = util < (max >> 2) ? CPUFREQ_RELATION_C : CPUFREQ_RELATION_L;
>> +       if (policy->fast_switch_possible) {
>> +               cpufreq_driver_fast_switch(policy, next_freq, rel);
>> +       } else {
>> +               sg_policy->relation = rel;
>> +               sg_policy->work_in_progress = true;
>> +               irq_work_queue(&sg_policy->irq_work);
>> +       }
>> +}
>> +
>> +static void sugov_update_single(struct update_util_data *data, u64 time,
>> +                               unsigned long util, unsigned long max)
>> +{
>> +       struct sugov_cpu *sg_cpu = container_of(data, struct sugov_cpu, update_util);
>> +       struct sugov_policy *sg_policy = sg_cpu->sg_policy;
>> +       unsigned int min_f, max_f, next_f;
>> +
>> +       if (!sugov_should_update_freq(sg_policy, time))
>> +               return;
>> +
>> +       min_f = sg_policy->policy->cpuinfo.min_freq;
>> +       max_f = sg_policy->policy->cpuinfo.max_freq;
>> +       next_f = util > max ? max_f : min_f + util * (max_f - min_f) / max;
>
> I think it has been pointed out in another email's thread but you
> should change the way the next_f is computed. util reflects the
> utilization of a CPU from 0 to its max compute capacity whereas
> ondemand was using the load at the current frequency during the last
> time window. I have understood that you want to keep same formula than
> ondemand as a starting point but you use a different input to
> calculate the next frequency so i don't see the rational of keeping
> this formula.

It is a formula that causes the entire available frequency range to be
utilized proportionally to the utilization as reported by the
scheduler (modulo the policy->min/max limits).  Its (significant IMO)
advantage is that it doesn't require any additional factors that would
need to be determined somehow.

> Saying that, even the simple formula next_f = util > max
> ? max_f : util * (max_f) / max will not work properly if the frequency
> invariance is enable because the utilization becomes capped by the
> current compute capacity so next_f will never be higher than current
> freq (unless a task move on the rq).  That was one reason of using a
> threshold in sched-freq proposal (and there are on going dev to try to
> solve this limitation).

Well, a different formula will have to be used along with frequency
invariance, then.

> IIIUC, frequency invariance is not enable on your platform so you have
> not seen the problem but you have probably see that selection of your
> next_f was not really stable. Without frequency invariance, the
> utilization will be overestimated when running at lower frequency so
> the governor will probably select a frequency that is higher than
> necessary but then the utilization will decrease at this higher
> frequency so the governor will probably decrease the frequency and so
> on until you found the right frequency that will generate the right
> utilisation value

I don't have any problems with that to be honest and if you aim at
selecting the perfect frequency at the first attempt, then good luck
with that anyway.

Now, I'm not saying that the formula used in this patch cannot be
improved or similar.  It very well may be possible to improve it.  I'm
only saying that it is good enough to start with, because of the
reasons mentioned above.

Still, if you can suggest to me what other formula specifically should
be used here, I'll consider using it.  Which will probably mean
comparing the two and seeing which one leads to better results.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]


#1348583 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2016-03-02 23:50 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8nKG-6bw-5@gated-at.bofh.it>
In reply to#1348366
On Wed, Mar 2, 2016 at 6:58 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Wed, Mar 2, 2016 at 6:10 PM, Vincent Guittot
> <vincent.guittot@linaro.org> wrote:
>> Hi Rafael,
>>
>>
>> On 2 March 2016 at 03:27, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>
>>> Add a new cpufreq scaling governor, called "schedutil", that uses
>>> scheduler-provided CPU utilization information as input for making
>>> its decisions.
>>>
>>> Doing that is possible after commit fe7034338ba0 (cpufreq: Add
>>> mechanism for registering utilization update callbacks) that
>>> introduced cpufreq_update_util() called by the scheduler on
>>> utilization changes (from CFS) and RT/DL task status updates.
>>> In particular, CPU frequency scaling decisions may be based on
>>> the the utilization data passed to cpufreq_update_util() by CFS.
>>>
>>> The new governor is relatively simple.
>>>
>>> The frequency selection formula used by it is essentially the same
>>> as the one used by the "ondemand" governor, although it doesn't use
>>> the additional up_threshold parameter, but instead of computing the
>>> load as the "non-idle CPU time" to "total CPU time" ratio, it takes
>>> the utilization data provided by CFS as input.  More specifically,
>>> it represents "load" as the util/max ratio, where util and max
>>> are the utilization and CPU capacity coming from CFS.
>>>
>>
>> [snip]
>>
>>> +
>>> +static void sugov_update_commit(struct sugov_policy *sg_policy, u64 time,
>>> +                               unsigned long util, unsigned long max,
>>> +                               unsigned int next_freq)
>>> +{
>>> +       struct cpufreq_policy *policy = sg_policy->policy;
>>> +       unsigned int rel;
>>> +
>>> +       if (next_freq > policy->max)
>>> +               next_freq = policy->max;
>>> +       else if (next_freq < policy->min)
>>> +               next_freq = policy->min;
>>> +
>>> +       sg_policy->last_freq_update_time = time;
>>> +       if (sg_policy->next_freq == next_freq)
>>> +               return;
>>> +
>>> +       sg_policy->next_freq = next_freq;
>>> +       /*
>>> +        * If utilization is less than max / 4, use RELATION_C to allow the
>>> +        * minimum frequency to be selected more often in case the distance from
>>> +        * it to the next available frequency in the table is significant.
>>> +        */
>>> +       rel = util < (max >> 2) ? CPUFREQ_RELATION_C : CPUFREQ_RELATION_L;
>>> +       if (policy->fast_switch_possible) {
>>> +               cpufreq_driver_fast_switch(policy, next_freq, rel);
>>> +       } else {
>>> +               sg_policy->relation = rel;
>>> +               sg_policy->work_in_progress = true;
>>> +               irq_work_queue(&sg_policy->irq_work);
>>> +       }
>>> +}
>>> +
>>> +static void sugov_update_single(struct update_util_data *data, u64 time,
>>> +                               unsigned long util, unsigned long max)
>>> +{
>>> +       struct sugov_cpu *sg_cpu = container_of(data, struct sugov_cpu, update_util);
>>> +       struct sugov_policy *sg_policy = sg_cpu->sg_policy;
>>> +       unsigned int min_f, max_f, next_f;
>>> +
>>> +       if (!sugov_should_update_freq(sg_policy, time))
>>> +               return;
>>> +
>>> +       min_f = sg_policy->policy->cpuinfo.min_freq;
>>> +       max_f = sg_policy->policy->cpuinfo.max_freq;
>>> +       next_f = util > max ? max_f : min_f + util * (max_f - min_f) / max;
>>
>> I think it has been pointed out in another email's thread but you
>> should change the way the next_f is computed. util reflects the
>> utilization of a CPU from 0 to its max compute capacity whereas
>> ondemand was using the load at the current frequency during the last
>> time window. I have understood that you want to keep same formula than
>> ondemand as a starting point but you use a different input to
>> calculate the next frequency so i don't see the rational of keeping
>> this formula.
>
> It is a formula that causes the entire available frequency range to be
> utilized proportionally to the utilization as reported by the
> scheduler (modulo the policy->min/max limits).  Its (significant IMO)
> advantage is that it doesn't require any additional factors that would
> need to be determined somehow.

In case a more formal derivation of this formula is needed, it is
based on the following 3 assumptions:

(1) Performance is a linear function of frequency.
(2) Required performance is a linear function of the utilization ratio
x = util/max as provided by the scheduler (0 <= x <= 1).
(3) The minimum possible frequency (min_freq) corresponds to x = 0 and
the maximum possible frequency (max_freq) corresponds to x = 1.

(1) and (2) combined imply that

f = a * x + b

(f - frequency, a, b - constants to be determined) and then (3) quite
trivially leads to b = min_freq and a = max_freq - min_freq.

Now, of course, the linearity assumptions may be questioned, but then
it's just the first approximation.  If you go any further, though, you
end up with an expansion series like this:

f(x) = c_0 + c_1 * x + c_2 * x^2 + c_3 * x^3 + ...

where all of the c_j need to be determined in principle.  With luck,
if you can guess what kind of a function f(x) may be, it may be
possible to reduce the number of coefficients to determine, but
question is whether or not that is going to work universally for all
systems.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]


#1349094 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromPeter Zijlstra <peterz@infradead.org>
Date2016-03-03 13:30 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8Aye-745-19@gated-at.bofh.it>
In reply to#1348583
On Wed, Mar 02, 2016 at 11:49:48PM +0100, Rafael J. Wysocki wrote:
> >>> +       min_f = sg_policy->policy->cpuinfo.min_freq;
> >>> +       max_f = sg_policy->policy->cpuinfo.max_freq;
> >>> +       next_f = util > max ? max_f : min_f + util * (max_f - min_f) / max;

> In case a more formal derivation of this formula is needed, it is
> based on the following 3 assumptions:
> 
> (1) Performance is a linear function of frequency.
> (2) Required performance is a linear function of the utilization ratio
> x = util/max as provided by the scheduler (0 <= x <= 1).

> (3) The minimum possible frequency (min_freq) corresponds to x = 0 and
> the maximum possible frequency (max_freq) corresponds to x = 1.
> 
> (1) and (2) combined imply that
> 
> f = a * x + b
> 
> (f - frequency, a, b - constants to be determined) and then (3) quite
> trivially leads to b = min_freq and a = max_freq - min_freq.

3 is the problem, that just doesn't make sense and is probably the
reason why you see very little selection of the min freq.

Suppose a machine with the following frequencies:

	500, 750, 1000

And a utilization of 0.4, how does asking for 500 + 0.4 * (1000-500) =
700 make any sense? Per your point 1, it should should be asking for
0.4 * 1000 = 400.

Because, per 1, at 500 it runs exactly half as fast as at 1000, and we
only need 0.4 times as much. Therefore 500 is more than sufficient.



Note. we all know that 1 is a 'broken' assumption, but lacking anything
better I think its a reasonable one to make.

[toc] | [prev] | [next] | [standalone]


#1349111 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromJuri Lelli <juri.lelli@arm.com>
Date2016-03-03 13:40 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8AHV-77v-41@gated-at.bofh.it>
In reply to#1349094
On 03/03/16 13:20, Peter Zijlstra wrote:
> On Wed, Mar 02, 2016 at 11:49:48PM +0100, Rafael J. Wysocki wrote:
> > >>> +       min_f = sg_policy->policy->cpuinfo.min_freq;
> > >>> +       max_f = sg_policy->policy->cpuinfo.max_freq;
> > >>> +       next_f = util > max ? max_f : min_f + util * (max_f - min_f) / max;
> 
> > In case a more formal derivation of this formula is needed, it is
> > based on the following 3 assumptions:
> > 
> > (1) Performance is a linear function of frequency.
> > (2) Required performance is a linear function of the utilization ratio
> > x = util/max as provided by the scheduler (0 <= x <= 1).
> 
> > (3) The minimum possible frequency (min_freq) corresponds to x = 0 and
> > the maximum possible frequency (max_freq) corresponds to x = 1.
> > 
> > (1) and (2) combined imply that
> > 
> > f = a * x + b
> > 
> > (f - frequency, a, b - constants to be determined) and then (3) quite
> > trivially leads to b = min_freq and a = max_freq - min_freq.
> 
> 3 is the problem, that just doesn't make sense and is probably the
> reason why you see very little selection of the min freq.
> 
> Suppose a machine with the following frequencies:
> 
> 	500, 750, 1000
> 
> And a utilization of 0.4, how does asking for 500 + 0.4 * (1000-500) =
> 700 make any sense? Per your point 1, it should should be asking for
> 0.4 * 1000 = 400.
> 
> Because, per 1, at 500 it runs exactly half as fast as at 1000, and we
> only need 0.4 times as much. Therefore 500 is more than sufficient.
> 

Oh, and that is probably also why the governor can reach max OPP with
freq invariance enabled (the point Vincent was making). When we run at
500 the util signal is capped at that capacity, but the formula makes us
requesting more, so we can jump to the next step and so on.

[toc] | [prev] | [next] | [standalone]


#1349342 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2016-03-03 17:30 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8Eiw-1kl-35@gated-at.bofh.it>
In reply to#1349094
On Thu, Mar 3, 2016 at 1:20 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Mar 02, 2016 at 11:49:48PM +0100, Rafael J. Wysocki wrote:
>> >>> +       min_f = sg_policy->policy->cpuinfo.min_freq;
>> >>> +       max_f = sg_policy->policy->cpuinfo.max_freq;
>> >>> +       next_f = util > max ? max_f : min_f + util * (max_f - min_f) / max;
>
>> In case a more formal derivation of this formula is needed, it is
>> based on the following 3 assumptions:
>>
>> (1) Performance is a linear function of frequency.
>> (2) Required performance is a linear function of the utilization ratio
>> x = util/max as provided by the scheduler (0 <= x <= 1).
>
>> (3) The minimum possible frequency (min_freq) corresponds to x = 0 and
>> the maximum possible frequency (max_freq) corresponds to x = 1.
>>
>> (1) and (2) combined imply that
>>
>> f = a * x + b
>>
>> (f - frequency, a, b - constants to be determined) and then (3) quite
>> trivially leads to b = min_freq and a = max_freq - min_freq.
>
> 3 is the problem, that just doesn't make sense and is probably the
> reason why you see very little selection of the min freq.

It is about mapping the entire [0,1] interval to the available frequency range.

I till overprovision things (the smaller x the more), but then it may
help the race-to-idle a bit in theory.

> Suppose a machine with the following frequencies:
>
>         500, 750, 1000
>
> And a utilization of 0.4, how does asking for 500 + 0.4 * (1000-500) =
> 700 make any sense? Per your point 1, it should should be asking for
> 0.4 * 1000 = 400.
>
> Because, per 1, at 500 it runs exactly half as fast as at 1000, and we
> only need 0.4 times as much. Therefore 500 is more than sufficient.

OK, but then I don't see why this reasoning only applies to the lower
bound of the frequency range.  Is there any reason why x = 1 should be
the only point mapping to max_freq?

If not, then I think it's reasonable to map the middle of the
available frequency range to x = 0.5 and then we have b = 0 and a =
(max_freq + min_freq) / 2.

I'll try that and see how it goes.

> Note. we all know that 1 is a 'broken' assumption, but lacking anything
> better I think its a reasonable one to make.

Right.

[toc] | [prev] | [next] | [standalone]


#1349345 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromPeter Zijlstra <peterz@infradead.org>
Date2016-03-03 17:40 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8Es9-1q6-1@gated-at.bofh.it>
In reply to#1349342
On Thu, Mar 03, 2016 at 05:24:32PM +0100, Rafael J. Wysocki wrote:
> On Thu, Mar 3, 2016 at 1:20 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Wed, Mar 02, 2016 at 11:49:48PM +0100, Rafael J. Wysocki wrote:
> >> >>> +       min_f = sg_policy->policy->cpuinfo.min_freq;
> >> >>> +       max_f = sg_policy->policy->cpuinfo.max_freq;
> >> >>> +       next_f = util > max ? max_f : min_f + util * (max_f - min_f) / max;
> >
> >> In case a more formal derivation of this formula is needed, it is
> >> based on the following 3 assumptions:
> >>
> >> (1) Performance is a linear function of frequency.
> >> (2) Required performance is a linear function of the utilization ratio
> >> x = util/max as provided by the scheduler (0 <= x <= 1).
> >
> >> (3) The minimum possible frequency (min_freq) corresponds to x = 0 and
> >> the maximum possible frequency (max_freq) corresponds to x = 1.
> >>
> >> (1) and (2) combined imply that
> >>
> >> f = a * x + b
> >>
> >> (f - frequency, a, b - constants to be determined) and then (3) quite
> >> trivially leads to b = min_freq and a = max_freq - min_freq.
> >
> > 3 is the problem, that just doesn't make sense and is probably the
> > reason why you see very little selection of the min freq.
> 
> It is about mapping the entire [0,1] interval to the available frequency range.

Yeah, but I don't see why that makes sense..

> I till overprovision things (the smaller x the more), but then it may
> help the race-to-idle a bit in theory.

So, since we also have the cpuidle information, could we not make a
better guess at race-to-idle?

> > Suppose a machine with the following frequencies:
> >
> >         500, 750, 1000
> >
> > And a utilization of 0.4, how does asking for 500 + 0.4 * (1000-500) =
> > 700 make any sense? Per your point 1, it should should be asking for
> > 0.4 * 1000 = 400.
> >
> > Because, per 1, at 500 it runs exactly half as fast as at 1000, and we
> > only need 0.4 times as much. Therefore 500 is more than sufficient.
> 
> OK, but then I don't see why this reasoning only applies to the lower
> bound of the frequency range.  Is there any reason why x = 1 should be
> the only point mapping to max_freq?

Well, everything that goes over the second to last freq would end up at
the last (max) freq.

Take again the 500,750,1000 example, everything that's >750 would end up
at 1000 (for relation_l, >875 for _c).

But given the platform's cpuidle information, maybe coupled with an avg
idle est, we can compute the benefit of race-to-idle and over provision
based on that, right?

> If not, then I think it's reasonable to map the middle of the
> available frequency range to x = 0.5 and then we have b = 0 and a =
> (max_freq + min_freq) / 2.

So I really think that approach falls apart on the low util bits, you
effectively always run above min speed, even if min is already vstly
over provisioned.

[toc] | [prev] | [next] | [standalone]


#1349351 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromPeter Zijlstra <peterz@infradead.org>
Date2016-03-03 17:50 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8EBQ-1uF-9@gated-at.bofh.it>
In reply to#1349345
On Thu, Mar 03, 2016 at 05:37:35PM +0100, Peter Zijlstra wrote:
> On Thu, Mar 03, 2016 at 05:24:32PM +0100, Rafael J. Wysocki wrote:
> > >> f = a * x + b

> > If not, then I think it's reasonable to map the middle of the
> > available frequency range to x = 0.5 and then we have b = 0 and a =
> > (max_freq + min_freq) / 2.
> 
> So I really think that approach falls apart on the low util bits, you
> effectively always run above min speed, even if min is already vstly
> over provisioned.

Ah nevermind, I cannot read. Yes that is worth trying I suppose. But the
b=0,a=1 thing seems more natural still.

[toc] | [prev] | [next] | [standalone]


#1349754 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2016-03-04 02:20 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8Mzo-7ld-17@gated-at.bofh.it>
In reply to#1349351
On Thu, Mar 3, 2016 at 5:47 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Mar 03, 2016 at 05:37:35PM +0100, Peter Zijlstra wrote:
>> On Thu, Mar 03, 2016 at 05:24:32PM +0100, Rafael J. Wysocki wrote:
>> > >> f = a * x + b
>
>> > If not, then I think it's reasonable to map the middle of the
>> > available frequency range to x = 0.5 and then we have b = 0 and a =
>> > (max_freq + min_freq) / 2.

That actually should be a = max_freq + min_freq, because I want
(max_freq + min_freq) / 2 = a / 2.

>> So I really think that approach falls apart on the low util bits, you
>> effectively always run above min speed, even if min is already vstly
>> over provisioned.
>
> Ah nevermind, I cannot read. Yes that is worth trying I suppose. But the
> b=0,a=1 thing seems more natural still.

It is somewhat imbalanced, though.  If all of the values of x are
equally probable (or equally frequent), the probability of running
above the middle frequency is lower than the probability of running
below it.

[toc] | [prev] | [next] | [standalone]


#1349356 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromPeter Zijlstra <peterz@infradead.org>
Date2016-03-03 18:00 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8ELv-1ze-5@gated-at.bofh.it>
In reply to#1349345
On Thu, Mar 03, 2016 at 04:55:44PM +0000, Juri Lelli wrote:
> On 03/03/16 17:37, Peter Zijlstra wrote:
> > But given the platform's cpuidle information, maybe coupled with an avg
> > idle est, we can compute the benefit of race-to-idle and over provision
> > based on that, right?
> > 
> 
> Shouldn't this kind of considerations be a scheduler thing? I'm not
> really getting why we want to put more "intelligence" in a new governor.
> Also, if I understand Ingo's point correctly, I think we want to make
> this kind of policy decisions inside the scheduler.

Well sure, put it in kernel/sched/cpufreq.c or wherever. My point was
more that we don't have to guess/hardcode race-to-idle assumptions but
can actually calculate some of that.

[toc] | [prev] | [next] | [standalone]


#1349421 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromJuri Lelli <juri.lelli@arm.com>
Date2016-03-03 18:20 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8F4T-1WU-27@gated-at.bofh.it>
In reply to#1349356
On 03/03/16 17:56, Peter Zijlstra wrote:
> On Thu, Mar 03, 2016 at 04:55:44PM +0000, Juri Lelli wrote:
> > On 03/03/16 17:37, Peter Zijlstra wrote:
> > > But given the platform's cpuidle information, maybe coupled with an avg
> > > idle est, we can compute the benefit of race-to-idle and over provision
> > > based on that, right?
> > > 
> > 
> > Shouldn't this kind of considerations be a scheduler thing? I'm not
> > really getting why we want to put more "intelligence" in a new governor.
> > Also, if I understand Ingo's point correctly, I think we want to make
> > this kind of policy decisions inside the scheduler.
> 
> Well sure, put it in kernel/sched/cpufreq.c or wherever. My point was
> more that we don't have to guess/hardcode race-to-idle assumptions but
> can actually calculate some of that.
> 

Right, thanks for clarifying!

[toc] | [prev] | [next] | [standalone]


#1349359 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromJuri Lelli <juri.lelli@arm.com>
Date2016-03-03 18:00 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8ELv-1ze-7@gated-at.bofh.it>
In reply to#1349345
On 03/03/16 17:37, Peter Zijlstra wrote:
> On Thu, Mar 03, 2016 at 05:24:32PM +0100, Rafael J. Wysocki wrote:
> > On Thu, Mar 3, 2016 at 1:20 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > > On Wed, Mar 02, 2016 at 11:49:48PM +0100, Rafael J. Wysocki wrote:
> > >> >>> +       min_f = sg_policy->policy->cpuinfo.min_freq;
> > >> >>> +       max_f = sg_policy->policy->cpuinfo.max_freq;
> > >> >>> +       next_f = util > max ? max_f : min_f + util * (max_f - min_f) / max;
> > >
> > >> In case a more formal derivation of this formula is needed, it is
> > >> based on the following 3 assumptions:
> > >>
> > >> (1) Performance is a linear function of frequency.
> > >> (2) Required performance is a linear function of the utilization ratio
> > >> x = util/max as provided by the scheduler (0 <= x <= 1).
> > >
> > >> (3) The minimum possible frequency (min_freq) corresponds to x = 0 and
> > >> the maximum possible frequency (max_freq) corresponds to x = 1.
> > >>
> > >> (1) and (2) combined imply that
> > >>
> > >> f = a * x + b
> > >>
> > >> (f - frequency, a, b - constants to be determined) and then (3) quite
> > >> trivially leads to b = min_freq and a = max_freq - min_freq.
> > >
> > > 3 is the problem, that just doesn't make sense and is probably the
> > > reason why you see very little selection of the min freq.
> > 
> > It is about mapping the entire [0,1] interval to the available frequency range.
> 
> Yeah, but I don't see why that makes sense..
> 
> > I till overprovision things (the smaller x the more), but then it may
> > help the race-to-idle a bit in theory.
> 
> So, since we also have the cpuidle information, could we not make a
> better guess at race-to-idle?
> 
> > > Suppose a machine with the following frequencies:
> > >
> > >         500, 750, 1000
> > >
> > > And a utilization of 0.4, how does asking for 500 + 0.4 * (1000-500) =
> > > 700 make any sense? Per your point 1, it should should be asking for
> > > 0.4 * 1000 = 400.
> > >
> > > Because, per 1, at 500 it runs exactly half as fast as at 1000, and we
> > > only need 0.4 times as much. Therefore 500 is more than sufficient.
> > 
> > OK, but then I don't see why this reasoning only applies to the lower
> > bound of the frequency range.  Is there any reason why x = 1 should be
> > the only point mapping to max_freq?
> 
> Well, everything that goes over the second to last freq would end up at
> the last (max) freq.
> 
> Take again the 500,750,1000 example, everything that's >750 would end up
> at 1000 (for relation_l, >875 for _c).
> 
> But given the platform's cpuidle information, maybe coupled with an avg
> idle est, we can compute the benefit of race-to-idle and over provision
> based on that, right?
> 

Shouldn't this kind of considerations be a scheduler thing? I'm not
really getting why we want to put more "intelligence" in a new governor.
Also, if I understand Ingo's point correctly, I think we want to make
this kind of policy decisions inside the scheduler.

[toc] | [prev] | [next] | [standalone]


#1349188 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromVincent Guittot <vincent.guittot@linaro.org>
Date2016-03-03 15:10 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8C71-88W-33@gated-at.bofh.it>
In reply to#1348583
On 2 March 2016 at 23:49, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Wed, Mar 2, 2016 at 6:58 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Wed, Mar 2, 2016 at 6:10 PM, Vincent Guittot
>> <vincent.guittot@linaro.org> wrote:
>>> Hi Rafael,
>>>
>>>
>>> On 2 March 2016 at 03:27, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>>
>>>> Add a new cpufreq scaling governor, called "schedutil", that uses
>>>> scheduler-provided CPU utilization information as input for making
>>>> its decisions.
>>>>
>>>> Doing that is possible after commit fe7034338ba0 (cpufreq: Add
>>>> mechanism for registering utilization update callbacks) that
>>>> introduced cpufreq_update_util() called by the scheduler on
>>>> utilization changes (from CFS) and RT/DL task status updates.
>>>> In particular, CPU frequency scaling decisions may be based on
>>>> the the utilization data passed to cpufreq_update_util() by CFS.
>>>>
>>>> The new governor is relatively simple.
>>>>
>>>> The frequency selection formula used by it is essentially the same
>>>> as the one used by the "ondemand" governor, although it doesn't use
>>>> the additional up_threshold parameter, but instead of computing the
>>>> load as the "non-idle CPU time" to "total CPU time" ratio, it takes
>>>> the utilization data provided by CFS as input.  More specifically,
>>>> it represents "load" as the util/max ratio, where util and max
>>>> are the utilization and CPU capacity coming from CFS.
>>>>
>>>
>>> [snip]
>>>
>>>> +
>>>> +static void sugov_update_commit(struct sugov_policy *sg_policy, u64 time,
>>>> +                               unsigned long util, unsigned long max,
>>>> +                               unsigned int next_freq)
>>>> +{
>>>> +       struct cpufreq_policy *policy = sg_policy->policy;
>>>> +       unsigned int rel;
>>>> +
>>>> +       if (next_freq > policy->max)
>>>> +               next_freq = policy->max;
>>>> +       else if (next_freq < policy->min)
>>>> +               next_freq = policy->min;
>>>> +
>>>> +       sg_policy->last_freq_update_time = time;
>>>> +       if (sg_policy->next_freq == next_freq)
>>>> +               return;
>>>> +
>>>> +       sg_policy->next_freq = next_freq;
>>>> +       /*
>>>> +        * If utilization is less than max / 4, use RELATION_C to allow the
>>>> +        * minimum frequency to be selected more often in case the distance from
>>>> +        * it to the next available frequency in the table is significant.
>>>> +        */
>>>> +       rel = util < (max >> 2) ? CPUFREQ_RELATION_C : CPUFREQ_RELATION_L;
>>>> +       if (policy->fast_switch_possible) {
>>>> +               cpufreq_driver_fast_switch(policy, next_freq, rel);
>>>> +       } else {
>>>> +               sg_policy->relation = rel;
>>>> +               sg_policy->work_in_progress = true;
>>>> +               irq_work_queue(&sg_policy->irq_work);
>>>> +       }
>>>> +}
>>>> +
>>>> +static void sugov_update_single(struct update_util_data *data, u64 time,
>>>> +                               unsigned long util, unsigned long max)
>>>> +{
>>>> +       struct sugov_cpu *sg_cpu = container_of(data, struct sugov_cpu, update_util);
>>>> +       struct sugov_policy *sg_policy = sg_cpu->sg_policy;
>>>> +       unsigned int min_f, max_f, next_f;
>>>> +
>>>> +       if (!sugov_should_update_freq(sg_policy, time))
>>>> +               return;
>>>> +
>>>> +       min_f = sg_policy->policy->cpuinfo.min_freq;
>>>> +       max_f = sg_policy->policy->cpuinfo.max_freq;
>>>> +       next_f = util > max ? max_f : min_f + util * (max_f - min_f) / max;
>>>
>>> I think it has been pointed out in another email's thread but you
>>> should change the way the next_f is computed. util reflects the
>>> utilization of a CPU from 0 to its max compute capacity whereas
>>> ondemand was using the load at the current frequency during the last
>>> time window. I have understood that you want to keep same formula than
>>> ondemand as a starting point but you use a different input to
>>> calculate the next frequency so i don't see the rational of keeping
>>> this formula.
>>
>> It is a formula that causes the entire available frequency range to be
>> utilized proportionally to the utilization as reported by the
>> scheduler (modulo the policy->min/max limits).  Its (significant IMO)
>> advantage is that it doesn't require any additional factors that would
>> need to be determined somehow.
>
> In case a more formal derivation of this formula is needed, it is
> based on the following 3 assumptions:
>
> (1) Performance is a linear function of frequency.
> (2) Required performance is a linear function of the utilization ratio
> x = util/max as provided by the scheduler (0 <= x <= 1).

Just to mention that the utilization that you are using, varies with
the frequency which add another variable in your equation

> (3) The minimum possible frequency (min_freq) corresponds to x = 0 and
> the maximum possible frequency (max_freq) corresponds to x = 1.
>
> (1) and (2) combined imply that
>
> f = a * x + b
>
> (f - frequency, a, b - constants to be determined) and then (3) quite
> trivially leads to b = min_freq and a = max_freq - min_freq.
>
> Now, of course, the linearity assumptions may be questioned, but then
> it's just the first approximation.  If you go any further, though, you
> end up with an expansion series like this:
>
> f(x) = c_0 + c_1 * x + c_2 * x^2 + c_3 * x^3 + ...
>
> where all of the c_j need to be determined in principle.  With luck,
> if you can guess what kind of a function f(x) may be, it may be
> possible to reduce the number of coefficients to determine, but
> question is whether or not that is going to work universally for all
> systems.
>
> Thanks,
> Rafael

[toc] | [prev] | [next] | [standalone]


#1349298 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromPeter Zijlstra <peterz@infradead.org>
Date2016-03-03 16:40 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8Dw7-Di-39@gated-at.bofh.it>
In reply to#1349188
On Thu, Mar 03, 2016 at 03:01:15PM +0100, Vincent Guittot wrote:
> > In case a more formal derivation of this formula is needed, it is
> > based on the following 3 assumptions:
> >
> > (1) Performance is a linear function of frequency.
> > (2) Required performance is a linear function of the utilization ratio
> > x = util/max as provided by the scheduler (0 <= x <= 1).
> 
> Just to mention that the utilization that you are using, varies with
> the frequency which add another variable in your equation

Right, x86 hasn't implemented arch_scale_freq_capacity(), so the
utilization values we use are all over the map. If we lower freq, the
util will go up, which would result in us bumping the freq again, etc..

[toc] | [prev] | [next] | [standalone]


#1349340 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromPeter Zijlstra <peterz@infradead.org>
Date2016-03-03 17:30 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8Eiv-1kl-31@gated-at.bofh.it>
In reply to#1349298
On Thu, Mar 03, 2016 at 04:38:17PM +0100, Peter Zijlstra wrote:
> On Thu, Mar 03, 2016 at 03:01:15PM +0100, Vincent Guittot wrote:
> > > In case a more formal derivation of this formula is needed, it is
> > > based on the following 3 assumptions:
> > >
> > > (1) Performance is a linear function of frequency.
> > > (2) Required performance is a linear function of the utilization ratio
> > > x = util/max as provided by the scheduler (0 <= x <= 1).
> > 
> > Just to mention that the utilization that you are using, varies with
> > the frequency which add another variable in your equation
> 
> Right, x86 hasn't implemented arch_scale_freq_capacity(), so the
> utilization values we use are all over the map. If we lower freq, the
> util will go up, which would result in us bumping the freq again, etc..

Something like the completely untested below should maybe work.

Rafael?

---
 arch/x86/include/asm/topology.h | 19 +++++++++++++++++++
 arch/x86/kernel/smpboot.c       | 24 ++++++++++++++++++++++++
 kernel/sched/core.c             |  1 +
 kernel/sched/sched.h            |  7 +++++++
 4 files changed, 51 insertions(+)

diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index 7f991bd5031b..af7b7259db94 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -146,4 +146,23 @@ struct pci_bus;
 int x86_pci_root_bus_node(int bus);
 void x86_pci_root_bus_resources(int bus, struct list_head *resources);
 
+#ifdef CONFIG_SMP
+
+#define arch_scale_freq_tick arch_scale_freq_tick
+#define arch_scale_freq_capacity arch_scale_freq_capacity
+
+DECLARE_PER_CPU(unsigned long, arch_cpu_freq);
+
+static inline arch_scale_freq_capacity(struct sched_domain *sd, int cpu)
+{
+	if (static_cpu_has(X86_FEATURE_APERFMPERF))
+		return per_cpu(arch_cpu_freq, cpu);
+	else
+		return SCHED_CAPACITY_SCALE;
+}
+
+extern void arch_scale_freq_tick(void);
+
+#endif
+
 #endif /* _ASM_X86_TOPOLOGY_H */
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 3bf1e0b5f827..7d459577ee44 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1647,3 +1647,27 @@ void native_play_dead(void)
 }
 
 #endif
+
+static DEFINE_PER_CPU(u64, arch_prev_aperf);
+static DEFINE_PER_CPU(u64, arch_prev_mperf);
+DEFINE_PER_CPU(unsigned long, arch_cpu_freq);
+
+void arch_scale_freq_tick(void)
+{
+	u64 aperf, mperf;
+	u64 acnt, mcnt;
+
+	if (!static_cpu_has(X86_FEATURE_APERFMPERF))
+		return;
+
+	aperf = rdmsrl(MSR_IA32_APERF);
+	mperf = rdmsrl(MSR_IA32_APERF);
+
+	acnt = aperf - this_cpu_read(arch_prev_aperf);
+	mcnt = mperf - this_cpu_read(arch_prev_mperf);
+
+	this_cpu_write(arch_prev_aperf, aperf);
+	this_cpu_write(arch_prev_mperf, mperf);
+
+	this_cpu_write(arch_cpu_freq, div64_u64(acnt * SCHED_CAPACITY_SCALE, mcnt));
+}
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 96e323b26ea9..35dbf909afb2 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2901,6 +2901,7 @@ void scheduler_tick(void)
 	struct rq *rq = cpu_rq(cpu);
 	struct task_struct *curr = rq->curr;
 
+	arch_scale_freq_tick();
 	sched_clock_tick();
 
 	raw_spin_lock(&rq->lock);
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index baa32075f98e..c3825c920e3f 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1408,6 +1408,13 @@ unsigned long arch_scale_freq_capacity(struct sched_domain *sd, int cpu)
 }
 #endif
 
+#ifndef arch_scale_freq_tick
+static __always_inline
+void arch_scale_freq_tick(void)
+{
+}
+#endif
+
 #ifndef arch_scale_cpu_capacity
 static __always_inline
 unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)

[toc] | [prev] | [next] | [standalone]


#1349353 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromPeter Zijlstra <peterz@infradead.org>
Date2016-03-03 17:50 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8EBR-1uF-25@gated-at.bofh.it>
In reply to#1349340
On Thu, Mar 03, 2016 at 05:28:29PM +0100, Peter Zijlstra wrote:
> +void arch_scale_freq_tick(void)
> +{
> +	u64 aperf, mperf;
> +	u64 acnt, mcnt;
> +
> +	if (!static_cpu_has(X86_FEATURE_APERFMPERF))
> +		return;
> +
> +	aperf = rdmsrl(MSR_IA32_APERF);
> +	mperf = rdmsrl(MSR_IA32_APERF);

Actually reading MPERF increases the chances of this working.

> +
> +	acnt = aperf - this_cpu_read(arch_prev_aperf);
> +	mcnt = mperf - this_cpu_read(arch_prev_mperf);
> +
> +	this_cpu_write(arch_prev_aperf, aperf);
> +	this_cpu_write(arch_prev_mperf, mperf);
> +
> +	this_cpu_write(arch_cpu_freq, div64_u64(acnt * SCHED_CAPACITY_SCALE, mcnt));
> +}

[toc] | [prev] | [next] | [standalone]


#1349428 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromDietmar Eggemann <dietmar.eggemann@arm.com>
Date2016-03-03 18:30 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8Fez-21e-13@gated-at.bofh.it>
In reply to#1349340
On 03/03/16 16:28, Peter Zijlstra wrote:
> On Thu, Mar 03, 2016 at 04:38:17PM +0100, Peter Zijlstra wrote:
>> On Thu, Mar 03, 2016 at 03:01:15PM +0100, Vincent Guittot wrote:
>>>> In case a more formal derivation of this formula is needed, it is
>>>> based on the following 3 assumptions:
>>>>
>>>> (1) Performance is a linear function of frequency.
>>>> (2) Required performance is a linear function of the utilization ratio
>>>> x = util/max as provided by the scheduler (0 <= x <= 1).
>>>
>>> Just to mention that the utilization that you are using, varies with
>>> the frequency which add another variable in your equation
>>
>> Right, x86 hasn't implemented arch_scale_freq_capacity(), so the
>> utilization values we use are all over the map. If we lower freq, the
>> util will go up, which would result in us bumping the freq again, etc..
> 
> Something like the completely untested below should maybe work.
> 
> Rafael?
> 

[...]

> +void arch_scale_freq_tick(void)
> +{
> +	u64 aperf, mperf;
> +	u64 acnt, mcnt;
> +
> +	if (!static_cpu_has(X86_FEATURE_APERFMPERF))
> +		return;
> +
> +	aperf = rdmsrl(MSR_IA32_APERF);
> +	mperf = rdmsrl(MSR_IA32_APERF);
> +
> +	acnt = aperf - this_cpu_read(arch_prev_aperf);
> +	mcnt = mperf - this_cpu_read(arch_prev_mperf);
> +
> +	this_cpu_write(arch_prev_aperf, aperf);
> +	this_cpu_write(arch_prev_mperf, mperf);
> +
> +	this_cpu_write(arch_cpu_freq, div64_u64(acnt * SCHED_CAPACITY_SCALE, mcnt));

Wasn't there the problem that this ratio goes to zero if the cpu is idle
in the old power estimation approach on x86?

[...]

[toc] | [prev] | [next] | [standalone]


#1349474 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromPeter Zijlstra <peterz@infradead.org>
Date2016-03-03 19:30 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8GaC-2Ea-19@gated-at.bofh.it>
In reply to#1349428
On Thu, Mar 03, 2016 at 05:28:55PM +0000, Dietmar Eggemann wrote:
> > +void arch_scale_freq_tick(void)
> > +{
> > +	u64 aperf, mperf;
> > +	u64 acnt, mcnt;
> > +
> > +	if (!static_cpu_has(X86_FEATURE_APERFMPERF))
> > +		return;
> > +
> > +	aperf = rdmsrl(MSR_IA32_APERF);
> > +	mperf = rdmsrl(MSR_IA32_APERF);
> > +
> > +	acnt = aperf - this_cpu_read(arch_prev_aperf);
> > +	mcnt = mperf - this_cpu_read(arch_prev_mperf);
> > +
> > +	this_cpu_write(arch_prev_aperf, aperf);
> > +	this_cpu_write(arch_prev_mperf, mperf);
> > +
> > +	this_cpu_write(arch_cpu_freq, div64_u64(acnt * SCHED_CAPACITY_SCALE, mcnt));
> 
> Wasn't there the problem that this ratio goes to zero if the cpu is idle
> in the old power estimation approach on x86?

Yeah, there was something funky.

SDM says they only count in C0 (ie. !idle), so it _should_ work.

[toc] | [prev] | [next] | [standalone]


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web