[PATCH] sched: update last_balance only when actually balanced

[PATCH] sched: update last_balance only when actually balanced

[Rik van Riel]

Currently rebalance_domains will happily update sd->last_balance
even if should_we_balance() decides that this CPU should not do
any load balancing for the sd.

This can prevent the CPU that should load balance for the sd from
doing load balancing, which can delay active balancing essentially
forever.

The solution is to only update sd->last_balance when load_balance()
actually did something.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 kernel/sched/fair.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 45943b2..4f7fee2 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7187,7 +7187,9 @@ static void rebalance_domains(struct rq *rq, enum cpu_idle_type idle)
 				 */
 				idle = idle_cpu(cpu) ? CPU_IDLE : CPU_NOT_IDLE;
 			}
-			sd->last_balance = jiffies;
+			/* Only update if we actually balanced. */
+			if (continue_balancing)
+				sd->last_balance = jiffies;
 			interval = get_sd_balance_interval(sd, idle != CPU_IDLE);
 		}
 		if (need_serialize)

Re: [PATCH] sched: update last_balance only when actually balanced

[Peter Zijlstra]

On Tue, Jul 22, 2014 at 02:21:21AM -0400, Rik van Riel wrote:
> Currently rebalance_domains will happily update sd->last_balance
> even if should_we_balance() decides that this CPU should not do
> any load balancing for the sd.
> 
> This can prevent the CPU that should load balance for the sd from
> doing load balancing, which can delay active balancing essentially
> forever.
> 
> The solution is to only update sd->last_balance when load_balance()
> actually did something.
> 
> Signed-off-by: Rik van Riel <riel@redhat.com>
> ---
>  kernel/sched/fair.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 45943b2..4f7fee2 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7187,7 +7187,9 @@ static void rebalance_domains(struct rq *rq, enum cpu_idle_type idle)
>  				 */
>  				idle = idle_cpu(cpu) ? CPU_IDLE : CPU_NOT_IDLE;
>  			}
> -			sd->last_balance = jiffies;
> +			/* Only update if we actually balanced. */
> +			if (continue_balancing)
> +				sd->last_balance = jiffies;
>  			interval = get_sd_balance_interval(sd, idle != CPU_IDLE);
>  		}
>  		if (need_serialize)

Did you actually see any difference with this patch?

The reason I'm asking is that the sched_domains are per cpu, so the
above assignment is only visible to that particular cpu, not other cpus.

Re: [PATCH] sched: update last_balance only when actually balanced

[Rik van Riel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/22/2014 08:04 AM, Peter Zijlstra wrote:

>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index
>> 45943b2..4f7fee2 100644 --- a/kernel/sched/fair.c +++
>> b/kernel/sched/fair.c @@ -7187,7 +7187,9 @@ static void
>> rebalance_domains(struct rq *rq, enum cpu_idle_type idle) */ idle
>> = idle_cpu(cpu) ? CPU_IDLE : CPU_NOT_IDLE; } -			sd->last_balance
>> = jiffies; +			/* Only update if we actually balanced. */ +			if
>> (continue_balancing) +				sd->last_balance = jiffies; interval =
>> get_sd_balance_interval(sd, idle != CPU_IDLE); } if
>> (need_serialize)
> 
> Did you actually see any difference with this patch?
> 
> The reason I'm asking is that the sched_domains are per cpu, so
> the above assignment is only visible to that particular cpu, not
> other cpus.

It was past 2AM, it may have been a placebo effect :)

Back to the drawing board, to figure out why the load balancing code
is not working quite as expected...

- -- 
All rights reversed
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTzodfAAoJEM553pKExN6DR3oH+wVSYjVu6eV1tzaDLw2xSvSl
rO8hC9GtIf3LX8NVp/UTVkQXt7wWDLcpV4f+AO9+ikB9Dk5YD9wfTF6vhZVrP8GV
Zjpd3HvO/H3RYomyDXLbocjZ97JsXBF7kxRWmYw0kixUIzhdt/sZKGc46aQXPnGy
BfcGcYYbBKeFr9rrYTTvHwN5XVB1ZMZBEWiH/6baXGgZ5V/rTdcZDw0VGXailpbn
eM5j33B9gCE6bYxD1uCrfZXdaO4BORCkiqfncpSVqLnl9k7B9HbY44bcbqht/qHy
B/F/7+imlvAfhwfQAhj8EYomaAT2GI8m6Ab7frxagJURJ1McU3r+2dgkkhsSqP8=
=QnLG
-----END PGP SIGNATURE-----