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

Groups > linux.kernel > #1330619 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-09 21:10 +0100
    Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Steve Muckle <steve.muckle@linaro.org> - 2016-02-10 02:10 +0100
      Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-10 03:00 +0100
        Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-10 04:10 +0100
          Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Steve Muckle <steve.muckle@linaro.org> - 2016-02-10 20:50 +0100
            Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-10 22:50 +0100
              Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Steve Muckle <steve.muckle@linaro.org> - 2016-02-10 23:10 +0100
                Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-10 23:20 +0100
      Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Peter Zijlstra <peterz@infradead.org> - 2016-02-11 13:10 +0100
        Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Juri Lelli <juri.lelli@arm.com> - 2016-02-11 13:30 +0100
          Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Peter Zijlstra <peterz@infradead.org> - 2016-02-11 16:30 +0100
            Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks Vincent Guittot <vincent.guittot@linaro.org> - 2016-02-11 19:30 +0100
              Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Peter Zijlstra <peterz@infradead.org> - 2016-02-12 15:10 +0100
                Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks Vincent Guittot <vincent.guittot@linaro.org> - 2016-02-12 15:50 +0100
        Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Steve Muckle <steve.muckle@linaro.org> - 2016-02-11 18:10 +0100
          Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-11 18:40 +0100
            Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Peter Zijlstra <peterz@infradead.org> - 2016-02-11 18:40 +0100
          Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Peter Zijlstra <peterz@infradead.org> - 2016-02-11 18:40 +0100
            Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Steve Muckle <steve.muckle@linaro.org> - 2016-02-11 20:00 +0100
              Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-11 20:10 +0100
                Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-12 14:50 +0100
              Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Peter Zijlstra <peterz@infradead.org> - 2016-02-12 15:20 +0100
                Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-12 17:10 +0100
                  Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-12 17:20 +0100
                    Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks Ashwin Chaugule <ashwin.chaugule@linaro.org> - 2016-02-12 18:00 +0100
                      Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-13 00:20 +0100
                  RE: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Doug Smythies" <dsmythies@telus.net> - 2016-02-12 18:10 +0100
                    Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-13 00:20 +0100
    Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Juri Lelli <juri.lelli@arm.com> - 2016-02-10 13:40 +0100
      Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-10 14:30 +0100
        Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Juri Lelli <juri.lelli@arm.com> - 2016-02-10 15:10 +0100
          Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-10 15:30 +0100
            Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Juri Lelli <juri.lelli@arm.com> - 2016-02-10 15:50 +0100
              Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-10 16:50 +0100
                Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Juri Lelli <juri.lelli@arm.com> - 2016-02-10 17:10 +0100
    Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Peter Zijlstra <peterz@infradead.org> - 2016-02-11 13:00 +0100
      Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-11 13:10 +0100
        Re: [PATCH 0/3] cpufreq: Replace timers with utilization update  callbacks Peter Zijlstra <peterz@infradead.org> - 2016-02-11 16:30 +0100
          Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-11 17:00 +0100
        Re: [PATCH 0/3] cpufreq: Replace timers with utilization update callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-11 21:50 +0100

Page 1 of 2  [1] 2  Next page →

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

On Tuesday, February 09, 2016 02:01:39 AM Rafael J. Wysocki wrote:
> 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.

In any case, if that concern turns out to be significant in practice, it may
be addressed like in the appended modification of patch [1/3] from the $subject
series.

With that things look like before from the cpufreq side, but the other sched
classes also get a chance to trigger a cpufreq update.  The drawback is the
cpu_clock() call instead of passing the time value from update_load_avg(), but
I guess we can live with that if necessary.

FWIW, this modification doesn't seem to break things on my test machine.

Thanks,
Rafael


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     |    7 +++++++
 kernel/sched/deadline.c   |    3 +++
 kernel/sched/fair.c       |   29 ++++++++++++++++++++++++++++-
 kernel/sched/rt.c         |    3 +++
 6 files changed, 92 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,11 @@ static inline unsigned long rlimit_max(u
 	return task_rlimit_max(current, limit);
 }
 
+void cpufreq_update_util(unsigned long util, unsigned long max);
+
+static inline void cpufreq_kick(void)
+{
+	cpufreq_update_util(ULONG_MAX, ULONG_MAX);
+}
+
 #endif
Index: linux-pm/kernel/sched/fair.c
===================================================================
--- linux-pm.orig/kernel/sched/fair.c
+++ linux-pm/kernel/sched/fair.c
@@ -2819,12 +2819,17 @@ static inline int update_cfs_rq_load_avg
 	return decayed || removed;
 }
 
