* Re: [PATCH v2] sched/pelt: Fix task util_est update filtering
2021-02-25 16:58 [PATCH v2] sched/pelt: Fix task util_est update filtering Vincent Donnefort
@ 2021-02-26 7:22 ` Vincent Guittot
2021-02-26 8:41 ` Peter Zijlstra
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Vincent Guittot @ 2021-02-26 7:22 UTC (permalink / raw)
To: Vincent Donnefort
Cc: Peter Zijlstra, mingo, Dietmar Eggemann, linux-kernel,
Patrick Bellasi, Valentin Schneider
On Thu, 25 Feb 2021 at 17:58, Vincent Donnefort
<vincent.donnefort@arm.com> wrote:
>
> Being called for each dequeue, util_est reduces the number of its updates
> by filtering out when the EWMA signal is different from the task util_avg
> by less than 1%. It is a problem for a sudden util_avg ramp-up. Due to the
> decay from a previous high util_avg, EWMA might now be close enough to
> the new util_avg. No update would then happen while it would leave
> ue.enqueued with an out-of-date value.
>
> Taking into consideration the two util_est members, EWMA and enqueued for
> the filtering, ensures, for both, an up-to-date value.
>
> This is for now an issue only for the trace probe that might return the
> stale value. Functional-wise, it isn't a problem, as the value is always
> accessed through max(enqueued, ewma).
>
> This problem has been observed using LISA's UtilConvergence:test_means on
> the sd845c board.
>
> No regression observed with Hackbench on sd845c and Perf-bench sched pipe
> on hikey/hikey960.
>
> Signed-off-by: Vincent Donnefort <vincent.donnefort@arm.com>
> Reviewed-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 9e4104ae39ae..214e02862994 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -3966,24 +3966,27 @@ static inline void util_est_dequeue(struct cfs_rq *cfs_rq,
> trace_sched_util_est_cfs_tp(cfs_rq);
> }
>
> +#define UTIL_EST_MARGIN (SCHED_CAPACITY_SCALE / 100)
> +
> /*
> - * Check if a (signed) value is within a specified (unsigned) margin,
> + * Check if a (signed) value is within the (unsigned) util_est margin,
> * based on the observation that:
> *
> * abs(x) < y := (unsigned)(x + y - 1) < (2 * y - 1)
> *
> - * NOTE: this only works when value + maring < INT_MAX.
> + * NOTE: this only works when value + UTIL_EST_MARGIN < INT_MAX.
> */
> -static inline bool within_margin(int value, int margin)
> +static inline bool util_est_within_margin(int value)
> {
> - return ((unsigned int)(value + margin - 1) < (2 * margin - 1));
> + return ((unsigned int)(value + UTIL_EST_MARGIN - 1) <
> + (2 * UTIL_EST_MARGIN - 1));
> }
>
> static inline void util_est_update(struct cfs_rq *cfs_rq,
> struct task_struct *p,
> bool task_sleep)
> {
> - long last_ewma_diff;
> + long last_ewma_diff, last_enqueued_diff;
> struct util_est ue;
>
> if (!sched_feat(UTIL_EST))
> @@ -4004,6 +4007,8 @@ static inline void util_est_update(struct cfs_rq *cfs_rq,
> if (ue.enqueued & UTIL_AVG_UNCHANGED)
> return;
>
> + last_enqueued_diff = ue.enqueued;
> +
> /*
> * Reset EWMA on utilization increases, the moving average is used only
> * to smooth utilization decreases.
> @@ -4017,12 +4022,17 @@ static inline void util_est_update(struct cfs_rq *cfs_rq,
> }
>
> /*
> - * Skip update of task's estimated utilization when its EWMA is
> + * Skip update of task's estimated utilization when its members are
> * already ~1% close to its last activation value.
> */
> last_ewma_diff = ue.enqueued - ue.ewma;
> - if (within_margin(last_ewma_diff, (SCHED_CAPACITY_SCALE / 100)))
> + last_enqueued_diff -= ue.enqueued;
> + if (util_est_within_margin(last_ewma_diff)) {
> + if (!util_est_within_margin(last_enqueued_diff))
> + goto done;
> +
> return;
> + }
>
> /*
> * To avoid overestimation of actual task utilization, skip updates if
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] sched/pelt: Fix task util_est update filtering
2021-02-25 16:58 [PATCH v2] sched/pelt: Fix task util_est update filtering Vincent Donnefort
2021-02-26 7:22 ` Vincent Guittot
@ 2021-02-26 8:41 ` Peter Zijlstra
2021-03-01 16:34 ` Dietmar Eggemann
2021-03-02 9:01 ` [tip: sched/core] " tip-bot2 for Vincent Donnefort
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2021-02-26 8:41 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mingo, vincent.guittot, dietmar.eggemann, linux-kernel,
patrick.bellasi, valentin.schneider
On Thu, Feb 25, 2021 at 04:58:20PM +0000, Vincent Donnefort wrote:
> +#define UTIL_EST_MARGIN (SCHED_CAPACITY_SCALE / 100)
> +
> /*
> - * Check if a (signed) value is within a specified (unsigned) margin,
> + * Check if a (signed) value is within the (unsigned) util_est margin,
> * based on the observation that:
> *
> * abs(x) < y := (unsigned)(x + y - 1) < (2 * y - 1)
> *
> - * NOTE: this only works when value + maring < INT_MAX.
> + * NOTE: this only works when value + UTIL_EST_MARGIN < INT_MAX.
> */
> -static inline bool within_margin(int value, int margin)
> +static inline bool util_est_within_margin(int value)
> {
> - return ((unsigned int)(value + margin - 1) < (2 * margin - 1));
> + return ((unsigned int)(value + UTIL_EST_MARGIN - 1) <
> + (2 * UTIL_EST_MARGIN - 1));
> }
> - if (within_margin(last_ewma_diff, (SCHED_CAPACITY_SCALE / 100)))
> + if (util_est_within_margin(last_ewma_diff)) {
What was the purpose of this change? What was a generic helper is now
super specific.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] sched/pelt: Fix task util_est update filtering
2021-02-26 8:41 ` Peter Zijlstra
@ 2021-03-01 16:34 ` Dietmar Eggemann
2021-03-01 17:21 ` Peter Zijlstra
0 siblings, 1 reply; 9+ messages in thread
From: Dietmar Eggemann @ 2021-03-01 16:34 UTC (permalink / raw)
To: Peter Zijlstra, Vincent Donnefort
Cc: mingo, vincent.guittot, linux-kernel, patrick.bellasi,
valentin.schneider
On 26/02/2021 09:41, Peter Zijlstra wrote:
> On Thu, Feb 25, 2021 at 04:58:20PM +0000, Vincent Donnefort wrote:
>> +#define UTIL_EST_MARGIN (SCHED_CAPACITY_SCALE / 100)
>> +
>> /*
>> - * Check if a (signed) value is within a specified (unsigned) margin,
>> + * Check if a (signed) value is within the (unsigned) util_est margin,
>> * based on the observation that:
>> *
>> * abs(x) < y := (unsigned)(x + y - 1) < (2 * y - 1)
>> *
>> - * NOTE: this only works when value + maring < INT_MAX.
>> + * NOTE: this only works when value + UTIL_EST_MARGIN < INT_MAX.
>> */
>> -static inline bool within_margin(int value, int margin)
>> +static inline bool util_est_within_margin(int value)
>> {
>> - return ((unsigned int)(value + margin - 1) < (2 * margin - 1));
>> + return ((unsigned int)(value + UTIL_EST_MARGIN - 1) <
>> + (2 * UTIL_EST_MARGIN - 1));
>> }
>
>> - if (within_margin(last_ewma_diff, (SCHED_CAPACITY_SCALE / 100)))
>> + if (util_est_within_margin(last_ewma_diff)) {
>
> What was the purpose of this change? What was a generic helper is now
> super specific.
I guess because it was only ever used in util_est for last_ewma_diff.
It's now used for last_ewma_diff and last_enqueued_diff, still only for
util_est though and both times with the same margin
(SCHED_CAPACITY_SCALE / 100)).
Vincent D. should be back on Wed from hols.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] sched/pelt: Fix task util_est update filtering
2021-03-01 16:34 ` Dietmar Eggemann
@ 2021-03-01 17:21 ` Peter Zijlstra
2021-03-03 10:06 ` Vincent Donnefort
0 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2021-03-01 17:21 UTC (permalink / raw)
To: Dietmar Eggemann
Cc: Vincent Donnefort, mingo, vincent.guittot, linux-kernel,
patrick.bellasi, valentin.schneider
On Mon, Mar 01, 2021 at 05:34:09PM +0100, Dietmar Eggemann wrote:
> On 26/02/2021 09:41, Peter Zijlstra wrote:
> > On Thu, Feb 25, 2021 at 04:58:20PM +0000, Vincent Donnefort wrote:
> >> +#define UTIL_EST_MARGIN (SCHED_CAPACITY_SCALE / 100)
> >> +
> >> /*
> >> - * Check if a (signed) value is within a specified (unsigned) margin,
> >> + * Check if a (signed) value is within the (unsigned) util_est margin,
> >> * based on the observation that:
> >> *
> >> * abs(x) < y := (unsigned)(x + y - 1) < (2 * y - 1)
> >> *
> >> - * NOTE: this only works when value + maring < INT_MAX.
> >> + * NOTE: this only works when value + UTIL_EST_MARGIN < INT_MAX.
> >> */
> >> -static inline bool within_margin(int value, int margin)
> >> +static inline bool util_est_within_margin(int value)
> >> {
> >> - return ((unsigned int)(value + margin - 1) < (2 * margin - 1));
> >> + return ((unsigned int)(value + UTIL_EST_MARGIN - 1) <
> >> + (2 * UTIL_EST_MARGIN - 1));
> >> }
> >
> >> - if (within_margin(last_ewma_diff, (SCHED_CAPACITY_SCALE / 100)))
> >> + if (util_est_within_margin(last_ewma_diff)) {
> >
> > What was the purpose of this change? What was a generic helper is now
> > super specific.
>
> I guess because it was only ever used in util_est for last_ewma_diff.
>
> It's now used for last_ewma_diff and last_enqueued_diff, still only for
> util_est though and both times with the same margin
> (SCHED_CAPACITY_SCALE / 100)).
>
> Vincent D. should be back on Wed from hols.
Fair enough; I've un-done it but kept the rest of the patch.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] sched/pelt: Fix task util_est update filtering
2021-03-01 17:21 ` Peter Zijlstra
@ 2021-03-03 10:06 ` Vincent Donnefort
0 siblings, 0 replies; 9+ messages in thread
From: Vincent Donnefort @ 2021-03-03 10:06 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Dietmar Eggemann, vincent.guittot, linux-kernel, patrick.bellasi,
valentin.schneider
On Mon, Mar 01, 2021 at 06:21:23PM +0100, Peter Zijlstra wrote:
> On Mon, Mar 01, 2021 at 05:34:09PM +0100, Dietmar Eggemann wrote:
> > On 26/02/2021 09:41, Peter Zijlstra wrote:
> > > On Thu, Feb 25, 2021 at 04:58:20PM +0000, Vincent Donnefort wrote:
> > >> +#define UTIL_EST_MARGIN (SCHED_CAPACITY_SCALE / 100)
> > >> +
> > >> /*
> > >> - * Check if a (signed) value is within a specified (unsigned) margin,
> > >> + * Check if a (signed) value is within the (unsigned) util_est margin,
> > >> * based on the observation that:
> > >> *
> > >> * abs(x) < y := (unsigned)(x + y - 1) < (2 * y - 1)
> > >> *
> > >> - * NOTE: this only works when value + maring < INT_MAX.
> > >> + * NOTE: this only works when value + UTIL_EST_MARGIN < INT_MAX.
> > >> */
> > >> -static inline bool within_margin(int value, int margin)
> > >> +static inline bool util_est_within_margin(int value)
> > >> {
> > >> - return ((unsigned int)(value + margin - 1) < (2 * margin - 1));
> > >> + return ((unsigned int)(value + UTIL_EST_MARGIN - 1) <
> > >> + (2 * UTIL_EST_MARGIN - 1));
> > >> }
> > >
> > >> - if (within_margin(last_ewma_diff, (SCHED_CAPACITY_SCALE / 100)))
> > >> + if (util_est_within_margin(last_ewma_diff)) {
> > >
> > > What was the purpose of this change? What was a generic helper is now
> > > super specific.
> >
> > I guess because it was only ever used in util_est for last_ewma_diff.
> >
> > It's now used for last_ewma_diff and last_enqueued_diff, still only for
> > util_est though and both times with the same margin
> > (SCHED_CAPACITY_SCALE / 100)).
> >
> > Vincent D. should be back on Wed from hols.
>
> Fair enough; I've un-done it but kept the rest of the patch.
Indeed I was off for couple of days. Apologies for the delay and thanks for
applying the patch.
--
Vincent
^ permalink raw reply [flat|nested] 9+ messages in thread
* [tip: sched/core] sched/pelt: Fix task util_est update filtering
2021-02-25 16:58 [PATCH v2] sched/pelt: Fix task util_est update filtering Vincent Donnefort
2021-02-26 7:22 ` Vincent Guittot
2021-02-26 8:41 ` Peter Zijlstra
@ 2021-03-02 9:01 ` tip-bot2 for Vincent Donnefort
2021-03-03 9:49 ` tip-bot2 for Vincent Donnefort
2021-03-06 11:42 ` tip-bot2 for Vincent Donnefort
4 siblings, 0 replies; 9+ messages in thread
From: tip-bot2 for Vincent Donnefort @ 2021-03-02 9:01 UTC (permalink / raw)
To: linux-tip-commits
Cc: Vincent Donnefort, Peter Zijlstra (Intel),
Dietmar Eggemann, Vincent Guittot, x86, linux-kernel
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 78ca1ab2718a5518171f2e7d0afad0b9752c4453
Gitweb: https://git.kernel.org/tip/78ca1ab2718a5518171f2e7d0afad0b9752c4453
Author: Vincent Donnefort <vincent.donnefort@arm.com>
AuthorDate: Thu, 25 Feb 2021 16:58:20
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Mon, 01 Mar 2021 18:17:26 +01:00
sched/pelt: Fix task util_est update filtering
Being called for each dequeue, util_est reduces the number of its updates
by filtering out when the EWMA signal is different from the task util_avg
by less than 1%. It is a problem for a sudden util_avg ramp-up. Due to the
decay from a previous high util_avg, EWMA might now be close enough to
the new util_avg. No update would then happen while it would leave
ue.enqueued with an out-of-date value.
Taking into consideration the two util_est members, EWMA and enqueued for
the filtering, ensures, for both, an up-to-date value.
This is for now an issue only for the trace probe that might return the
stale value. Functional-wise, it isn't a problem, as the value is always
accessed through max(enqueued, ewma).
This problem has been observed using LISA's UtilConvergence:test_means on
the sd845c board.
No regression observed with Hackbench on sd845c and Perf-bench sched pipe
on hikey/hikey960.
Signed-off-by: Vincent Donnefort <vincent.donnefort@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://lkml.kernel.org/r/20210225165820.1377125-1-vincent.donnefort@arm.com
---
kernel/sched/fair.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 1af51a6..f5d6541 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3941,6 +3941,8 @@ static inline void util_est_dequeue(struct cfs_rq *cfs_rq,
trace_sched_util_est_cfs_tp(cfs_rq);
}
+#define UTIL_EST_MARGIN (SCHED_CAPACITY_SCALE / 100)
+
/*
* Check if a (signed) value is within a specified (unsigned) margin,
* based on the observation that:
@@ -3958,7 +3960,7 @@ static inline void util_est_update(struct cfs_rq *cfs_rq,
struct task_struct *p,
bool task_sleep)
{
- long last_ewma_diff;
+ long last_ewma_diff, last_enqueued_diff;
struct util_est ue;
if (!sched_feat(UTIL_EST))
@@ -3979,6 +3981,8 @@ static inline void util_est_update(struct cfs_rq *cfs_rq,
if (ue.enqueued & UTIL_AVG_UNCHANGED)
return;
+ last_enqueued_diff = ue.enqueued;
+
/*
* Reset EWMA on utilization increases, the moving average is used only
* to smooth utilization decreases.
@@ -3992,12 +3996,17 @@ static inline void util_est_update(struct cfs_rq *cfs_rq,
}
/*
- * Skip update of task's estimated utilization when its EWMA is
+ * Skip update of task's estimated utilization when its members are
* already ~1% close to its last activation value.
*/
last_ewma_diff = ue.enqueued - ue.ewma;
- if (within_margin(last_ewma_diff, (SCHED_CAPACITY_SCALE / 100)))
+ last_enqueued_diff -= ue.enqueued;
+ if (within_margin(last_ewma_diff, UTIL_EST_MARGIN)) {
+ if (!within_margin(last_enqueued_diff, UTIL_EST_MARGIN))
+ goto done;
+
return;
+ }
/*
* To avoid overestimation of actual task utilization, skip updates if
^ permalink raw reply related [flat|nested] 9+ messages in thread* [tip: sched/core] sched/pelt: Fix task util_est update filtering
2021-02-25 16:58 [PATCH v2] sched/pelt: Fix task util_est update filtering Vincent Donnefort
` (2 preceding siblings ...)
2021-03-02 9:01 ` [tip: sched/core] " tip-bot2 for Vincent Donnefort
@ 2021-03-03 9:49 ` tip-bot2 for Vincent Donnefort
2021-03-06 11:42 ` tip-bot2 for Vincent Donnefort
4 siblings, 0 replies; 9+ messages in thread
From: tip-bot2 for Vincent Donnefort @ 2021-03-03 9:49 UTC (permalink / raw)
To: linux-tip-commits
Cc: Vincent Donnefort, Peter Zijlstra (Intel),
Dietmar Eggemann, Vincent Guittot, x86, linux-kernel
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 9357e217ba642b39ce89f9cd5b5f3e5a21712283
Gitweb: https://git.kernel.org/tip/9357e217ba642b39ce89f9cd5b5f3e5a21712283
Author: Vincent Donnefort <vincent.donnefort@arm.com>
AuthorDate: Thu, 25 Feb 2021 16:58:20
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 03 Mar 2021 10:33:00 +01:00
sched/pelt: Fix task util_est update filtering
Being called for each dequeue, util_est reduces the number of its updates
by filtering out when the EWMA signal is different from the task util_avg
by less than 1%. It is a problem for a sudden util_avg ramp-up. Due to the
decay from a previous high util_avg, EWMA might now be close enough to
the new util_avg. No update would then happen while it would leave
ue.enqueued with an out-of-date value.
Taking into consideration the two util_est members, EWMA and enqueued for
the filtering, ensures, for both, an up-to-date value.
This is for now an issue only for the trace probe that might return the
stale value. Functional-wise, it isn't a problem, as the value is always
accessed through max(enqueued, ewma).
This problem has been observed using LISA's UtilConvergence:test_means on
the sd845c board.
No regression observed with Hackbench on sd845c and Perf-bench sched pipe
on hikey/hikey960.
Signed-off-by: Vincent Donnefort <vincent.donnefort@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://lkml.kernel.org/r/20210225165820.1377125-1-vincent.donnefort@arm.com
---
kernel/sched/fair.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 1af51a6..f5d6541 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3941,6 +3941,8 @@ static inline void util_est_dequeue(struct cfs_rq *cfs_rq,
trace_sched_util_est_cfs_tp(cfs_rq);
}
+#define UTIL_EST_MARGIN (SCHED_CAPACITY_SCALE / 100)
+
/*
* Check if a (signed) value is within a specified (unsigned) margin,
* based on the observation that:
@@ -3958,7 +3960,7 @@ static inline void util_est_update(struct cfs_rq *cfs_rq,
struct task_struct *p,
bool task_sleep)
{
- long last_ewma_diff;
+ long last_ewma_diff, last_enqueued_diff;
struct util_est ue;
if (!sched_feat(UTIL_EST))
@@ -3979,6 +3981,8 @@ static inline void util_est_update(struct cfs_rq *cfs_rq,
if (ue.enqueued & UTIL_AVG_UNCHANGED)
return;
+ last_enqueued_diff = ue.enqueued;
+
/*
* Reset EWMA on utilization increases, the moving average is used only
* to smooth utilization decreases.
@@ -3992,12 +3996,17 @@ static inline void util_est_update(struct cfs_rq *cfs_rq,
}
/*
- * Skip update of task's estimated utilization when its EWMA is
+ * Skip update of task's estimated utilization when its members are
* already ~1% close to its last activation value.
*/
last_ewma_diff = ue.enqueued - ue.ewma;
- if (within_margin(last_ewma_diff, (SCHED_CAPACITY_SCALE / 100)))
+ last_enqueued_diff -= ue.enqueued;
+ if (within_margin(last_ewma_diff, UTIL_EST_MARGIN)) {
+ if (!within_margin(last_enqueued_diff, UTIL_EST_MARGIN))
+ goto done;
+
return;
+ }
/*
* To avoid overestimation of actual task utilization, skip updates if
^ permalink raw reply related [flat|nested] 9+ messages in thread* [tip: sched/core] sched/pelt: Fix task util_est update filtering
2021-02-25 16:58 [PATCH v2] sched/pelt: Fix task util_est update filtering Vincent Donnefort
` (3 preceding siblings ...)
2021-03-03 9:49 ` tip-bot2 for Vincent Donnefort
@ 2021-03-06 11:42 ` tip-bot2 for Vincent Donnefort
4 siblings, 0 replies; 9+ messages in thread
From: tip-bot2 for Vincent Donnefort @ 2021-03-06 11:42 UTC (permalink / raw)
To: linux-tip-commits
Cc: Vincent Donnefort, Peter Zijlstra (Intel),
Ingo Molnar, Dietmar Eggemann, Vincent Guittot, x86,
linux-kernel
The following commit has been merged into the sched/core branch of tip:
Commit-ID: b89997aa88f0b07d8a6414c908af75062103b8c9
Gitweb: https://git.kernel.org/tip/b89997aa88f0b07d8a6414c908af75062103b8c9
Author: Vincent Donnefort <vincent.donnefort@arm.com>
AuthorDate: Thu, 25 Feb 2021 16:58:20
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Sat, 06 Mar 2021 12:40:22 +01:00
sched/pelt: Fix task util_est update filtering
Being called for each dequeue, util_est reduces the number of its updates
by filtering out when the EWMA signal is different from the task util_avg
by less than 1%. It is a problem for a sudden util_avg ramp-up. Due to the
decay from a previous high util_avg, EWMA might now be close enough to
the new util_avg. No update would then happen while it would leave
ue.enqueued with an out-of-date value.
Taking into consideration the two util_est members, EWMA and enqueued for
the filtering, ensures, for both, an up-to-date value.
This is for now an issue only for the trace probe that might return the
stale value. Functional-wise, it isn't a problem, as the value is always
accessed through max(enqueued, ewma).
This problem has been observed using LISA's UtilConvergence:test_means on
the sd845c board.
No regression observed with Hackbench on sd845c and Perf-bench sched pipe
on hikey/hikey960.
Signed-off-by: Vincent Donnefort <vincent.donnefort@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://lkml.kernel.org/r/20210225165820.1377125-1-vincent.donnefort@arm.com
---
kernel/sched/fair.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 1af51a6..f5d6541 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3941,6 +3941,8 @@ static inline void util_est_dequeue(struct cfs_rq *cfs_rq,
trace_sched_util_est_cfs_tp(cfs_rq);
}
+#define UTIL_EST_MARGIN (SCHED_CAPACITY_SCALE / 100)
+
/*
* Check if a (signed) value is within a specified (unsigned) margin,
* based on the observation that:
@@ -3958,7 +3960,7 @@ static inline void util_est_update(struct cfs_rq *cfs_rq,
struct task_struct *p,
bool task_sleep)
{
- long last_ewma_diff;
+ long last_ewma_diff, last_enqueued_diff;
struct util_est ue;
if (!sched_feat(UTIL_EST))
@@ -3979,6 +3981,8 @@ static inline void util_est_update(struct cfs_rq *cfs_rq,
if (ue.enqueued & UTIL_AVG_UNCHANGED)
return;
+ last_enqueued_diff = ue.enqueued;
+
/*
* Reset EWMA on utilization increases, the moving average is used only
* to smooth utilization decreases.
@@ -3992,12 +3996,17 @@ static inline void util_est_update(struct cfs_rq *cfs_rq,
}
/*
- * Skip update of task's estimated utilization when its EWMA is
+ * Skip update of task's estimated utilization when its members are
* already ~1% close to its last activation value.
*/
last_ewma_diff = ue.enqueued - ue.ewma;
- if (within_margin(last_ewma_diff, (SCHED_CAPACITY_SCALE / 100)))
+ last_enqueued_diff -= ue.enqueued;
+ if (within_margin(last_ewma_diff, UTIL_EST_MARGIN)) {
+ if (!within_margin(last_enqueued_diff, UTIL_EST_MARGIN))
+ goto done;
+
return;
+ }
/*
* To avoid overestimation of actual task utilization, skip updates if
^ permalink raw reply related [flat|nested] 9+ messages in thread