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

Groups > linux.kernel > #1322164 > unrolled thread

[PATCH 0/3] cpufreq: Replace timers with utilization update callbacks

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-01-30 00:10 +0100
    [PATCH 1/3] cpufreq: Add a mechanism for registering utilization update callbacks "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-01-30 00:10 +0100
      Re: [PATCH 1/3] cpufreq: Add a mechanism for registering utilization  update callbacks Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-04 04:40 +0100
    Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-03 23:20 +0100
      Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> - 2016-02-04 01:10 +0100
        Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-04 18:20 +0100
      Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Juri Lelli <juri.lelli@arm.com> - 2016-02-04 12:00 +0100
        Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-04 18:20 +0100
      Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-09 00:10 +0100
        Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Steve Muckle <steve.muckle@linaro.org> - 2016-02-09 01:40 +0100
          Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-09 02:10 +0100

#1322164 — [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks

Hi,

The following patch series introduces a mechanism allowing the cpufreq core
and "setpolicy" drivers to provide utilization update callbacks to be invoked
by the scheduler on utilization changes.  Those callbacks can be used to run
the sampling and frequency adjustments code (intel_pstate) or to schedule the
execution of that code in process context (cpufreq core) instead of per-CPU
deferrable timers used in cpufreq today (which Thomas complained about during
the last Kernel Summit).

[1/3] Introduce a mechanism for calling into cpufreq from the scheduler and
      registering callbacks to be executed from there.

[2/3] Modify intel_pstate to use the mechanism introduced by [1/3] instead
      of per-CPU deferrable timers to do its work.

This isn't entirely straightforward as the scheduler context running those
callbacks is really special.  Among other things it can only use raw
spinlocks and cannot invoke wake_up_process() directly.  Also, calling
ktime_get() from there may be too expensive on some systems.  All that has to
be taken into account, but even then the change allows some lines of code to be
cut from the driver.

Some performance and energy consumption measurements have been carried out with
an earlier version of this patch and it looks like the changes lead to a
slightly better performing system that consumes slightly less energy at the
same time overall.

[3/3] Modify the cpufreq core to use the mechanism introduced by [1/3] instead
      of per-CPU deferrable timers to queue up the execution of governor work.

Again, this isn't really straightforward for the above reasons, but still the
code size is reduced a bit by the changes.

I'm still unsure about the energy consumption and performance impact of [3/3]
as earlier versions of it led to inconsistent results (most likely due to bugs
in them that hopefully have been fixed in this version).  In particular, the
additional irq_work may turn out to be problematic, but more optimizations are
possible on top of this one even if it makes things worse by itself.

For example, it should be possible to move the execution of state selection
code into the utilization update callback itself, at least in principle, for
all governors.  The P-state/OPP adjustment may need to be run from process
context still, but for the drivers that can do it without sleeping it should
be possible to move that into the utilization update callback as well.

The patches are on top of 4.5-rc1 and have been tested on a couple of x86
machines.

Thanks,
Rafael

[toc] | [next] | [standalone]

#1322165 — [PATCH 1/3] cpufreq: Add a mechanism for registering utilization update callbacks

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Introduce a mechanism by which parts of the cpufreq subsystem
("setpolicy" drivers or the core) can register callbacks to be
executed from cpufreq_update_util() which is invoked by the
scheduler's update_load_avg() on CPU utilization changes.

This allows the "setpolicy" drivers to dispense with their timers
and do all of the computations they need and frequency/voltage
adjustments in the update_load_avg() code path, among other things.

The scheduler changes were suggested by Peter Zijlstra.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/cpufreq.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/cpufreq.h   |    7 +++++++
 include/linux/sched.h     |    2 ++
 kernel/sched/fair.c       |   29 ++++++++++++++++++++++++++++-
 4 files changed, 81 insertions(+), 1 deletion(-)

Index: linux-pm/include/linux/sched.h
===================================================================
--- linux-pm.orig/include/linux/sched.h
+++ linux-pm/include/linux/sched.h
@@ -3207,4 +3207,6 @@ static inline unsigned long rlimit_max(u
 	return task_rlimit_max(current, limit);
 }
 
+void cpufreq_update_util(u64 time, unsigned long util, unsigned long max);
+
 #endif
