Re: [RFC PATCH RESEND 01/10] sched/fair: Do not set_rd_overloaded() if rd->online != env->cpus

[K Prateek Nayak <kprateek.nayak@amd.com>]

Hello Xin,

On 9/11/2026 6:36 AM, Xin Zhao wrote:
> On Thu, 10 Sep 2026 14:00:43 +0530 K Prateek Nayak <kprateek.nayak@amd.com> wrote:
> 
>> Only two cases manipulate env.cpus:
>>
>> 1. LBF_DST_PINNED: The CPU doing the load balancing clears itself from
>>    env.cpus since pinned tasks cannot be moved to it and goes to
>>    "more_balance" but "more_balance" does not recompute stats and never
>>    reaches update_sd_lb_stats().
>>
>> 2. LBF_ALL_PINNED: CPU with no movable task is cleared from env.cpus.
>>    How will rd->overload being set for a CPU that cannot be helped make
>>    newidle balance any more efficient?
> 
> The effective range of LBF_ALL_PINNED is specific to a particular src CPU
> and a particular dst CPU, whereas rd->overload is indeed a global marker
> that affects all CPUs with idle states. Regardless of whether case 1 has
> been processed, it seems unreasonable to me that rd->overload could be
> incorrectly cleared due to case 2, because the scopes of the LBF_ALL_PINNED
> and ->overload flags are not equivalent.

What is the point of doing load balancing if the CPUs that are overloaded
have all their tasks pinned? Those are just wasted cycles.
>>  Since LBF_ALL_PINNED is known with busiest's rq_lock held, maybe you
>>  can set a rq->flag and later consume it in add_nr_running() to
>>  do set_rd_overloaded() selectively.
> 
> If the global rd->overload is incorrectly cleared due to LBF_ALL_PINNED
> from src (CPUA) to dst (CPUB), it is possible that CPUB may experience
> no changes in nr_running for a certain period of time.

Why? Tasks can still wake up on it no?

All that clearing rq->overloaded does is indicate to newidle balance
that there aren't any CPUs with movable tasks on them and it is futile
to do any load balancing.

Do you have any numbers where Patch 1 specifically improves stuff?

> Therefore, I think
> modifying it in add_nr_running may not be appropriate. I'm not sure if my
> understanding is correct.

I was thinking something along the lines of:

  (Only build tested)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 5de115f67065..e78bbdab637f 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3000,6 +3000,15 @@ static int affine_move_task(struct rq *rq, struct task_struct *p, struct rq_flag
 			complete = true;
 		}
 
+		/*
+		 * At least one task on this rq might be movable again.
+		 * Check if rq->overloaded needs to be changed.
+		 */
+		if (rq->all_pinned && rq->nr_running > 1) {
+			set_rd_overloaded(rq->rd, 1);
+			rq->all_pinned = 0;
+		}
+
 		preempt_disable();
 		task_rq_unlock(rq, p, rf);
 		if (push_task) {
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 66e3b5cd5902..563327eb5ae8 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -13614,6 +13614,19 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
 		 */
 		cur_ld_moved = detach_tasks(&env);
 
+		/*
+		 * Indicate this rq currently has all its tasks pinned.
+		 * Next enqueue will reset rd->overloaded accordingly
+		 * if it was cleared during load balancing.
+		 *
+		 * XXX: Do this only when update_sd_lb_stats() clears
+		 * sd_overloaded? Can this be used to skip CPUs with
+		 * pinned tasks in sched_balance_find_src_rq()?
+		 */
+		if (!sd_parent &&
+		    ((env.flags & (LBF_DST_PINNED | LBF_ALL_PINNED)) == LBF_ALL_PINNED))
+			busiest->all_pinned = 1;
+
 		/*
 		 * We've detached some tasks from busiest_rq. Every
 		 * task is masked "TASK_ON_RQ_MIGRATING", so we can safely
@@ -13751,6 +13764,7 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
 
 		/* Record that we found at least one task that could run on this_cpu */
 		env.flags &= ~LBF_ALL_PINNED;
+		busiest->all_pinned = 0;
 
 		/*
 		 * ->active_balance synchronizes accesses to
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 5950391b873d..651a6e637e37 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1361,6 +1361,9 @@ struct rq {
 	struct cpuidle_state	*idle_state;
 #endif
 
+	unsigned char		all_pinned;
+				/* hole */
+
 	unsigned int		nr_pinned;
 	unsigned int		push_busy;
 	struct cpu_stop_work	push_work;
@@ -3060,8 +3063,10 @@ static inline void add_nr_running(struct rq *rq, unsigned count)
 		call_trace_sched_update_nr_running(rq, count);
 	}
 
-	if (prev_nr < 2 && rq->nr_running >= 2)
+	if ((prev_nr < 2 || rq->all_pinned) && rq->nr_running >= 2) {
 		set_rd_overloaded(rq->rd, 1);
+		rq->all_pinned = 0;
+	}
 
 	sched_update_tick_dependency(rq);
 }
-- 
Thanks and Regards,
Prateek