* Re: [PATCH] sched/fair: fix condition of avg_load calculation
[not found] <BL0PR14MB3779226ECE6B526471FA91FD9AF40@BL0PR14MB3779.namprd14.prod.outlook.com>
@ 2020-03-19 8:06 ` Vincent Guittot
2020-03-19 17:12 ` Mel Gorman
1 sibling, 0 replies; 2+ messages in thread
From: Vincent Guittot @ 2020-03-19 8:06 UTC (permalink / raw)
To: Tao Zhou
Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, linux-kernel,
Hillf Danton, T. Zhou
On Thu, 19 Mar 2020 at 04:36, Tao Zhou <ouwen210@hotmail.com> wrote:
>
> In update_sg_wakeup_stats(), the comment says:
>
> Computing avg_load makes sense only when group is fully
> busy or overloaded.
>
> But, the code below this comment does not check like this.
>
> From reading the code about avg_load in other functions, I
> confirm that avg_load should be calculated in fully busy or
> overloaded case. The comment is correct and the checking
> condition is wrong. So, change that condition.
>
> Fixes: 57abff067a08 ("sched/fair: Rework find_idlest_group()")
> Signed-off-by: Tao Zhou <ouwen210@hotmail.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Thanks
> ---
> kernel/sched/fair.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 1dea8554ead0..9cae03676b0d 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8613,7 +8613,8 @@ static inline void update_sg_wakeup_stats(struct sched_domain *sd,
> * Computing avg_load makes sense only when group is fully busy or
> * overloaded
> */
> - if (sgs->group_type < group_fully_busy)
> + if (sgs->group_type == group_fully_busy ||
> + sgs->group_type == group_overloaded)
> sgs->avg_load = (sgs->group_load * SCHED_CAPACITY_SCALE) /
> sgs->group_capacity;
> }
> --
> 2.24.1
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [PATCH] sched/fair: fix condition of avg_load calculation
[not found] <BL0PR14MB3779226ECE6B526471FA91FD9AF40@BL0PR14MB3779.namprd14.prod.outlook.com>
2020-03-19 8:06 ` [PATCH] sched/fair: fix condition of avg_load calculation Vincent Guittot
@ 2020-03-19 17:12 ` Mel Gorman
1 sibling, 0 replies; 2+ messages in thread
From: Mel Gorman @ 2020-03-19 17:12 UTC (permalink / raw)
To: Tao Zhou
Cc: mingo, peterz, juri.lelli, dietmar.eggemann, rostedt, bsegall,
vincent.guittot, linux-kernel, hdanton, t1zhou
On Thu, Mar 19, 2020 at 11:39:20AM +0800, Tao Zhou wrote:
> In update_sg_wakeup_stats(), the comment says:
>
> Computing avg_load makes sense only when group is fully
> busy or overloaded.
>
> But, the code below this comment does not check like this.
>
> From reading the code about avg_load in other functions, I
> confirm that avg_load should be calculated in fully busy or
> overloaded case. The comment is correct and the checking
> condition is wrong. So, change that condition.
>
> Fixes: 57abff067a08 ("sched/fair: Rework find_idlest_group()")
> Signed-off-by: Tao Zhou <ouwen210@hotmail.com>
Actual impact is variable, some machines for stressed overload benefit
but it's not universal. That is somewhat expected given that the heavily
overloaded case is tricky at the best of times. For tbench ramping up
load, it's also not universally beneficial but some machines heavily
benefit. Despite the range of results that are machine-dependant, the
patch looks correct so;
Acked-by: Mel Gorman <mgorman@suse.de>
--
Mel Gorman
SUSE Labs
^ permalink raw reply [flat|nested] 2+ messages in thread