Index: linux-pm/kernel/sched/fair.c
===================================================================
--- linux-pm.orig/kernel/sched/fair.c
+++ linux-pm/kernel/sched/fair.c
@@ -2803,12 +2803,17 @@ static inline int update_cfs_rq_load_avg
 	return decayed || removed;
 }
 
+__weak void cpufreq_update_util(u64 time, unsigned long util, unsigned long max)
+{
+}
+
 /* Update task and its cfs_rq load average */
 static inline void update_load_avg(struct sched_entity *se, int update_tg)
 {
 	struct cfs_rq *cfs_rq = cfs_rq_of(se);
 	u64 now = cfs_rq_clock_task(cfs_rq);
-	int cpu = cpu_of(rq_of(cfs_rq));
+	struct rq *rq = rq_of(cfs_rq);
+	int cpu = cpu_of(rq);
 
 	/*
 	 * Track task load average for carrying it to new CPU after migrated, and
@@ -2820,6 +2825,28 @@ static inline void update_load_avg(struc
 
 	if (update_cfs_rq_load_avg(now, cfs_rq) && update_tg)
 		update_tg_load_avg(cfs_rq, 0);
+
+	if (cpu == smp_processor_id() && &rq->cfs == cfs_rq) {
+		unsigned long max = rq->cpu_capacity_orig;
+
+		/*
+		 * There are a few boundary cases this might miss but it should
+		 * get called often enough that that should (hopefully) not be
+		 * a real problem -- added to that it only calls on the local
+		 * CPU, so if we enqueue remotely we'll loose an update, but
+		 * the next tick/schedule should update.
+		 *
+		 * It will not get called when we go idle, because the idle
+		 * thread is a different class (!fair), nor will the utilization
+		 * number include things like RT tasks.
+		 *
+		 * As is, the util number is not freq invariant (we'd have to
+		 * implement arch_scale_freq_capacity() for that).
+		 *
+		 * See cpu_util().
+		 */
+		cpufreq_update_util(now, min(cfs_rq->avg.util_avg, max), max);
+	}
 }
 
 static void attach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -102,6 +102,50 @@ static LIST_HEAD(cpufreq_governor_list);
 static struct cpufreq_driver *cpufreq_driver;
 static DEFINE_PER_CPU(struct cpufreq_policy *, cpufreq_cpu_data);
 static DEFINE_RWLOCK(cpufreq_driver_lock);
+
+static DEFINE_PER_CPU(struct update_util_data *, cpufreq_update_util_data);
+
+/**
+ * cpufreq_set_update_util_data - Populate the CPU's update_util_data pointer.
+ * @cpu: The CPU to set the pointer for.
+ * @data: New pointer value.
+ *
+ * Set and publish the update_util_data pointer for the given CPU.  That pointer
+ * points to a struct update_util_data object containing a callback function
+ * to call from cpufreq_update_util().  That function will be called from an RCU
+ * read-side critical section, so it must not sleep.
+ *
+ * Callers must use RCU callbacks to free any memory that might be accessed
+ * via the old update_util_data pointer or invoke synchronize_rcu() right after
+ * this function to avoid use-after-free.
+ */
+void cpufreq_set_update_util_data(int cpu, struct update_util_data *data)
+{
+	rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
+}
+EXPORT_SYMBOL_GPL(cpufreq_set_update_util_data);
+
+/**
+ * cpufreq_update_util - Take a note about CPU utilization changes.
+ * @util: Current utilization.
+ * @max: Utilization ceiling.
+ *
+ * This function is called by the scheduler on every invocation of
+ * update_load_avg() on the CPU whose utilization is being updated.
+ */
+void cpufreq_update_util(u64 time, unsigned long util, unsigned long max)
+{
+	struct update_util_data *data;
+
+	rcu_read_lock();
+
+	data = rcu_dereference(*this_cpu_ptr(&cpufreq_update_util_data));
+	if (data && data->func)
+		data->func(data, time, util, max);
+
+	rcu_read_unlock();
+}
+
 DEFINE_MUTEX(cpufreq_governor_lock);
 
 /* Flag to suspend/resume CPUFreq governors */
