Commit 38d707c5 authored by Ingo Molnar's avatar Ingo Molnar
Browse files

sched/balancing: Change 'enum cpu_idle_type' to have more natural definitions 



The cpu_idle_type enum has the confusingly inverted property
that 'not idle' is 1, and 'idle' is '0'.

This resulted in a number of unnecessary complications in the code.

Reverse the order, remove the CPU_NOT_IDLE type, and convert
all code to a natural boolean form.

It's much more readable:

  -       enum cpu_idle_type idle = this_rq->idle_balance ?
  -                                               CPU_IDLE : CPU_NOT_IDLE;
  -
  +       enum cpu_idle_type idle = this_rq->idle_balance;

  --------------------------------

  -       if (env->idle == CPU_NOT_IDLE || !busiest->sum_nr_running)
  +       if (!env->idle || !busiest->sum_nr_running)

  --------------------------------

And gets rid of the double negation in these usages:

  -               if (env->idle != CPU_NOT_IDLE && env->src_rq->nr_running <= 1)
  +               if (env->idle && env->src_rq->nr_running <= 1)

Furthermore, this makes code much more obvious where there's
differentiation between CPU_IDLE and CPU_NEWLY_IDLE.

Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
Reviewed-by: default avatarValentin Schneider <vschneid@redhat.com>
Reviewed-by: default avatarVincent Guittot <vincent.guittot@linaro.org>
Cc: "Gautham R. Shenoy" <gautham.shenoy@amd.com>
Link: https://lore.kernel.org/r/20240308105901.1096078-4-mingo@kernel.org
parent 02a61f32

include/linux/sched/idle.h

+1 −1
Original line number Diff line number Diff line
@@ -5,8 +5,8 @@ 
#include <linux/sched.h>



enum cpu_idle_type {

	__CPU_NOT_IDLE = 0,

	CPU_IDLE,

	CPU_NOT_IDLE,

	CPU_NEWLY_IDLE,

	CPU_MAX_IDLE_TYPES

};

kernel/sched/fair.c

+12 −15
Original line number Diff line number Diff line
@@ -9070,7 +9070,7 @@ static int detach_tasks(struct lb_env *env) 
		 * We don't want to steal all, otherwise we may be treated likewise,

		 * which could at worst lead to a livelock crash.

		 */

		if (env->idle != CPU_NOT_IDLE && env->src_rq->nr_running <= 1)

		if (env->idle && env->src_rq->nr_running <= 1)

			break;



		env->loop++;
@@ -9803,7 +9803,7 @@ static inline bool smt_vs_nonsmt_groups(struct sched_group *sg1, 
static inline bool smt_balance(struct lb_env *env, struct sg_lb_stats *sgs,

			       struct sched_group *group)

{

	if (env->idle == CPU_NOT_IDLE)

	if (!env->idle)

		return false;



	/*
@@ -9827,7 +9827,7 @@ static inline long sibling_imbalance(struct lb_env *env, 
	int ncores_busiest, ncores_local;

	long imbalance;



	if (env->idle == CPU_NOT_IDLE || !busiest->sum_nr_running)

	if (!env->idle || !busiest->sum_nr_running)

		return 0;



	ncores_busiest = sds->busiest->cores;
@@ -9927,8 +9927,7 @@ static inline void update_sg_lb_stats(struct lb_env *env, 
				sgs->group_misfit_task_load = rq->misfit_task_load;

				*sg_status |= SG_OVERLOAD;

			}

		} else if ((env->idle != CPU_NOT_IDLE) &&

			   sched_reduced_capacity(rq, env->sd)) {

		} else if (env->idle && sched_reduced_capacity(rq, env->sd)) {

			/* Check for a task running on a CPU with reduced capacity */

			if (sgs->group_misfit_task_load < load)

				sgs->group_misfit_task_load = load;
@@ -9940,7 +9939,7 @@ static inline void update_sg_lb_stats(struct lb_env *env, 
	sgs->group_weight = group->group_weight;



	/* Check if dst CPU is idle and preferred to this group */

	if (!local_group && env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&

	if (!local_group && env->idle && sgs->sum_h_nr_running &&

	    sched_group_asym(env, sgs, group))

		sgs->group_asym_packing = 1;
@@ -10698,7 +10697,7 @@ static inline void calculate_imbalance(struct lb_env *env, struct sd_lb_stats *s 
			 * waiting task in this overloaded busiest group. Let's

			 * try to pull it.

			 */

			if (env->idle != CPU_NOT_IDLE && env->imbalance == 0) {

			if (env->idle && env->imbalance == 0) {

				env->migration_type = migrate_task;

				env->imbalance = 1;

			}
@@ -10913,7 +10912,7 @@ static struct sched_group *find_busiest_group(struct lb_env *env) 
		goto force_balance;



	if (busiest->group_type != group_overloaded) {

		if (env->idle == CPU_NOT_IDLE) {

		if (!env->idle) {

			/*

			 * If the busiest group is not overloaded (and as a

			 * result the local one too) but this CPU is already
@@ -11121,7 +11120,7 @@ asym_active_balance(struct lb_env *env) 
	 * the lower priority @env::dst_cpu help it. Do not follow

	 * CPU priority.

	 */

	return env->idle != CPU_NOT_IDLE && sched_use_asym_prio(env->sd, env->dst_cpu) &&

	return env->idle && sched_use_asym_prio(env->sd, env->dst_cpu) &&

	       (sched_asym_prefer(env->dst_cpu, env->src_cpu) ||

		!sched_use_asym_prio(env->sd, env->src_cpu));

}
@@ -11159,7 +11158,7 @@ static int need_active_balance(struct lb_env *env) 
	 * because of other sched_class or IRQs if more capacity stays

	 * available on dst_cpu.

	 */

	if ((env->idle != CPU_NOT_IDLE) &&

	if (env->idle &&

	    (env->src_rq->cfs.h_nr_running == 1)) {

		if ((check_cpu_capacity(env->src_rq, sd)) &&

		    (capacity_of(env->src_cpu)*sd->imbalance_pct < capacity_of(env->dst_cpu)*100))
@@ -11735,8 +11734,8 @@ static void rebalance_domains(struct rq *rq, enum cpu_idle_type idle) 
				 * env->dst_cpu, so we can't know our idle

				 * state even if we migrated tasks. Update it.

				 */

				idle = idle_cpu(cpu) ? CPU_IDLE : CPU_NOT_IDLE;

				busy = idle != CPU_IDLE && !sched_idle_cpu(cpu);

				idle = idle_cpu(cpu);

				busy = !idle && !sched_idle_cpu(cpu);

			}

			sd->last_balance = jiffies;

			interval = get_sd_balance_interval(sd, busy);
@@ -12416,9 +12415,7 @@ static int newidle_balance(struct rq *this_rq, struct rq_flags *rf) 
static __latent_entropy void run_rebalance_domains(struct softirq_action *h)

{

	struct rq *this_rq = this_rq();

	enum cpu_idle_type idle = this_rq->idle_balance ?

						CPU_IDLE : CPU_NOT_IDLE;



	enum cpu_idle_type idle = this_rq->idle_balance;

	/*

	 * If this CPU has a pending nohz_balance_kick, then do the

	 * balancing on behalf of the other idle CPUs whose ticks are