Commit 23fb06d9 authored by Valentin Schneider's avatar Valentin Schneider Committed by Peter Zijlstra
Browse files

sched/fair: Clean up active balance nr_balance_failed trickery 



When triggering an active load balance, sd->nr_balance_failed is set to
such a value that any further can_migrate_task() using said sd will ignore
the output of task_hot().

This behaviour makes sense, as active load balance intentionally preempts a
rq's running task to migrate it right away, but this asynchronous write is
a bit shoddy, as the stopper thread might run active_load_balance_cpu_stop
before the sd->nr_balance_failed write either becomes visible to the
stopper's CPU or even happens on the CPU that appended the stopper work.

Add a struct lb_env flag to denote active balancing, and use it in
can_migrate_task(). Remove the sd->nr_balance_failed write that served the
same purpose. Cleanup the LBF_DST_PINNED active balance special case.

Signed-off-by: default avatarValentin Schneider <valentin.schneider@arm.com>
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: default avatarDietmar Eggemann <dietmar.eggemann@arm.com>
Reviewed-by: default avatarVincent Guittot <vincent.guittot@linaro.org>
Link: https://lkml.kernel.org/r/20210407220628.3798191-3-valentin.schneider@arm.com
parent 9bcb959d

kernel/sched/fair.c

+15 −16
Original line number Diff line number Diff line
@@ -7453,6 +7453,7 @@ enum migration_type { 
#define LBF_NEED_BREAK	0x02

#define LBF_DST_PINNED  0x04

#define LBF_SOME_PINNED	0x08

#define LBF_ACTIVE_LB	0x10



struct lb_env {

	struct sched_domain	*sd;
@@ -7614,10 +7615,13 @@ int can_migrate_task(struct task_struct *p, struct lb_env *env) 
		 * our sched_group. We may want to revisit it if we couldn't

		 * meet load balance goals by pulling other tasks on src_cpu.

		 *

		 * Avoid computing new_dst_cpu for NEWLY_IDLE or if we have

		 * already computed one in current iteration.

		 * Avoid computing new_dst_cpu

		 * - for NEWLY_IDLE

		 * - if we have already computed one in current iteration

		 * - if it's an active balance

		 */

		if (env->idle == CPU_NEWLY_IDLE || (env->flags & LBF_DST_PINNED))

		if (env->idle == CPU_NEWLY_IDLE ||

		    env->flags & (LBF_DST_PINNED | LBF_ACTIVE_LB))

			return 0;



		/* Prevent to re-select dst_cpu via env's CPUs: */
@@ -7642,10 +7646,14 @@ int can_migrate_task(struct task_struct *p, struct lb_env *env) 


	/*

	 * Aggressive migration if:

	 * 1) destination numa is preferred

	 * 2) task is cache cold, or

	 * 3) too many balance attempts have failed.

	 * 1) active balance

	 * 2) destination numa is preferred

	 * 3) task is cache cold, or

	 * 4) too many balance attempts have failed.

	 */

	if (env->flags & LBF_ACTIVE_LB)

		return 1;



	tsk_cache_hot = migrate_degrades_locality(p, env);

	if (tsk_cache_hot == -1)

		tsk_cache_hot = task_hot(p, env);
@@ -9836,9 +9844,6 @@ static int load_balance(int this_cpu, struct rq *this_rq, 
					active_load_balance_cpu_stop, busiest,

					&busiest->active_balance_work);

			}



			/* We've kicked active balancing, force task migration. */

			sd->nr_balance_failed = sd->cache_nice_tries+1;

		}

	} else {

		sd->nr_balance_failed = 0;
@@ -9988,13 +9993,7 @@ static int active_load_balance_cpu_stop(void *data) 
			.src_cpu	= busiest_rq->cpu,

			.src_rq		= busiest_rq,

			.idle		= CPU_IDLE,

			/*

			 * can_migrate_task() doesn't need to compute new_dst_cpu

			 * for active balancing. Since we have CPU_IDLE, but no

			 * @dst_grpmask we need to make that test go away with lying

			 * about DST_PINNED.

			 */

			.flags		= LBF_DST_PINNED,

			.flags		= LBF_ACTIVE_LB,

		};



		schedstat_inc(sd->alb_count);