[vz8] kernel/sched/fair.c: Add more missing update_rq_clock() calls

Submitted by Andrey Ryabinin on Sept. 29, 2020, 1:55 p.m.

Details

Message ID 20200929135536.17706-1-aryabinin@virtuozzo.com
State New
Series "kernel/sched/fair.c: Add missing update_rq_clock() calls"
Headers show

Commit Message

Andrey Ryabinin Sept. 29, 2020, 1:55 p.m.
Add update_rq_clock() for 'target_rq' to avoid WARN() coming
from attach_task(). Also add update_rq_clock(env.src_rq); in
load_balance() for detach_task().

https://jira.sw.ru/browse/PSBM-108013
Reported-by: Kirill Tkhai <ktkhai@virtuozzo.com>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 kernel/sched/fair.c | 2 ++
 1 file changed, 2 insertions(+)

Patch hide | download patch | download mbox

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index e6dc21d5fa03..99dcb9e77efd 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7817,6 +7817,7 @@  static int cpulimit_balance_cpu_stop(void *data)
 		schedstat_inc(sd->clb_count);
 
 		update_rq_clock(rq);
+		update_rq_clock(target_rq);
 		if (do_cpulimit_balance(&env))
 			schedstat_inc(sd->clb_pushed);
 		else
@@ -9177,6 +9178,7 @@  static int load_balance(int this_cpu, struct rq *this_rq,
 			env.loop = 0;
 			local_irq_save(rf.flags);
 			double_rq_lock(env.dst_rq, busiest);
+			update_rq_clock(env.src_rq);
 			update_rq_clock(env.dst_rq);
 			cur_ld_moved = ld_moved = move_task_groups(&env);
 			double_rq_unlock(env.dst_rq, busiest);

Comments

Kirill Tkhai Sept. 29, 2020, 2:34 p.m.
On 29.09.2020 16:55, Andrey Ryabinin wrote:
> Add update_rq_clock() for 'target_rq' to avoid WARN() coming
> from attach_task(). Also add update_rq_clock(env.src_rq); in
> load_balance() for detach_task().
> 
> https://jira.sw.ru/browse/PSBM-108013
> Reported-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> ---
>  kernel/sched/fair.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index e6dc21d5fa03..99dcb9e77efd 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7817,6 +7817,7 @@ static int cpulimit_balance_cpu_stop(void *data)
>  		schedstat_inc(sd->clb_count);
>  
>  		update_rq_clock(rq);
> +		update_rq_clock(target_rq);
>  		if (do_cpulimit_balance(&env))
>  			schedstat_inc(sd->clb_pushed);
>  		else
> @@ -9177,6 +9178,7 @@ static int load_balance(int this_cpu, struct rq *this_rq,
>  			env.loop = 0;
>  			local_irq_save(rf.flags);
>  			double_rq_lock(env.dst_rq, busiest);
> +			update_rq_clock(env.src_rq);

Fortunately, we may avoid update_rq_clock here, since srq_rq clock are already updated.
Also, I'm afraid this will bring to SCHED_WARN_ON(rq->clock_update_flags & RQCF_UPDATED)
in this function. It looks we should use rq_repin_lock() here instead:

@@ -9177,6 +9177,7 @@ static int load_balance(int this_cpu, struct rq *this_rq,
 			env.loop = 0;
 			local_irq_save(rf.flags);
 			double_rq_lock(env.dst_rq, busiest);
+			rq_repin_lock(rq, &rf);
 			update_rq_clock(env.dst_rq);
 			cur_ld_moved = ld_moved = move_task_groups(&env);
 			double_rq_unlock(env.dst_rq, busiest);

>  			update_rq_clock(env.dst_rq);
>  			cur_ld_moved = ld_moved = move_task_groups(&env);
>  			double_rq_unlock(env.dst_rq, busiest);
>