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


Groups > linux.kernel > #1362238 > unrolled thread

[PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()

Started bySteve Muckle <steve.muckle@linaro.org>
First post2016-03-22 01:30 +0100
Last post2016-03-31 15:00 +0200
Articles 15 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg() Steve Muckle <steve.muckle@linaro.org> - 2016-03-22 01:30 +0100
    Re: [PATCH 1/2] sched/fair: move cpufreq hook to  update_cfs_rq_load_avg() Steve Muckle <steve.muckle@linaro.org> - 2016-03-28 18:40 +0200
      Re: [PATCH 1/2] sched/fair: move cpufreq hook to  update_cfs_rq_load_avg() Steve Muckle <steve.muckle@linaro.org> - 2016-03-28 21:40 +0200
        Re: [PATCH 1/2] sched/fair: move cpufreq hook to  update_cfs_rq_load_avg() Peter Zijlstra <peterz@infradead.org> - 2016-03-30 21:40 +0200
          Re: [PATCH 1/2] sched/fair: move cpufreq hook to  update_cfs_rq_load_avg() Steve Muckle <steve.muckle@linaro.org> - 2016-03-31 03:50 +0200
            Re: [PATCH 1/2] sched/fair: move cpufreq hook to  update_cfs_rq_load_avg() Peter Zijlstra <peterz@infradead.org> - 2016-03-31 09:40 +0200
              Re: [PATCH 1/2] sched/fair: move cpufreq hook to  update_cfs_rq_load_avg() Steve Muckle <steve.muckle@linaro.org> - 2016-03-31 23:30 +0200
                Re: [PATCH 1/2] sched/fair: move cpufreq hook to  update_cfs_rq_load_avg() Peter Zijlstra <peterz@infradead.org> - 2016-04-01 11:30 +0200
          Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg() Vincent Guittot <vincent.guittot@linaro.org> - 2016-03-31 11:30 +0200
            Re: [PATCH 1/2] sched/fair: move cpufreq hook to  update_cfs_rq_load_avg() Peter Zijlstra <peterz@infradead.org> - 2016-03-31 11:40 +0200
              Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg() Vincent Guittot <vincent.guittot@linaro.org> - 2016-03-31 12:00 +0200
                Re: [PATCH 1/2] sched/fair: move cpufreq hook to  update_cfs_rq_load_avg() Peter Zijlstra <peterz@infradead.org> - 2016-03-31 12:50 +0200
                  Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg() Vincent Guittot <vincent.guittot@linaro.org> - 2016-03-31 14:20 +0200
                    Re: [PATCH 1/2] sched/fair: move cpufreq hook to  update_cfs_rq_load_avg() Peter Zijlstra <peterz@infradead.org> - 2016-03-31 14:40 +0200
                      Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg() Vincent Guittot <vincent.guittot@linaro.org> - 2016-03-31 15:00 +0200

#1362238 — [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()

FromSteve Muckle <steve.muckle@linaro.org>
Date2016-03-22 01:30 +0100
Subject[PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()
Message-ID<rfimR-4U6-13@gated-at.bofh.it>
The cpufreq hook should be called whenever the root cfs_rq
utilization changes so update_cfs_rq_load_avg() is a better
place for it. The current location is not invoked in the
enqueue_entity() or update_blocked_averages() paths.

Suggested-by: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Steve Muckle <smuckle@linaro.org>
---
 kernel/sched/fair.c | 50 ++++++++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 46d64e4ccfde..d418deb04049 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2825,7 +2825,9 @@ static inline u64 cfs_rq_clock_task(struct cfs_rq *cfs_rq);
 static inline int update_cfs_rq_load_avg(u64 now, struct cfs_rq *cfs_rq)
 {
 	struct sched_avg *sa = &cfs_rq->avg;
+	struct rq *rq = rq_of(cfs_rq);
 	int decayed, removed = 0;
+	int cpu = cpu_of(rq);
 
 	if (atomic_long_read(&cfs_rq->removed_load_avg)) {
 		s64 r = atomic_long_xchg(&cfs_rq->removed_load_avg, 0);
@@ -2840,7 +2842,7 @@ static inline int update_cfs_rq_load_avg(u64 now, struct cfs_rq *cfs_rq)
 		sa->util_sum = max_t(s32, sa->util_sum - r * LOAD_AVG_MAX, 0);
 	}
 
-	decayed = __update_load_avg(now, cpu_of(rq_of(cfs_rq)), sa,
+	decayed = __update_load_avg(now, cpu, sa,
 		scale_load_down(cfs_rq->load.weight), cfs_rq->curr != NULL, cfs_rq);
 
 #ifndef CONFIG_64BIT
@@ -2848,28 +2850,6 @@ static inline int update_cfs_rq_load_avg(u64 now, struct cfs_rq *cfs_rq)
 	cfs_rq->load_last_update_time_copy = sa->last_update_time;
 #endif
 
-	return decayed || removed;
-}
-
-/* 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);
-	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
-	 * track group sched_entity load average for task_h_load calc in migration
-	 */
-	__update_load_avg(now, cpu, &se->avg,
-			  se->on_rq * scale_load_down(se->load.weight),
-			  cfs_rq->curr == se, NULL);
-
-	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;
 
@@ -2890,8 +2870,30 @@ static inline void update_load_avg(struct sched_entity *se, int update_tg)
 		 * See cpu_util().
 		 */
 		cpufreq_update_util(rq_clock(rq),
-				    min(cfs_rq->avg.util_avg, max), max);
+				    min(sa->util_avg, max), max);
 	}
+
+	return decayed || removed;
+}
+
+/* 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);
+	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
+	 * track group sched_entity load average for task_h_load calc in migration
+	 */
+	__update_load_avg(now, cpu, &se->avg,
+			  se->on_rq * scale_load_down(se->load.weight),
+			  cfs_rq->curr == se, NULL);
+
+	if (update_cfs_rq_load_avg(now, cfs_rq) && update_tg)
+		update_tg_load_avg(cfs_rq, 0);
 }
 
 static void attach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
