Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1362238 > unrolled thread
| Started by | Steve Muckle <steve.muckle@linaro.org> |
|---|---|
| First post | 2016-03-22 01:30 +0100 |
| Last post | 2016-03-31 15:00 +0200 |
| Articles | 15 — 3 participants |
Back to article view | Back to linux.kernel
[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
| From | Steve Muckle <steve.muckle@linaro.org> |
|---|---|
| Date | 2016-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]
| From | Steve Muckle <steve.muckle@linaro.org> |
|---|---|
| Date | 2016-03-28 18:40 +0200 |
| Subject | Re: [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]
| From | Steve Muckle <steve.muckle@linaro.org> |
|---|---|
| Date | 2016-03-28 21:40 +0200 |
| Subject | Re: [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]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-03-30 21:40 +0200 |
| Subject | Re: [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]
| From | Steve Muckle <steve.muckle@linaro.org> |
|---|---|
| Date | 2016-03-31 03:50 +0200 |
| Subject | Re: [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]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-03-31 09:40 +0200 |
| Subject | Re: [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]
| From | Steve Muckle <steve.muckle@linaro.org> |
|---|---|
| Date | 2016-03-31 23:30 +0200 |
| Subject | Re: [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]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-04-01 11:30 +0200 |
| Subject | Re: [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]
| From | Vincent Guittot <vincent.guittot@linaro.org> |
|---|---|
| Date | 2016-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]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-03-31 11:40 +0200 |
| Subject | Re: [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]
| From | Vincent Guittot <vincent.guittot@linaro.org> |
|---|---|
| Date | 2016-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]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-03-31 12:50 +0200 |
| Subject | Re: [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]
| From | Vincent Guittot <vincent.guittot@linaro.org> |
|---|---|
| Date | 2016-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]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-03-31 14:40 +0200 |
| Subject | Re: [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]
| From | Vincent Guittot <vincent.guittot@linaro.org> |
|---|---|
| Date | 2016-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