Index: linux-pm/include/linux/cpufreq.h
===================================================================
--- linux-pm.orig/include/linux/cpufreq.h
+++ linux-pm/include/linux/cpufreq.h
@@ -322,6 +322,13 @@ int cpufreq_unregister_driver(struct cpu
 const char *cpufreq_get_current_driver(void);
 void *cpufreq_get_driver_data(void);
 
+struct update_util_data {
+	void (*func)(struct update_util_data *data,
+		     u64 time, unsigned long util, unsigned long max);
+};
+
+void cpufreq_set_update_util_data(int cpu, struct update_util_data *data);
+
 static inline void cpufreq_verify_within_limits(struct cpufreq_policy *policy,
 		unsigned int min, unsigned int max)
 {

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

#1326368 — Re: [PATCH 1/3] cpufreq: Add a mechanism for registering utilization update callbacks

On 29-01-16, 23:53, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Introduce a mechanism by which parts of the cpufreq subsystem
> ("setpolicy" drivers or the core) can register callbacks to be
> executed from cpufreq_update_util() which is invoked by the
> scheduler's update_load_avg() on CPU utilization changes.
> 
> This allows the "setpolicy" drivers to dispense with their timers
> and do all of the computations they need and frequency/voltage
> adjustments in the update_load_avg() code path, among other things.
> 
> The scheduler changes were suggested by Peter Zijlstra.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/cpufreq/cpufreq.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/cpufreq.h   |    7 +++++++
>  include/linux/sched.h     |    2 ++
>  kernel/sched/fair.c       |   29 ++++++++++++++++++++++++++++-
>  4 files changed, 81 insertions(+), 1 deletion(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

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

#1326003

On Friday, January 29, 2016 11:52:15 PM Rafael J. Wysocki wrote:
> Hi,
> 
> The following patch series introduces a mechanism allowing the cpufreq core
> and "setpolicy" drivers to provide utilization update callbacks to be invoked
> by the scheduler on utilization changes.  Those callbacks can be used to run
> the sampling and frequency adjustments code (intel_pstate) or to schedule the
> execution of that code in process context (cpufreq core) instead of per-CPU
> deferrable timers used in cpufreq today (which Thomas complained about during
> the last Kernel Summit).
> 
> [1/3] Introduce a mechanism for calling into cpufreq from the scheduler and
>       registering callbacks to be executed from there.
> 
> [2/3] Modify intel_pstate to use the mechanism introduced by [1/3] instead
>       of per-CPU deferrable timers to do its work.
> 
> This isn't entirely straightforward as the scheduler context running those
> callbacks is really special.  Among other things it can only use raw
> spinlocks and cannot invoke wake_up_process() directly.  Also, calling
> ktime_get() from there may be too expensive on some systems.  All that has to
> be taken into account, but even then the change allows some lines of code to be
> cut from the driver.
> 
> Some performance and energy consumption measurements have been carried out with
> an earlier version of this patch and it looks like the changes lead to a
> slightly better performing system that consumes slightly less energy at the
> same time overall.
> 
> [3/3] Modify the cpufreq core to use the mechanism introduced by [1/3] instead
>       of per-CPU deferrable timers to queue up the execution of governor work.
> 
> Again, this isn't really straightforward for the above reasons, but still the
> code size is reduced a bit by the changes.
> 
> I'm still unsure about the energy consumption and performance impact of [3/3]
> as earlier versions of it led to inconsistent results (most likely due to bugs
> in them that hopefully have been fixed in this version).  In particular, the
> additional irq_work may turn out to be problematic, but more optimizations are
> possible on top of this one even if it makes things worse by itself.
> 
> For example, it should be possible to move the execution of state selection
> code into the utilization update callback itself, at least in principle, for
> all governors.  The P-state/OPP adjustment may need to be run from process
> context still, but for the drivers that can do it without sleeping it should
> be possible to move that into the utilization update callback as well.
> 
> The patches are on top of 4.5-rc1 and have been tested on a couple of x86
> machines.

Well, no responses here, so I'm inclined to believe that this series is fine
by everybody (at least by everybody in the CC).

I can wait for a few days more, but new material is starting to pile up on top
of these patches and I'll simply need to move forward at one point.

Thanks,
Rafael

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

#1326300 — Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks

On 02/03/2016 02:20 PM, Rafael J. Wysocki wrote:
> On Friday, January 29, 2016 11:52:15 PM Rafael J. Wysocki wrote:
>> Hi,
>>
>> The following patch series introduces a mechanism allowing the cpufreq core
>> and "setpolicy" drivers to provide utilization update callbacks to be invoked
>> by the scheduler on utilization changes.  Those callbacks can be used to run
>> the sampling and frequency adjustments code (intel_pstate) or to schedule the
>> execution of that code in process context (cpufreq core) instead of per-CPU
>> deferrable timers used in cpufreq today (which Thomas complained about during
>> the last Kernel Summit).
>>
>> [1/3] Introduce a mechanism for calling into cpufreq from the scheduler and
>>        registering callbacks to be executed from there.
>>
>> [2/3] Modify intel_pstate to use the mechanism introduced by [1/3] instead
>>        of per-CPU deferrable timers to do its work.
>>
>> This isn't entirely straightforward as the scheduler context running those
>> callbacks is really special.  Among other things it can only use raw
>> spinlocks and cannot invoke wake_up_process() directly.  Also, calling
>> ktime_get() from there may be too expensive on some systems.  All that has to
>> be taken into account, but even then the change allows some lines of code to be
>> cut from the driver.
>>
>> Some performance and energy consumption measurements have been carried out with
>> an earlier version of this patch and it looks like the changes lead to a
>> slightly better performing system that consumes slightly less energy at the
>> same time overall.
>>
>> [3/3] Modify the cpufreq core to use the mechanism introduced by [1/3] instead
>>        of per-CPU deferrable timers to queue up the execution of governor work.
>>
>> Again, this isn't really straightforward for the above reasons, but still the
>> code size is reduced a bit by the changes.
>>
>> I'm still unsure about the energy consumption and performance impact of [3/3]
>> as earlier versions of it led to inconsistent results (most likely due to bugs
>> in them that hopefully have been fixed in this version).  In particular, the
>> additional irq_work may turn out to be problematic, but more optimizations are
>> possible on top of this one even if it makes things worse by itself.
>>
>> For example, it should be possible to move the execution of state selection
>> code into the utilization update callback itself, at least in principle, for
>> all governors.  The P-state/OPP adjustment may need to be run from process
>> context still, but for the drivers that can do it without sleeping it should
>> be possible to move that into the utilization update callback as well.
>>
>> The patches are on top of 4.5-rc1 and have been tested on a couple of x86
>> machines.
> Well, no responses here, so I'm inclined to believe that this series is fine
> by everybody (at least by everybody in the CC).
>
> I can wait for a few days more, but new material is starting to pile up on top
> of these patches and I'll simply need to move forward at one point.
Based on the test results for intel_pstate and acpi_cpufreq, I don't see 
any problem in applying these patches.

Thanks,
Srinivas
> Thanks,
> Rafael
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

#1327031

On Thu, Feb 4, 2016 at 1:08 AM, Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
>
> On 02/03/2016 02:20 PM, Rafael J. Wysocki wrote:
>>
>> On Friday, January 29, 2016 11:52:15 PM Rafael J. Wysocki wrote:
>>>
>>> Hi,
>>>
>>> The following patch series introduces a mechanism allowing the cpufreq
>>> core
>>> and "setpolicy" drivers to provide utilization update callbacks to be
>>> invoked
>>> by the scheduler on utilization changes.  Those callbacks can be used to
>>> run
>>> the sampling and frequency adjustments code (intel_pstate) or to schedule
>>> the
>>> execution of that code in process context (cpufreq core) instead of
>>> per-CPU
>>> deferrable timers used in cpufreq today (which Thomas complained about
>>> during
>>> the last Kernel Summit).
>>>
>>> [1/3] Introduce a mechanism for calling into cpufreq from the scheduler
>>> and
>>>        registering callbacks to be executed from there.
>>>
>>> [2/3] Modify intel_pstate to use the mechanism introduced by [1/3]
>>> instead
>>>        of per-CPU deferrable timers to do its work.
>>>
>>> This isn't entirely straightforward as the scheduler context running
>>> those
>>> callbacks is really special.  Among other things it can only use raw
>>> spinlocks and cannot invoke wake_up_process() directly.  Also, calling
>>> ktime_get() from there may be too expensive on some systems.  All that
>>> has to
>>> be taken into account, but even then the change allows some lines of code
>>> to be
>>> cut from the driver.
>>>
>>> Some performance and energy consumption measurements have been carried
>>> out with
>>> an earlier version of this patch and it looks like the changes lead to a
>>> slightly better performing system that consumes slightly less energy at
>>> the
>>> same time overall.
>>>
>>> [3/3] Modify the cpufreq core to use the mechanism introduced by [1/3]
>>> instead
>>>        of per-CPU deferrable timers to queue up the execution of governor
>>> work.
>>>
>>> Again, this isn't really straightforward for the above reasons, but still
>>> the
>>> code size is reduced a bit by the changes.
>>>
>>> I'm still unsure about the energy consumption and performance impact of
>>> [3/3]
>>> as earlier versions of it led to inconsistent results (most likely due to
>>> bugs
>>> in them that hopefully have been fixed in this version).  In particular,
>>> the
>>> additional irq_work may turn out to be problematic, but more
>>> optimizations are
>>> possible on top of this one even if it makes things worse by itself.
>>>
>>> For example, it should be possible to move the execution of state
>>> selection
>>> code into the utilization update callback itself, at least in principle,
>>> for
>>> all governors.  The P-state/OPP adjustment may need to be run from
>>> process
>>> context still, but for the drivers that can do it without sleeping it
>>> should
>>> be possible to move that into the utilization update callback as well.
>>>
>>> The patches are on top of 4.5-rc1 and have been tested on a couple of x86
>>> machines.
>>
>> Well, no responses here, so I'm inclined to believe that this series is
>> fine
>> by everybody (at least by everybody in the CC).
>>
>> I can wait for a few days more, but new material is starting to pile up on
>> top
>> of these patches and I'll simply need to move forward at one point.
>
> Based on the test results for intel_pstate and acpi_cpufreq, I don't see any
> problem in applying these patches.

OK, I'm taking this as an ACK for the intel_pstate changes. :-)

Thanks,
Rafael

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

#1326659 — Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks

Hi Rafael,

On 03/02/16 23:20, Rafael J. Wysocki wrote:
> On Friday, January 29, 2016 11:52:15 PM Rafael J. Wysocki wrote:
> > Hi,
> > 
> > The following patch series introduces a mechanism allowing the cpufreq core
> > and "setpolicy" drivers to provide utilization update callbacks to be invoked
> > by the scheduler on utilization changes.  Those callbacks can be used to run
> > the sampling and frequency adjustments code (intel_pstate) or to schedule the
> > execution of that code in process context (cpufreq core) instead of per-CPU
> > deferrable timers used in cpufreq today (which Thomas complained about during
> > the last Kernel Summit).
> > 
> > [1/3] Introduce a mechanism for calling into cpufreq from the scheduler and
> >       registering callbacks to be executed from there.
> > 
> > [2/3] Modify intel_pstate to use the mechanism introduced by [1/3] instead
> >       of per-CPU deferrable timers to do its work.
> > 
> > This isn't entirely straightforward as the scheduler context running those
> > callbacks is really special.  Among other things it can only use raw
> > spinlocks and cannot invoke wake_up_process() directly.  Also, calling
> > ktime_get() from there may be too expensive on some systems.  All that has to
> > be taken into account, but even then the change allows some lines of code to be
> > cut from the driver.
> > 
> > Some performance and energy consumption measurements have been carried out with
> > an earlier version of this patch and it looks like the changes lead to a
> > slightly better performing system that consumes slightly less energy at the
> > same time overall.
> > 
> > [3/3] Modify the cpufreq core to use the mechanism introduced by [1/3] instead
> >       of per-CPU deferrable timers to queue up the execution of governor work.
> > 
> > Again, this isn't really straightforward for the above reasons, but still the
> > code size is reduced a bit by the changes.
> > 
> > I'm still unsure about the energy consumption and performance impact of [3/3]
> > as earlier versions of it led to inconsistent results (most likely due to bugs
> > in them that hopefully have been fixed in this version).  In particular, the
> > additional irq_work may turn out to be problematic, but more optimizations are
> > possible on top of this one even if it makes things worse by itself.
> > 
> > For example, it should be possible to move the execution of state selection
> > code into the utilization update callback itself, at least in principle, for
> > all governors.  The P-state/OPP adjustment may need to be run from process
> > context still, but for the drivers that can do it without sleeping it should
> > be possible to move that into the utilization update callback as well.
> > 
> > The patches are on top of 4.5-rc1 and have been tested on a couple of x86
> > machines.
> 
> Well, no responses here, so I'm inclined to believe that this series is fine
> by everybody (at least by everybody in the CC).
> 

I did intend to test and review this series, but then other patches
required attention as well and I didn't find time to have a look at
these. Sorry about that. Also, if I can speak for him, I think that
Steve is OOO this week.

> I can wait for a few days more, but new material is starting to pile up on top
> of these patches and I'll simply need to move forward at one point.
> 

Unfortunately, I can't promise anything at the moment, but, if I find
some time, I'll run some tests (BTW, do you have alredy something that I
can put to run on my boxes?). I guess I can eventually do that after
this gets merged as well.

Best,

- Juri

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

#1327032

On Thu, Feb 4, 2016 at 11:51 AM, Juri Lelli <juri.lelli@arm.com> wrote:
> Hi Rafael,
>
> On 03/02/16 23:20, Rafael J. Wysocki wrote:
>> On Friday, January 29, 2016 11:52:15 PM Rafael J. Wysocki wrote:
>> > Hi,
>> >
>> > The following patch series introduces a mechanism allowing the cpufreq core
>> > and "setpolicy" drivers to provide utilization update callbacks to be invoked
>> > by the scheduler on utilization changes.  Those callbacks can be used to run
>> > the sampling and frequency adjustments code (intel_pstate) or to schedule the
>> > execution of that code in process context (cpufreq core) instead of per-CPU
>> > deferrable timers used in cpufreq today (which Thomas complained about during
>> > the last Kernel Summit).
>> >
>> > [1/3] Introduce a mechanism for calling into cpufreq from the scheduler and
>> >       registering callbacks to be executed from there.
>> >
>> > [2/3] Modify intel_pstate to use the mechanism introduced by [1/3] instead
>> >       of per-CPU deferrable timers to do its work.
>> >
>> > This isn't entirely straightforward as the scheduler context running those
>> > callbacks is really special.  Among other things it can only use raw
>> > spinlocks and cannot invoke wake_up_process() directly.  Also, calling
>> > ktime_get() from there may be too expensive on some systems.  All that has to
>> > be taken into account, but even then the change allows some lines of code to be
>> > cut from the driver.
>> >
>> > Some performance and energy consumption measurements have been carried out with
>> > an earlier version of this patch and it looks like the changes lead to a
>> > slightly better performing system that consumes slightly less energy at the
>> > same time overall.
>> >
>> > [3/3] Modify the cpufreq core to use the mechanism introduced by [1/3] instead
>> >       of per-CPU deferrable timers to queue up the execution of governor work.
>> >
>> > Again, this isn't really straightforward for the above reasons, but still the
>> > code size is reduced a bit by the changes.
>> >
>> > I'm still unsure about the energy consumption and performance impact of [3/3]
>> > as earlier versions of it led to inconsistent results (most likely due to bugs
>> > in them that hopefully have been fixed in this version).  In particular, the
>> > additional irq_work may turn out to be problematic, but more optimizations are
>> > possible on top of this one even if it makes things worse by itself.
>> >
>> > For example, it should be possible to move the execution of state selection
>> > code into the utilization update callback itself, at least in principle, for
>> > all governors.  The P-state/OPP adjustment may need to be run from process
>> > context still, but for the drivers that can do it without sleeping it should
>> > be possible to move that into the utilization update callback as well.
>> >
>> > The patches are on top of 4.5-rc1 and have been tested on a couple of x86
>> > machines.
>>
>> Well, no responses here, so I'm inclined to believe that this series is fine
>> by everybody (at least by everybody in the CC).
>>
>
> I did intend to test and review this series, but then other patches
> required attention as well and I didn't find time to have a look at
> these. Sorry about that. Also, if I can speak for him, I think that
> Steve is OOO this week.

No problem at all.

>> I can wait for a few days more, but new material is starting to pile up on top
>> of these patches and I'll simply need to move forward at one point.
>>
>
> Unfortunately, I can't promise anything at the moment, but, if I find
> some time, I'll run some tests (BTW, do you have alredy something that I
> can put to run on my boxes?). I guess I can eventually do that after
> this gets merged as well.

Thanks!

Well, everything that might regress performance-wise or from the
energy consumption standpoint would be good to run.

Thanks,
Rafael

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

#1329675

On Wednesday, February 03, 2016 11:20:19 PM Rafael J. Wysocki wrote:
> On Friday, January 29, 2016 11:52:15 PM Rafael J. Wysocki wrote:
> > Hi,
> > 
> > The following patch series introduces a mechanism allowing the cpufreq core
> > and "setpolicy" drivers to provide utilization update callbacks to be invoked
> > by the scheduler on utilization changes.  Those callbacks can be used to run
> > the sampling and frequency adjustments code (intel_pstate) or to schedule the
> > execution of that code in process context (cpufreq core) instead of per-CPU
> > deferrable timers used in cpufreq today (which Thomas complained about during
> > the last Kernel Summit).
> > 
> > [1/3] Introduce a mechanism for calling into cpufreq from the scheduler and
> >       registering callbacks to be executed from there.
> > 
> > [2/3] Modify intel_pstate to use the mechanism introduced by [1/3] instead
> >       of per-CPU deferrable timers to do its work.
> > 
> > This isn't entirely straightforward as the scheduler context running those
> > callbacks is really special.  Among other things it can only use raw
> > spinlocks and cannot invoke wake_up_process() directly.  Also, calling
> > ktime_get() from there may be too expensive on some systems.  All that has to
> > be taken into account, but even then the change allows some lines of code to be
> > cut from the driver.
> > 
> > Some performance and energy consumption measurements have been carried out with
> > an earlier version of this patch and it looks like the changes lead to a
> > slightly better performing system that consumes slightly less energy at the
> > same time overall.
> > 
> > [3/3] Modify the cpufreq core to use the mechanism introduced by [1/3] instead
> >       of per-CPU deferrable timers to queue up the execution of governor work.
> > 
> > Again, this isn't really straightforward for the above reasons, but still the
> > code size is reduced a bit by the changes.
> > 
> > I'm still unsure about the energy consumption and performance impact of [3/3]
> > as earlier versions of it led to inconsistent results (most likely due to bugs
> > in them that hopefully have been fixed in this version).  In particular, the
> > additional irq_work may turn out to be problematic, but more optimizations are
> > possible on top of this one even if it makes things worse by itself.
> > 
> > For example, it should be possible to move the execution of state selection
> > code into the utilization update callback itself, at least in principle, for
> > all governors.  The P-state/OPP adjustment may need to be run from process
> > context still, but for the drivers that can do it without sleeping it should
> > be possible to move that into the utilization update callback as well.
> > 
> > The patches are on top of 4.5-rc1 and have been tested on a couple of x86
> > machines.
> 
> Well, no responses here, so I'm inclined to believe that this series is fine
> by everybody (at least by everybody in the CC).
> 
> I can wait for a few days more, but new material is starting to pile up on top
> of these patches and I'll simply need to move forward at one point.

Now that all review comments have been addressed in patch [3/3], I'm going to
put this series into linux-next.

There already is 20+ patches on top of it in the queue including fixes for
bugs that have haunted us for quite some time (and that functionally depend on
this set) and I'd really like all that to get enough linux-next coverage, so
there really isn't more time to wait.

Thanks,
Rafael

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

#1329800 — Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks

Hi Rafael,

On 02/08/2016 03:06 PM, Rafael J. Wysocki wrote:
> Now that all review comments have been addressed in patch [3/3], I'm going to
> put this series into linux-next.
> 
> There already is 20+ patches on top of it in the queue including fixes for
> bugs that have haunted us for quite some time (and that functionally depend on
> this set) and I'd really like all that to get enough linux-next coverage, so
> there really isn't more time to wait.

Sorry for the late reply. As Juri mentioned I was OOO last week and
really just got to look at this today.

One concern I had was, given that the lone scheduler update hook is in
CFS, is it possible for governor updates to be stalled due to RT or DL
task activity?

thanks,
Steve

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

#1329807

On Tue, Feb 9, 2016 at 1:39 AM, Steve Muckle <steve.muckle@linaro.org> wrote:
> Hi Rafael,
>
> On 02/08/2016 03:06 PM, Rafael J. Wysocki wrote:
>> Now that all review comments have been addressed in patch [3/3], I'm going to
>> put this series into linux-next.
>>
>> There already is 20+ patches on top of it in the queue including fixes for
>> bugs that have haunted us for quite some time (and that functionally depend on
>> this set) and I'd really like all that to get enough linux-next coverage, so
>> there really isn't more time to wait.
>
> Sorry for the late reply. As Juri mentioned I was OOO last week and
> really just got to look at this today.
>
> One concern I had was, given that the lone scheduler update hook is in
> CFS, is it possible for governor updates to be stalled due to RT or DL
> task activity?

I don't think they may be completely stalled, but I'd prefer Peter to
answer that as he suggested to do it this way.

Peter?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web