-- 
2.4.10

[toc] | [next] | [standalone]


#1365576 — Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()

FromSteve Muckle <steve.muckle@linaro.org>
Date2016-03-28 18:40 +0200
SubjectRe: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()
Message-ID<rhImS-1aq-15@gated-at.bofh.it>
In reply to#1362238
Hi Dietmar,

On 03/28/2016 05:02 AM, Dietmar Eggemann wrote:
> Hi Steve,
> 
> these patches fall into the bucket of 'optimization of updating the
> value only if the root cfs_rq util has changed' as discussed in '[PATCH
> 5/8] sched/cpufreq: pass sched class into cpufreq_update_util' of Mike
> T's current series '[PATCH 0/8] schedutil enhancements', right?

I would say just the second patch is an optimization. The first and
third patches cover additional paths in CFS where the hook should be
called but currently is not, which I think is a correctness issue.

> I wonder if it makes sense to apply them before a proper 'capacity vote
> aggregation from CFS/RT/DL' has been agreed upon?

Getting the right call sites for the hook in CFS should be orthogonal to
the sched class vote aggregation IMO.

> Otherwise I agree with the changes in your 3 patches (inc. "[RFC PATCH]
> sched/fair: call cpufreq hook in additional paths") to only invoke
> cpufreq_update_util() if &rq->cfs.avg.util_avg has really changed.
> 
> 
> On 03/22/2016 01:21 AM, Steve Muckle wrote:
>> The cpufreq hook should be called whenever the root cfs_rq
>> utilization changes so update_cfs_rq_load_avg() is a better
>> place for it. The current location is not invoked in the
>> enqueue_entity() or update_blocked_averages() paths.
>>
>> Suggested-by: Vincent Guittot <vincent.guittot@linaro.org>
>> Signed-off-by: Steve Muckle <smuckle@linaro.org>
>> ---
>>   kernel/sched/fair.c | 50
>> ++++++++++++++++++++++++++------------------------
>>   1 file changed, 26 insertions(+), 24 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 46d64e4ccfde..d418deb04049 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -2825,7 +2825,9 @@ static inline u64 cfs_rq_clock_task(struct
>> cfs_rq *cfs_rq);
>>   static inline int update_cfs_rq_load_avg(u64 now, struct cfs_rq
>> *cfs_rq)
>>   {
>>       struct sched_avg *sa = &cfs_rq->avg;
>> +    struct rq *rq = rq_of(cfs_rq);
>>       int decayed, removed = 0;
>> +    int cpu = cpu_of(rq);
>>
>>       if (atomic_long_read(&cfs_rq->removed_load_avg)) {
>>           s64 r = atomic_long_xchg(&cfs_rq->removed_load_avg, 0);
>> @@ -2840,7 +2842,7 @@ static inline int update_cfs_rq_load_avg(u64
>> now, struct cfs_rq *cfs_rq)
>>           sa->util_sum = max_t(s32, sa->util_sum - r * LOAD_AVG_MAX, 0);
>>       }
>>
>> -    decayed = __update_load_avg(now, cpu_of(rq_of(cfs_rq)), sa,
>> +    decayed = __update_load_avg(now, cpu, sa,
>>           scale_load_down(cfs_rq->load.weight), cfs_rq->curr != NULL,
>> cfs_rq);
> 
> Why did you change these 3 lines above? You reverted this back in "[RFC
> PATCH] sched/fair: call cpufreq hook in additional paths".

If all three patches are accepted in principle I can restructure them if
so desired. I did not want to introduce a dependency between them and
patch 2 represents the cleanest implementation at that point.

Thanks for the review!

thanks,
Steve

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


#1365645 — Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()

FromSteve Muckle <steve.muckle@linaro.org>
Date2016-03-28 21:40 +0200
SubjectRe: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()
Message-ID<rhLb3-36F-11@gated-at.bofh.it>
In reply to#1365576
On 03/28/2016 11:30 AM, Dietmar Eggemann wrote:
> On 03/28/2016 06:34 PM, Steve Muckle wrote:
>> Hi Dietmar,
>>
>> On 03/28/2016 05:02 AM, Dietmar Eggemann wrote:
>>> Hi Steve,
>>>
>>> these patches fall into the bucket of 'optimization of updating the
>>> value only if the root cfs_rq util has changed' as discussed in '[PATCH
>>> 5/8] sched/cpufreq: pass sched class into cpufreq_update_util' of Mike
>>> T's current series '[PATCH 0/8] schedutil enhancements', right?
>>
>> I would say just the second patch is an optimization. The first and
>> third patches cover additional paths in CFS where the hook should be
>> called but currently is not, which I think is a correctness issue.
> 
> Not disagreeing here but I don't know if this level of accuracy is
> really needed. I mean we currently miss updates in
> enqueue_task_fair()->enqueue_entity()->enqueue_entity_load_avg() and
> idle_balance()/rebalance_domains()->update_blocked_averages() but there
> are plenty of call sides of update_load_avg(se, ...) with
> '&rq_of(cfs_rq_of(se))->cfs == cfs_rq_of(se)'.
>
> The question for me is does schedutil work better with this new, more
> accurate signal? IMO, not receiving a bunch of consecutive
> cpufreq_update_util's w/ the same 'util' value is probably a good thing,
> unless we see the interaction with RT/DL class as mentioned by Sai. Here
> an agreement on the design for the 'capacity vote aggregation from
> CFS/RT/DL' would help to clarify.

Without covering all the paths where CFS utilization changes it's
possible to have to wait up to a tick to act on some changes, since the
tick is the only guaranteed regularly-occurring instance of the hook.
That's an unacceptable amount of latency IMO...

thanks,
Steve

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


#1367535 — Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()

FromPeter Zijlstra <peterz@infradead.org>
Date2016-03-30 21:40 +0200
SubjectRe: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()
Message-ID<riu89-1FR-7@gated-at.bofh.it>
In reply to#1365645
On Mon, Mar 28, 2016 at 12:38:26PM -0700, Steve Muckle wrote:
> Without covering all the paths where CFS utilization changes it's
> possible to have to wait up to a tick to act on some changes, since the
> tick is the only guaranteed regularly-occurring instance of the hook.
> That's an unacceptable amount of latency IMO...

Note that even with your patches that might still be the case. Remote
wakeups might not happen on the destination CPU at all, so it might not
be until the next tick (which always happens locally) that we'll
'observe' the utilization change brought with the wakeups.

We could force all the remote wakeups to IPI the destination CPU, but
that comes at a significant performance cost.

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


#1367745 — Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()

FromSteve Muckle <steve.muckle@linaro.org>
Date2016-03-31 03:50 +0200
SubjectRe: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()
Message-ID<rizUe-5Lv-15@gated-at.bofh.it>
In reply to#1367535
On 03/30/2016 12:35 PM, Peter Zijlstra wrote:
> On Mon, Mar 28, 2016 at 12:38:26PM -0700, Steve Muckle wrote:
>> Without covering all the paths where CFS utilization changes it's
>> possible to have to wait up to a tick to act on some changes, since the
>> tick is the only guaranteed regularly-occurring instance of the hook.
>> That's an unacceptable amount of latency IMO...
> 
> Note that even with your patches that might still be the case. Remote
> wakeups might not happen on the destination CPU at all, so it might not
> be until the next tick (which always happens locally) that we'll
> 'observe' the utilization change brought with the wakeups.
> 
> We could force all the remote wakeups to IPI the destination CPU, but
> that comes at a significant performance cost.

What about only IPI'ing the destination when the utilization change is
known to require a higher CPU frequency?

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


#1367914 — Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()

FromPeter Zijlstra <peterz@infradead.org>
Date2016-03-31 09:40 +0200
SubjectRe: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()
Message-ID<riFmW-1xL-31@gated-at.bofh.it>
In reply to#1367745
On Wed, Mar 30, 2016 at 06:42:20PM -0700, Steve Muckle wrote:
> On 03/30/2016 12:35 PM, Peter Zijlstra wrote:
> > On Mon, Mar 28, 2016 at 12:38:26PM -0700, Steve Muckle wrote:
> >> Without covering all the paths where CFS utilization changes it's
> >> possible to have to wait up to a tick to act on some changes, since the
> >> tick is the only guaranteed regularly-occurring instance of the hook.
> >> That's an unacceptable amount of latency IMO...
> > 
> > Note that even with your patches that might still be the case. Remote
> > wakeups might not happen on the destination CPU at all, so it might not
> > be until the next tick (which always happens locally) that we'll
> > 'observe' the utilization change brought with the wakeups.
> > 
> > We could force all the remote wakeups to IPI the destination CPU, but
> > that comes at a significant performance cost.
> 
> What about only IPI'ing the destination when the utilization change is
> known to require a higher CPU frequency?

Can't, the way the wakeup path is constructed we would be sending the
IPI way before we know about utilization.

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


#1368828 — Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()

FromSteve Muckle <steve.muckle@linaro.org>
Date2016-03-31 23:30 +0200
SubjectRe: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()
Message-ID<riSkb-2AW-39@gated-at.bofh.it>
In reply to#1367914
On 03/31/2016 12:37 AM, Peter Zijlstra wrote:
> On Wed, Mar 30, 2016 at 06:42:20PM -0700, Steve Muckle wrote:
>> On 03/30/2016 12:35 PM, Peter Zijlstra wrote:
>>> On Mon, Mar 28, 2016 at 12:38:26PM -0700, Steve Muckle wrote:
>>>> Without covering all the paths where CFS utilization changes it's
>>>> possible to have to wait up to a tick to act on some changes, since the
>>>> tick is the only guaranteed regularly-occurring instance of the hook.
>>>> That's an unacceptable amount of latency IMO...
>>>
>>> Note that even with your patches that might still be the case. Remote
>>> wakeups might not happen on the destination CPU at all, so it might not
>>> be until the next tick (which always happens locally) that we'll
>>> 'observe' the utilization change brought with the wakeups.
>>>
>>> We could force all the remote wakeups to IPI the destination CPU, but
>>> that comes at a significant performance cost.
>>
>> What about only IPI'ing the destination when the utilization change is
>> known to require a higher CPU frequency?
> 
> Can't, the way the wakeup path is constructed we would be sending the
> IPI way before we know about utilization.

Sorry I thought we were referring to the possibility of sending an IPI
to just run the cpufreq driver rather than to conduct the whole wakeup
operation.

My thinking was in CFS we get rid of the (cpu == smp_processor_id())
condition for calling the cpufreq hook.

The sched governor can then calculate utilization and frequency required
for cpu. If (cpu == smp_processor_id()), the update is processed
normally. If (cpu != smp_processor_id()) and the new frequency is higher
than cpu's Fcur, the sched gov IPIs cpu to continue running the update
operation. Otherwise, the update is dropped.

Does that sound plausible?

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


#1369127 — Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()

FromPeter Zijlstra <peterz@infradead.org>
Date2016-04-01 11:30 +0200
SubjectRe: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()
Message-ID<rj3yW-2hY-13@gated-at.bofh.it>
In reply to#1368828
On Thu, Mar 31, 2016 at 02:26:06PM -0700, Steve Muckle wrote:
> > Can't, the way the wakeup path is constructed we would be sending the
> > IPI way before we know about utilization.
> 
> Sorry I thought we were referring to the possibility of sending an IPI
> to just run the cpufreq driver rather than to conduct the whole wakeup
> operation.
> 
> My thinking was in CFS we get rid of the (cpu == smp_processor_id())
> condition for calling the cpufreq hook.
> 
> The sched governor can then calculate utilization and frequency required
> for cpu. If (cpu == smp_processor_id()), the update is processed
> normally. If (cpu != smp_processor_id()) and the new frequency is higher
> than cpu's Fcur, the sched gov IPIs cpu to continue running the update
> operation. Otherwise, the update is dropped.
> 
> Does that sound plausible?

Can be done I suppose..

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


#1368078

FromVincent Guittot <vincent.guittot@linaro.org>
Date2016-03-31 11:30 +0200
Message-ID<riH5p-2PS-45@gated-at.bofh.it>
In reply to#1367535
On 30 March 2016 at 21:35, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Mar 28, 2016 at 12:38:26PM -0700, Steve Muckle wrote:
>> Without covering all the paths where CFS utilization changes it's
>> possible to have to wait up to a tick to act on some changes, since the
>> tick is the only guaranteed regularly-occurring instance of the hook.
>> That's an unacceptable amount of latency IMO...
>
> Note that even with your patches that might still be the case. Remote
> wakeups might not happen on the destination CPU at all, so it might not
> be until the next tick (which always happens locally) that we'll
> 'observe' the utilization change brought with the wakeups.
>
> We could force all the remote wakeups to IPI the destination CPU, but
> that comes at a significant performance cost.

Isn't a reschedule ipi already sent in this case ?

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


#1368097 — Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()

FromPeter Zijlstra <peterz@infradead.org>
Date2016-03-31 11:40 +0200
SubjectRe: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()
Message-ID<riHf4-2VE-23@gated-at.bofh.it>
In reply to#1368078
On Thu, Mar 31, 2016 at 11:27:22AM +0200, Vincent Guittot wrote:
> On 30 March 2016 at 21:35, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Mon, Mar 28, 2016 at 12:38:26PM -0700, Steve Muckle wrote:
> >> Without covering all the paths where CFS utilization changes it's
> >> possible to have to wait up to a tick to act on some changes, since the
> >> tick is the only guaranteed regularly-occurring instance of the hook.
> >> That's an unacceptable amount of latency IMO...
> >
> > Note that even with your patches that might still be the case. Remote
> > wakeups might not happen on the destination CPU at all, so it might not
> > be until the next tick (which always happens locally) that we'll
> > 'observe' the utilization change brought with the wakeups.
> >
> > We could force all the remote wakeups to IPI the destination CPU, but
> > that comes at a significant performance cost.
> 
> Isn't a reschedule ipi already sent in this case ?

In what case? Assuming you talk about a remove wakeup, no. Only if that
wakeup results in a preemption, which isn't a given.

And we really don't want to carry the 'has util increased' information
all the way down to where we make that decision.

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


#1368125

FromVincent Guittot <vincent.guittot@linaro.org>
Date2016-03-31 12:00 +0200
Message-ID<riHyq-32C-17@gated-at.bofh.it>
In reply to#1368097
On 31 March 2016 at 11:34, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Mar 31, 2016 at 11:27:22AM +0200, Vincent Guittot wrote:
>> On 30 March 2016 at 21:35, Peter Zijlstra <peterz@infradead.org> wrote:
>> > On Mon, Mar 28, 2016 at 12:38:26PM -0700, Steve Muckle wrote:
>> >> Without covering all the paths where CFS utilization changes it's
>> >> possible to have to wait up to a tick to act on some changes, since the
>> >> tick is the only guaranteed regularly-occurring instance of the hook.
>> >> That's an unacceptable amount of latency IMO...
>> >
>> > Note that even with your patches that might still be the case. Remote
>> > wakeups might not happen on the destination CPU at all, so it might not
>> > be until the next tick (which always happens locally) that we'll
>> > 'observe' the utilization change brought with the wakeups.
>> >
>> > We could force all the remote wakeups to IPI the destination CPU, but
>> > that comes at a significant performance cost.
>>
>> Isn't a reschedule ipi already sent in this case ?
>
> In what case? Assuming you talk about a remove wakeup, no. Only if that
> wakeup results in a preemption, which isn't a given.

yes, i was speaking about a remote wakeup.
In the ttwu_queue_remote,  there is a call to smp_send_reschedule. Is
there another way to add a remote task in the wake list ?

>
> And we really don't want to carry the 'has util increased' information
> all the way down to where we make that decision.

yes i agree

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


#1368168 — Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()

FromPeter Zijlstra <peterz@infradead.org>
Date2016-03-31 12:50 +0200
SubjectRe: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()
Message-ID<riIkN-3DO-11@gated-at.bofh.it>
In reply to#1368125
On Thu, Mar 31, 2016 at 11:50:40AM +0200, Vincent Guittot wrote:
> > In what case? Assuming you talk about a remove wakeup, no. Only if that
> > wakeup results in a preemption, which isn't a given.
> 
> yes, i was speaking about a remote wakeup.
> In the ttwu_queue_remote,  there is a call to smp_send_reschedule. Is
> there another way to add a remote task in the wake list ?

Right, but at that point we don't yet know how much util will change.

And doing that IPI unconditionally is expensive; see:

  518cd6234178 ("sched: Only queue remote wakeups when crossing cache boundaries")

13% regression on TCP_RR.

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


#1368234

FromVincent Guittot <vincent.guittot@linaro.org>
Date2016-03-31 14:20 +0200
Message-ID<riJJU-4OT-13@gated-at.bofh.it>
In reply to#1368168
On 31 March 2016 at 12:47, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Mar 31, 2016 at 11:50:40AM +0200, Vincent Guittot wrote:
>> > In what case? Assuming you talk about a remove wakeup, no. Only if that
>> > wakeup results in a preemption, which isn't a given.
>>
>> yes, i was speaking about a remote wakeup.
>> In the ttwu_queue_remote,  there is a call to smp_send_reschedule. Is
>> there another way to add a remote task in the wake list ?
>
> Right, but at that point we don't yet know how much util will change.
>
> And doing that IPI unconditionally is expensive; see:
>
>   518cd6234178 ("sched: Only queue remote wakeups when crossing cache boundaries")
>
> 13% regression on TCP_RR.

Ok.
In fact, I looks for the sequence where the utilization of a rq is not
updated until the next tick but i can't find it.
If cpu doesn't share cache, task is added to wake list and an ipi is
sent and the utilization. Otherwise, we directly enqueue the task on
the rq and the utilization is updated

Is there another path ?

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


#1368255 — Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()

FromPeter Zijlstra <peterz@infradead.org>
Date2016-03-31 14:40 +0200
SubjectRe: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()
Message-ID<riK3h-531-45@gated-at.bofh.it>
In reply to#1368234
On Thu, Mar 31, 2016 at 02:14:50PM +0200, Vincent Guittot wrote:
> In fact, I looks for the sequence where the utilization of a rq is not
> updated until the next tick but i can't find it.

No, util it always updated, however..

> If cpu doesn't share cache, task is added to wake list and an ipi is
> sent and the utilization. 

Here we run:

 ttwu_do_activate()
   ttwu_activate()
     activate_task()
       enqueue_task()
         p->sched_class->enqueue_task() := enqueue_task_fair()
	   update_load_avg()
	     update_cfs_rq_load_avg()
	       cfs_rq_util_change()

On the local cpu, and we can indeed call out to have the frequency
changed.

> Otherwise, we directly enqueue the task on
> the rq and the utilization is updated

But here we run it on a remote cpu, so we cannot call out and the
frequency remains the same.

So if a remote wakeup on the same LLC domain happens, utilization will
increase but we will not observe until the next tick.

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


#1368277

FromVincent Guittot <vincent.guittot@linaro.org>
Date2016-03-31 15:00 +0200
Message-ID<riKmD-5a4-35@gated-at.bofh.it>
In reply to#1368255
On 31 March 2016 at 14:34, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Mar 31, 2016 at 02:14:50PM +0200, Vincent Guittot wrote:
>> In fact, I looks for the sequence where the utilization of a rq is not
>> updated until the next tick but i can't find it.
>
> No, util it always updated, however..
>
>> If cpu doesn't share cache, task is added to wake list and an ipi is
>> sent and the utilization.
>
> Here we run:
>
>  ttwu_do_activate()
>    ttwu_activate()
>      activate_task()
>        enqueue_task()
>          p->sched_class->enqueue_task() := enqueue_task_fair()
>            update_load_avg()
>              update_cfs_rq_load_avg()
>                cfs_rq_util_change()
>
> On the local cpu, and we can indeed call out to have the frequency
> changed.
>
>> Otherwise, we directly enqueue the task on
>> the rq and the utilization is updated
>
> But here we run it on a remote cpu, so we cannot call out and the
> frequency remains the same.
>
> So if a remote wakeup on the same LLC domain happens, utilization will
> increase but we will not observe until the next tick.

ok. I forgot that we have the condition cpu == smp_processor_id() in
cfs_rq_util_change.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web