+__weak void cpufreq_update_util(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
@@ -2836,6 +2841,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(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(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, cpu_clock(smp_processor_id()), 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)
 {
Index: linux-pm/kernel/sched/rt.c
===================================================================
--- linux-pm.orig/kernel/sched/rt.c
+++ linux-pm/kernel/sched/rt.c
@@ -2212,6 +2212,9 @@ static void task_tick_rt(struct rq *rq,
 
 	update_curr_rt(rq);
 
+	/* Kick cpufreq to prevent it from stalling. */
+	cpufreq_kick();
+
 	watchdog(rq, p);
 
 	/*
Index: linux-pm/kernel/sched/deadline.c
===================================================================
--- linux-pm.orig/kernel/sched/deadline.c
+++ linux-pm/kernel/sched/deadline.c
@@ -1197,6 +1197,9 @@ static void task_tick_dl(struct rq *rq,
 {
 	update_curr_dl(rq);
 
+	/* Kick cpufreq to prevent it from stalling. */
+	cpufreq_kick();
+
 	/*
 	 * Even when we have runtime, update_curr_dl() might have resulted in us
 	 * not being the leftmost task anymore. In that case NEED_RESCHED will

[toc] | [next] | [standalone]

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

On 02/09/2016 12:05 PM, Rafael J. Wysocki wrote:
>>> 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.
> 
> In any case, if that concern turns out to be significant in practice, it may
> be addressed like in the appended modification of patch [1/3] from the $subject
> series.
> 
> With that things look like before from the cpufreq side, but the other sched
> classes also get a chance to trigger a cpufreq update.  The drawback is the
> cpu_clock() call instead of passing the time value from update_load_avg(), but
> I guess we can live with that if necessary.
> 
> FWIW, this modification doesn't seem to break things on my test machine.
> 
...
> Index: linux-pm/kernel/sched/rt.c
> ===================================================================
> --- linux-pm.orig/kernel/sched/rt.c
> +++ linux-pm/kernel/sched/rt.c
> @@ -2212,6 +2212,9 @@ static void task_tick_rt(struct rq *rq,
>  
>  	update_curr_rt(rq);
>  
> +	/* Kick cpufreq to prevent it from stalling. */
> +	cpufreq_kick();
> +
>  	watchdog(rq, p);
>  
>  	/*
> Index: linux-pm/kernel/sched/deadline.c
> ===================================================================
> --- linux-pm.orig/kernel/sched/deadline.c
> +++ linux-pm/kernel/sched/deadline.c
> @@ -1197,6 +1197,9 @@ static void task_tick_dl(struct rq *rq,
>  {
>  	update_curr_dl(rq);
>  
> +	/* Kick cpufreq to prevent it from stalling. */
> +	cpufreq_kick();
> +
>  	/*
>  	 * Even when we have runtime, update_curr_dl() might have resulted in us
>  	 * not being the leftmost task anymore. In that case NEED_RESCHED will

I think additional hooks such as enqueue/dequeue would be needed in
RT/DL. The task tick callbacks will only run if a task in that class is
executing at the time of the tick. There could be intermittent RT/DL
task activity in a frequency domain (the only task activity there, no
CFS tasks) that doesn't happen to overlap the tick. Worst case the task
activity could be periodic in such a way that it never overlaps the tick
and the update is never made.

thanks,
steve

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

#1330846

On Wed, Feb 10, 2016 at 2:02 AM, Steve Muckle <steve.muckle@linaro.org> wrote:
> On 02/09/2016 12:05 PM, Rafael J. Wysocki wrote:
>>>> 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.
>>
>> In any case, if that concern turns out to be significant in practice, it may
>> be addressed like in the appended modification of patch [1/3] from the $subject
>> series.
>>
>> With that things look like before from the cpufreq side, but the other sched
>> classes also get a chance to trigger a cpufreq update.  The drawback is the
>> cpu_clock() call instead of passing the time value from update_load_avg(), but
>> I guess we can live with that if necessary.
>>
>> FWIW, this modification doesn't seem to break things on my test machine.
>>
> ...
>> Index: linux-pm/kernel/sched/rt.c
>> ===================================================================
>> --- linux-pm.orig/kernel/sched/rt.c
>> +++ linux-pm/kernel/sched/rt.c
>> @@ -2212,6 +2212,9 @@ static void task_tick_rt(struct rq *rq,
>>
>>       update_curr_rt(rq);
>>
>> +     /* Kick cpufreq to prevent it from stalling. */
>> +     cpufreq_kick();
>> +
>>       watchdog(rq, p);
>>
>>       /*
>> Index: linux-pm/kernel/sched/deadline.c
>> ===================================================================
>> --- linux-pm.orig/kernel/sched/deadline.c
>> +++ linux-pm/kernel/sched/deadline.c
>> @@ -1197,6 +1197,9 @@ static void task_tick_dl(struct rq *rq,
>>  {
>>       update_curr_dl(rq);
>>
>> +     /* Kick cpufreq to prevent it from stalling. */
>> +     cpufreq_kick();
>> +
>>       /*
>>        * Even when we have runtime, update_curr_dl() might have resulted in us
>>        * not being the leftmost task anymore. In that case NEED_RESCHED will
>
> I think additional hooks such as enqueue/dequeue would be needed in
> RT/DL. The task tick callbacks will only run if a task in that class is
> executing at the time of the tick. There could be intermittent RT/DL
> task activity in a frequency domain (the only task activity there, no
> CFS tasks) that doesn't happen to overlap the tick. Worst case the task
> activity could be periodic in such a way that it never overlaps the tick
> and the update is never made.

So if I'm reading this correctly, it would be better to put the hooks
into update_curr_rt/dl()?

Thanks,
Rafael

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

#1330872

On Wed, Feb 10, 2016 at 2:57 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Wed, Feb 10, 2016 at 2:02 AM, Steve Muckle <steve.muckle@linaro.org> wrote:
>> On 02/09/2016 12:05 PM, Rafael J. Wysocki wrote:
>>>>> 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.
>>>
>>> In any case, if that concern turns out to be significant in practice, it may
>>> be addressed like in the appended modification of patch [1/3] from the $subject
>>> series.
>>>
>>> With that things look like before from the cpufreq side, but the other sched
>>> classes also get a chance to trigger a cpufreq update.  The drawback is the
>>> cpu_clock() call instead of passing the time value from update_load_avg(), but
>>> I guess we can live with that if necessary.
>>>
>>> FWIW, this modification doesn't seem to break things on my test machine.
>>>
>> ...
>>> Index: linux-pm/kernel/sched/rt.c
>>> ===================================================================
>>> --- linux-pm.orig/kernel/sched/rt.c
>>> +++ linux-pm/kernel/sched/rt.c
>>> @@ -2212,6 +2212,9 @@ static void task_tick_rt(struct rq *rq,
>>>
>>>       update_curr_rt(rq);
>>>
>>> +     /* Kick cpufreq to prevent it from stalling. */
>>> +     cpufreq_kick();
>>> +
>>>       watchdog(rq, p);
>>>
>>>       /*
>>> Index: linux-pm/kernel/sched/deadline.c
>>> ===================================================================
>>> --- linux-pm.orig/kernel/sched/deadline.c
>>> +++ linux-pm/kernel/sched/deadline.c
>>> @@ -1197,6 +1197,9 @@ static void task_tick_dl(struct rq *rq,
>>>  {
>>>       update_curr_dl(rq);
>>>
>>> +     /* Kick cpufreq to prevent it from stalling. */
>>> +     cpufreq_kick();
>>> +
>>>       /*
>>>        * Even when we have runtime, update_curr_dl() might have resulted in us
>>>        * not being the leftmost task anymore. In that case NEED_RESCHED will
>>
>> I think additional hooks such as enqueue/dequeue would be needed in
>> RT/DL. The task tick callbacks will only run if a task in that class is
>> executing at the time of the tick. There could be intermittent RT/DL
>> task activity in a frequency domain (the only task activity there, no
>> CFS tasks) that doesn't happen to overlap the tick. Worst case the task
>> activity could be periodic in such a way that it never overlaps the tick
>> and the update is never made.
>
> So if I'm reading this correctly, it would be better to put the hooks
> into update_curr_rt/dl()?

If done this way, I guess we may pass rq_clock_task(rq) as the time
arg to cpufreq_update_util() from there and then the cpu_lock() call
I've added to this prototype won't be necessary any more.

Thanks,
Rafael

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

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

On 02/09/2016 07:09 PM, Rafael J. Wysocki wrote:
>>> >> I think additional hooks such as enqueue/dequeue would be needed in
>>> >> RT/DL. The task tick callbacks will only run if a task in that class is
>>> >> executing at the time of the tick. There could be intermittent RT/DL
>>> >> task activity in a frequency domain (the only task activity there, no
>>> >> CFS tasks) that doesn't happen to overlap the tick. Worst case the task
>>> >> activity could be periodic in such a way that it never overlaps the tick
>>> >> and the update is never made.
>> >
>> > So if I'm reading this correctly, it would be better to put the hooks
>> > into update_curr_rt/dl()?

That should AFAICS be sufficient to avoid stalling. It may be more than
is required as that covers more than just enqueue/dequeue but I'm not
sure offhand.

>
> If done this way, I guess we may pass rq_clock_task(rq) as the time
> arg to cpufreq_update_util() from there and then the cpu_lock() call
> I've added to this prototype won't be necessary any more.

Is it rq_clock_task() or rq_clock()? The former can omit irq time so may
gradually fall behind wall clock time, delaying callbacks in cpufreq.

thanks,
Steve

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

#1331513

On Wed, Feb 10, 2016 at 8:47 PM, Steve Muckle <steve.muckle@linaro.org> wrote:
> On 02/09/2016 07:09 PM, Rafael J. Wysocki wrote:
>>>> >> I think additional hooks such as enqueue/dequeue would be needed in
>>>> >> RT/DL. The task tick callbacks will only run if a task in that class is
>>>> >> executing at the time of the tick. There could be intermittent RT/DL
>>>> >> task activity in a frequency domain (the only task activity there, no
>>>> >> CFS tasks) that doesn't happen to overlap the tick. Worst case the task
>>>> >> activity could be periodic in such a way that it never overlaps the tick
>>>> >> and the update is never made.
>>> >
>>> > So if I'm reading this correctly, it would be better to put the hooks
>>> > into update_curr_rt/dl()?
>
> That should AFAICS be sufficient to avoid stalling. It may be more than
> is required as that covers more than just enqueue/dequeue but I'm not
> sure offhand.
>
>>
>> If done this way, I guess we may pass rq_clock_task(rq) as the time
>> arg to cpufreq_update_util() from there and then the cpu_lock() call
>> I've added to this prototype won't be necessary any more.
>
> Is it rq_clock_task() or rq_clock()? The former can omit irq time so may
> gradually fall behind wall clock time, delaying callbacks in cpufreq.

What matters to us is the difference between the current time and the
time we previously took a sample and there shouldn't be too much
difference between the two in that respect.

Both are good enough IMO, but I can update the patch to use rq_clock()
if that's preferred.

Thanks,
Rafael

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

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

On 02/10/2016 01:49 PM, Rafael J. Wysocki wrote:
>>> If done this way, I guess we may pass rq_clock_task(rq) as the time
>>> >> arg to cpufreq_update_util() from there and then the cpu_lock() call
>>> >> I've added to this prototype won't be necessary any more.
>> >
>> > Is it rq_clock_task() or rq_clock()? The former can omit irq time so may
>> > gradually fall behind wall clock time, delaying callbacks in cpufreq.
>
> What matters to us is the difference between the current time and the
> time we previously took a sample and there shouldn't be too much
> difference between the two in that respect.

Sorry, the reference to wall clock time was unnecessary. I just meant it
can lose time, which could cause cpufreq updates to be delayed during
irq heavy periods.

> Both are good enough IMO, but I can update the patch to use rq_clock()
> if that's preferred.

I do believe rq_clock should be used as workloads such as heavy
networking could spend a significant portion of time in interrupts,
skewing rq_clock_task significantly, assuming I understand it correctly.

thanks,
Steve

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

#1331543

On Wed, Feb 10, 2016 at 11:07 PM, Steve Muckle <steve.muckle@linaro.org> wrote:
> On 02/10/2016 01:49 PM, Rafael J. Wysocki wrote:
>>>> If done this way, I guess we may pass rq_clock_task(rq) as the time
>>>> >> arg to cpufreq_update_util() from there and then the cpu_lock() call
>>>> >> I've added to this prototype won't be necessary any more.
>>> >
>>> > Is it rq_clock_task() or rq_clock()? The former can omit irq time so may
>>> > gradually fall behind wall clock time, delaying callbacks in cpufreq.
>>
>> What matters to us is the difference between the current time and the
>> time we previously took a sample and there shouldn't be too much
>> difference between the two in that respect.
>
> Sorry, the reference to wall clock time was unnecessary. I just meant it
> can lose time, which could cause cpufreq updates to be delayed during
> irq heavy periods.
>
>> Both are good enough IMO, but I can update the patch to use rq_clock()
>> if that's preferred.
>
> I do believe rq_clock should be used as workloads such as heavy
> networking could spend a significant portion of time in interrupts,
> skewing rq_clock_task significantly, assuming I understand it correctly.

OK, I'll send an update, then.

Thanks,
Rafael

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

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

On Tue, Feb 09, 2016 at 05:02:33PM -0800, Steve Muckle wrote:
> > Index: linux-pm/kernel/sched/deadline.c
> > ===================================================================
> > --- linux-pm.orig/kernel/sched/deadline.c
> > +++ linux-pm/kernel/sched/deadline.c
> > @@ -1197,6 +1197,9 @@ static void task_tick_dl(struct rq *rq,
> >  {
> >  	update_curr_dl(rq);
> >  
> > +	/* Kick cpufreq to prevent it from stalling. */
> > +	cpufreq_kick();
> > +
> >  	/*
> >  	 * Even when we have runtime, update_curr_dl() might have resulted in us
> >  	 * not being the leftmost task anymore. In that case NEED_RESCHED will
> 
> I think additional hooks such as enqueue/dequeue would be needed in
> RT/DL. The task tick callbacks will only run if a task in that class is
> executing at the time of the tick. There could be intermittent RT/DL
> task activity in a frequency domain (the only task activity there, no
> CFS tasks) that doesn't happen to overlap the tick. Worst case the task
> activity could be periodic in such a way that it never overlaps the tick
> and the update is never made.

No, for RT (RR/FIFO) we do not have enough information to do anything
useful. Basically RR/FIFO should result in running 100% whenever we
schedule such a task.

That means RR/FIFO want a hook in pick_next_task_rt() to bump the freq
to 100% and leave it there until something else gets to run.

For DL it basically wants to set a minimum freq based on reserved
utilization, so that is __setparam_dl() or somewhere around there.

And we should either use CPPC hints for min freq or manually ensure that
the CFS callback will not select something less than this.

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

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

Hi Peter,

On 11/02/16 12:59, Peter Zijlstra wrote:
> On Tue, Feb 09, 2016 at 05:02:33PM -0800, Steve Muckle wrote:
> > > Index: linux-pm/kernel/sched/deadline.c
> > > ===================================================================
> > > --- linux-pm.orig/kernel/sched/deadline.c
> > > +++ linux-pm/kernel/sched/deadline.c
> > > @@ -1197,6 +1197,9 @@ static void task_tick_dl(struct rq *rq,
> > >  {
> > >  	update_curr_dl(rq);
> > >  
> > > +	/* Kick cpufreq to prevent it from stalling. */
> > > +	cpufreq_kick();
> > > +
> > >  	/*
> > >  	 * Even when we have runtime, update_curr_dl() might have resulted in us
> > >  	 * not being the leftmost task anymore. In that case NEED_RESCHED will
> > 
> > I think additional hooks such as enqueue/dequeue would be needed in
> > RT/DL. The task tick callbacks will only run if a task in that class is
> > executing at the time of the tick. There could be intermittent RT/DL
> > task activity in a frequency domain (the only task activity there, no
> > CFS tasks) that doesn't happen to overlap the tick. Worst case the task
> > activity could be periodic in such a way that it never overlaps the tick
> > and the update is never made.
> 
> No, for RT (RR/FIFO) we do not have enough information to do anything
> useful. Basically RR/FIFO should result in running 100% whenever we
> schedule such a task.
> 
> That means RR/FIFO want a hook in pick_next_task_rt() to bump the freq
> to 100% and leave it there until something else gets to run.
> 

Vincent is trying to play with rt_avg (in the last sched-freq thread) to
see if we can get some information about RT as well. I understand that
from a theoretical perspective that's not much we can say of such tasks,
and bumping to max can be the only sensible thing to do, but there are
users of RT (ehm, Android) that will probably see differences in energy
consumption if we do so. Yeah, maybe the should use a different policy,
yes.

> For DL it basically wants to set a minimum freq based on reserved
> utilization, so that is __setparam_dl() or somewhere around there.
> 

I think we could do better than this once Luca's reclaiming stuff gets
in. The reserved bw is usually somewhat pessimistic. But this is a
different discussion, maybe.

Best,

- Juri

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

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

On Thu, Feb 11, 2016 at 12:24:29PM +0000, Juri Lelli wrote:
> Hi Peter,
> 
> On 11/02/16 12:59, Peter Zijlstra wrote:
> > On Tue, Feb 09, 2016 at 05:02:33PM -0800, Steve Muckle wrote:
> > > > Index: linux-pm/kernel/sched/deadline.c
> > > > ===================================================================
> > > > --- linux-pm.orig/kernel/sched/deadline.c
> > > > +++ linux-pm/kernel/sched/deadline.c
> > > > @@ -1197,6 +1197,9 @@ static void task_tick_dl(struct rq *rq,
> > > >  {
> > > >  	update_curr_dl(rq);
> > > >  
> > > > +	/* Kick cpufreq to prevent it from stalling. */
> > > > +	cpufreq_kick();
> > > > +
> > > >  	/*
> > > >  	 * Even when we have runtime, update_curr_dl() might have resulted in us
> > > >  	 * not being the leftmost task anymore. In that case NEED_RESCHED will
> > > 
> > > I think additional hooks such as enqueue/dequeue would be needed in
> > > RT/DL. The task tick callbacks will only run if a task in that class is
> > > executing at the time of the tick. There could be intermittent RT/DL
> > > task activity in a frequency domain (the only task activity there, no
> > > CFS tasks) that doesn't happen to overlap the tick. Worst case the task
> > > activity could be periodic in such a way that it never overlaps the tick
> > > and the update is never made.
> > 
> > No, for RT (RR/FIFO) we do not have enough information to do anything
> > useful. Basically RR/FIFO should result in running 100% whenever we
> > schedule such a task.
> > 
> > That means RR/FIFO want a hook in pick_next_task_rt() to bump the freq
> > to 100% and leave it there until something else gets to run.
> > 
> 
> Vincent is trying to play with rt_avg (in the last sched-freq thread) to
> see if we can get some information about RT as well. I understand that
> from a theoretical perspective that's not much we can say of such tasks,
> and bumping to max can be the only sensible thing to do, but there are
> users of RT (ehm, Android) that will probably see differences in energy
> consumption if we do so. Yeah, maybe the should use a different policy,
> yes.

Can't we just leave broken people get broken results? Trying to use
rt_avg for this is just insane. We should ensure that people using this
thing correctly get correct results, the rest can take a hike.

Using rt_avg gets us to the place where people who want to do the right
thing cannot, and that is bad.

> > For DL it basically wants to set a minimum freq based on reserved
> > utilization, so that is __setparam_dl() or somewhere around there.
> > 
> 
> I think we could do better than this once Luca's reclaiming stuff gets
> in. The reserved bw is usually somewhat pessimistic. But this is a
> different discussion, maybe.

Sure, there's cleverer things that can be done. But a simple one would
indeed be the min guarantee based on accepted bandwidth.

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

#1332284

On 11 February 2016 at 16:26, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Feb 11, 2016 at 12:24:29PM +0000, Juri Lelli wrote:
>> Hi Peter,
>>
>> On 11/02/16 12:59, Peter Zijlstra wrote:
>> >
>> > No, for RT (RR/FIFO) we do not have enough information to do anything
>> > useful. Basically RR/FIFO should result in running 100% whenever we
>> > schedule such a task.
>> >
>> > That means RR/FIFO want a hook in pick_next_task_rt() to bump the freq
>> > to 100% and leave it there until something else gets to run.
>> >
>>
>> Vincent is trying to play with rt_avg (in the last sched-freq thread) to
>> see if we can get some information about RT as well. I understand that
>> from a theoretical perspective that's not much we can say of such tasks,
>> and bumping to max can be the only sensible thing to do, but there are
>> users of RT (ehm, Android) that will probably see differences in energy
>> consumption if we do so. Yeah, maybe the should use a different policy,
>> yes.
>
> Can't we just leave broken people get broken results? Trying to use
> rt_avg for this is just insane. We should ensure that people using this
> thing correctly get correct results, the rest can take a hike.
>
> Using rt_avg gets us to the place where people who want to do the right
> thing cannot, and that is bad.

I agree that using rt_avg is not the best choice to evaluate the
capacity that is used by RT tasks but it has the advantage of been
already there. Do you mean that we should use another way to compute
the capacity that is used by rt tasks to then select the frequency  ?
Or do you mean that we can't do anything else than asking for max
frequency ?

Trying to set max frequency just before scheduling RT task is not
really doable on a lot of platform because the sequence that changes
the frequency can sleep and takes more time than the run time of the
task. At the end, we will have set max frequency once the task has
finished to run. There is no other solution than increasing the
min_freq of cpufreq to a level that will ensure enough compute
capacity for RT task with such high constraints that cpufreq can't
react. For other RT tasks, we can probably found a way to set a
frequency that can fit both RT constraints and power consumption.

>
>> > For DL it basically wants to set a minimum freq based on reserved
>> > utilization, so that is __setparam_dl() or somewhere around there.
>> >
>>
>> I think we could do better than this once Luca's reclaiming stuff gets
>> in. The reserved bw is usually somewhat pessimistic. But this is a
>> different discussion, maybe.
>
> Sure, there's cleverer things that can be done. But a simple one would
> indeed be the min guarantee based on accepted bandwidth.
> --
> 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]

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

On Thu, Feb 11, 2016 at 07:23:55PM +0100, Vincent Guittot wrote:
> I agree that using rt_avg is not the best choice to evaluate the
> capacity that is used by RT tasks but it has the advantage of been
> already there. Do you mean that we should use another way to compute
> the capacity that is used by rt tasks to then select the frequency  ?

Nope, RR/FIFO simply do not contain enough information to compute
anything from.

> Or do you mean that we can't do anything else than asking for max
> frequency ?

Yep.

> Trying to set max frequency just before scheduling RT task is not
> really doable on a lot of platform because the sequence that changes
> the frequency can sleep and takes more time than the run time of the
> task.

So what people do today is shoot cpufreq in the head and not use it,
maybe that's the 'right' thing on these platforms.

> At the end, we will have set max frequency once the task has
> finished to run. There is no other solution than increasing the
> min_freq of cpufreq to a level that will ensure enough compute
> capacity for RT task with such high constraints that cpufreq can't
> react.

But you cannot a priori tell how much time RR/FIFO tasks will require,
that's the entire problem with them. We can compute a hysterical
average, but that _will_ mis predict the future and get you
underruns/deadline misses.

> For other RT tasks, we can probably found a way to set a
> frequency that can fit both RT constraints and power consumption.

You cannot, not without adding a lot more information about what these
tasks are doing, and that is not captured in the task model.

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

#1332788

On 12 February 2016 at 15:04, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Feb 11, 2016 at 07:23:55PM +0100, Vincent Guittot wrote:
>> I agree that using rt_avg is not the best choice to evaluate the
>> capacity that is used by RT tasks but it has the advantage of been
>> already there. Do you mean that we should use another way to compute
>> the capacity that is used by rt tasks to then select the frequency  ?
>
> Nope, RR/FIFO simply do not contain enough information to compute
> anything from.
>
>> Or do you mean that we can't do anything else than asking for max
>> frequency ?
>
> Yep.
>
>> Trying to set max frequency just before scheduling RT task is not
>> really doable on a lot of platform because the sequence that changes
>> the frequency can sleep and takes more time than the run time of the
>> task.
>
> So what people do today is shoot cpufreq in the head and not use it,
> maybe that's the 'right' thing on these platforms.
>
>> At the end, we will have set max frequency once the task has
>> finished to run. There is no other solution than increasing the
>> min_freq of cpufreq to a level that will ensure enough compute
>> capacity for RT task with such high constraints that cpufreq can't
>> react.
>
> But you cannot a priori tell how much time RR/FIFO tasks will require,
> that's the entire problem with them. We can compute a hysterical
> average, but that _will_ mis predict the future and get you
> underruns/deadline misses.
>
>> For other RT tasks, we can probably found a way to set a
>> frequency that can fit both RT constraints and power consumption.
>
> You cannot, not without adding a lot more information about what these
> tasks are doing, and that is not captured in the task model.

Another point to take into account is that the RT tasks will "steal"
the compute capacity that has been requested by the cfs tasks.

Let takes the example of a CPU with 3 OPP on which run 2 rt tasks A
and B and 1 cfs task C.
Let assume that the real time constraint of RT task A is too agressive
for the lowest OPP0 and that the change of the frequency of the core
is too slow compare to this constraint but the real time constraint of
RT task B can be handle whatever the OPP. System don't have other
choice than setting the cpufreq min freq to OPP1 to be sure that
constraint of task A will be covered at anytime. Then, we still have 2
possible OPPs. The CFS task asks for compute capacity that fits in
OPP1 but a part of this capacity will be stolen by RT tasks. If we
monitor the load of RT tasks and request capacity for these RT tasks
according to their current utilization, we can decide to switch to
highest OPP2 to ensure that task C will have enough remaining
capacity. A lot of embedded platform faces such kind of use cases

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

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

Hi Peter,

On 02/11/2016 03:59 AM, Peter Zijlstra wrote:
>> I think additional hooks such as enqueue/dequeue would be needed in
>> > RT/DL. The task tick callbacks will only run if a task in that class is
>> > executing at the time of the tick. There could be intermittent RT/DL
>> > task activity in a frequency domain (the only task activity there, no
>> > CFS tasks) that doesn't happen to overlap the tick. Worst case the task
>> > activity could be periodic in such a way that it never overlaps the tick
>> > and the update is never made.
>
> No, for RT (RR/FIFO) we do not have enough information to do anything
> useful. Basically RR/FIFO should result in running 100% whenever we
> schedule such a task.
> 
> That means RR/FIFO want a hook in pick_next_task_rt() to bump the freq
> to 100% and leave it there until something else gets to run.
>
> For DL it basically wants to set a minimum freq based on reserved
> utilization, so that is __setparam_dl() or somewhere around there.
> 
> And we should either use CPPC hints for min freq or manually ensure that
> the CFS callback will not select something less than this.

Rafael's changes aren't specifying particular frequencies/capacities in
the scheduler hooks. They're just pokes to get cpufreq to run, in order
to eliminate cpufreq's timers.

My concern above is that pokes are guaranteed to keep occurring when
there is only RT or DL activity so nothing breaks.

thanks,
Steve

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

#1332259

On Thu, Feb 11, 2016 at 6:30 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Feb 11, 2016 at 09:06:04AM -0800, Steve Muckle wrote:
>> Hi Peter,
>>
>> >> > I think additional hooks such as enqueue/dequeue would be needed in
>> >> > RT/DL.
>
> That is what I reacted to mostly. Enqueue/dequeue hooks don't really
> make much sense for RT / DL.
>
>> Rafael's changes aren't specifying particular frequencies/capacities in
>> the scheduler hooks. They're just pokes to get cpufreq to run, in order
>> to eliminate cpufreq's timers.
>>
>> My concern above is that pokes are guaranteed to keep occurring when
>> there is only RT or DL activity so nothing breaks.
>
> The hook in their respective tick handler should ensure stuff is called
> sporadically and isn't stalled.

I've updated the patch in the meantime
(https://patchwork.kernel.org/patch/8283431/).

Should I move the RT/DL hooks to task_tick_rt/dl(), respectively?

Thanks,
Rafael

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

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

On Thu, Feb 11, 2016 at 06:34:05PM +0100, Rafael J. Wysocki wrote:
> I've updated the patch in the meantime
> (https://patchwork.kernel.org/patch/8283431/).
> 
> Should I move the RT/DL hooks to task_tick_rt/dl(), respectively?

Probably, this really is about kicking cpufreq to do something, right?
update_curr_*() seems overkill for that.

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

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

On Thu, Feb 11, 2016 at 09:06:04AM -0800, Steve Muckle wrote:
> Hi Peter,
> 
> >> > I think additional hooks such as enqueue/dequeue would be needed in
> >> > RT/DL.

That is what I reacted to mostly. Enqueue/dequeue hooks don't really
make much sense for RT / DL.

> Rafael's changes aren't specifying particular frequencies/capacities in
> the scheduler hooks. They're just pokes to get cpufreq to run, in order
> to eliminate cpufreq's timers.
> 
> My concern above is that pokes are guaranteed to keep occurring when
> there is only RT or DL activity so nothing breaks.

The hook in their respective tick handler should ensure stuff is called
sporadically and isn't stalled.

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

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

On 02/11/2016 09:30 AM, Peter Zijlstra wrote:
>> My concern above is that pokes are guaranteed to keep occurring when
>> > there is only RT or DL activity so nothing breaks.
>
> The hook in their respective tick handler should ensure stuff is called
> sporadically and isn't stalled.

But that's only true if the RT/DL tasks happen to be running when the
tick arrives right?

Couldn't we have RT/DL activity which doesn't overlap with the tick? And
if no CFS tasks happen to be executing on that CPU, we'll never trigger
the cpufreq update. This could go on for an arbitrarily long time
depending on the periodicity of the work.

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

#1332323

On Thu, Feb 11, 2016 at 7:52 PM, Steve Muckle <steve.muckle@linaro.org> wrote:
> On 02/11/2016 09:30 AM, Peter Zijlstra wrote:
>>> My concern above is that pokes are guaranteed to keep occurring when
>>> > there is only RT or DL activity so nothing breaks.
>>
>> The hook in their respective tick handler should ensure stuff is called
>> sporadically and isn't stalled.
>
> But that's only true if the RT/DL tasks happen to be running when the
> tick arrives right?
>
> Couldn't we have RT/DL activity which doesn't overlap with the tick? And
> if no CFS tasks happen to be executing on that CPU, we'll never trigger
> the cpufreq update. This could go on for an arbitrarily long time
> depending on the periodicity of the work.

I'm thinking that two additional hooks in enqueue_task_rt/dl() might
help here.  Then, we will hit either the tick or enqueue and that
should do the trick.

Peter, what do you think?

Rafael

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

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web