Re: [PATCH] sched/fair: Distribute burst-woken tasks when rq depth reaches 3x average

[Li kunyu]

Hi Prateek,

On 2026/11/5 11:44, K Prateek Nayak wrote:
> On 11/4/2026 2:48 PM, Li kunyu wrote:
>> +/*
>> + * Check if a CPU's rq depth reaches 3x the system-wide average tracked
>> + * in root_domain (scaled by SCHED_CAPACITY_SCALE, floored at 1 task).
>> + */
>> +static inline bool rq_burst_overloaded(int cpu)
>> +{
>> +	struct rq *rq = cpu_rq(cpu);
>> +	unsigned long avg;
>> +
>> +	if (unlikely(!rq->rd))
>> +		return false;
>> +
>> +	avg = max_t(unsigned long, READ_ONCE(rq->rd->avg_nr_running),
>> +		    SCHED_CAPACITY_SCALE);
>> +
>> +	return (READ_ONCE(rq->nr_running) << SCHED_CAPACITY_SHIFT) >= avg * 3;
> You can just store (avg * 3) >> SCHED_CAPACITY_SHIFT in rd and skip
> this math all the time.

Thanks for pointing this out; this part has been updated in the new code.

>> +}
>> +
>> +static inline void reset_periodic_lb_interval(int cpu)
>> +{
>> +	struct sched_domain *sd;
>> +
>> +	/*
>> +	 * Only the most basic values are updated here to avoid excessive
>> +	 * cost.
>> +	 */
>> +	for_each_domain(cpu, sd) {
>> +		if (READ_ONCE(sd->balance_interval) > sd->min_interval)
>> +			WRITE_ONCE(sd->balance_interval, sd->min_interval);
> Isn't 'sd->balance_interval' only stretched for LBF_ALL_PINNED cases?
> Why does this make a difference?

I don't quite understand this part. Should we remove the assignment to
sd->balance_interval?

Besides LBF_ALL_PINNED and active_balance, sd->balance_interval is also doubled
in the out_balanced / out_all_pinned /out_one_pinned paths when the system was
previously idle or balanced (if (sd->balance_interval < sd->max_interval)
sd->balance_interval *= 2;).

The assignment to sd->balance_interval has now been removed here, and the
following code has been added:

if (time_after(READ_ONCE(rq->next_balance), now))
        WRITE_ONCE(rq->next_balance, now);

The test results are quite good.

>> +	}
>> +}
>> +
>>  /*
>>   * select_task_rq_fair: Select target runqueue for the waking task in domains
>>   * that have the relevant SD flag set. In practice, this is SD_BALANCE_WAKE,
>> @@ -9972,11 +10004,19 @@ select_task_rq_fair(struct task_struct *p, int prev_cpu, int wake_flags)
>>  
>>  	/* Slow path */
>>  	if (unlikely(sd))
>> -		return sched_balance_find_dst_cpu(sd, p, cpu, prev_cpu, sd_flag);
>> -
>> +		new_cpu = sched_balance_find_dst_cpu(sd, p, cpu, prev_cpu, sd_flag);
>>  	/* Fast path */
>> -	if (wake_flags & WF_TTWU)
>> -		return select_idle_sibling(p, prev_cpu, new_cpu);
>> +	else if (wake_flags & WF_TTWU)
>> +		new_cpu = select_idle_sibling(p, prev_cpu, new_cpu);
> Why even do select_idle_sibling() if you are not going to use that
> data? Can't we check rq_burst_overloaded() beforehand?

The code has already been placed at this location, as follows:

@@ -9942,6 +9963,17 @@ select_task_rq_fair(struct task_struct *p, int prev_cpu,
int wake_flags)
                want_affine = !wake_wide(p) && cpumask_test_cpu(cpu, p->cpus_ptr);
        }

+       target_rq = want_affine ? this_rq() : cpu_rq(prev_cpu);
+       if (unlikely(rq_burst_overloaded(target_rq))) {
+               int dist_cpu = cpumask_any_and_distribute(p->cpus_ptr,
cpu_active_mask);
+
+               if (likely(dist_cpu < nr_cpumask_bits &&
cpu_rq(dist_cpu)->nr_running <
+                          target_rq->nr_running)) {
+                       reset_periodic_lb_interval(dist_cpu);
+                       return dist_cpu;
+               }
+       }
+
        for_each_domain(cpu, tmp) {


>> +
>> +	if (unlikely(rq_burst_overloaded(new_cpu))) {
>> +		int dist_cpu = cpumask_any_and_distribute(p->cpus_ptr, cpu_active_mask);
>> +
>> +		if (likely(dist_cpu < nr_cpumask_bits)) {
>> +			reset_periodic_lb_interval(new_cpu);
> This runs for every successful selection? Is that right? Why?

Here, the call has been updated to reset_periodic_lb_interval(dist_cpu);.

>> +			return dist_cpu;
>> +		}
>> +	}
>>  
>>  	return new_cpu;
>>  }
>> @@ -11685,6 +11725,7 @@ struct sg_lb_stats {
>>  struct sd_lb_stats {
>>  	struct sched_group *busiest;		/* Busiest group in this sd */
>>  	struct sched_group *local;		/* Local group in this sd */
>> +	unsigned long total_nr_running;		/* Sum of nr_running in this sd */
>>  	unsigned long total_load;		/* Total load of all groups in sd */
>>  	unsigned long total_capacity;		/* Total capacity of all groups in sd */
>>  	unsigned long avg_load;			/* Average load across all groups in sd */
>> @@ -12964,6 +13005,7 @@ static inline void update_sd_lb_stats(struct lb_env *env, struct sd_lb_stats *sd
>>  		/* Now, start updating sd_lb_stats */
>>  		sds->total_load += sgs->group_load;
>>  		sds->total_capacity += sgs->group_capacity;
>> +		sds->total_nr_running += sgs->sum_nr_running;
>>  
>>  		sum_util += sgs->group_util;
>>  		sg = sg->next;
>> @@ -12982,11 +13024,31 @@ static inline void update_sd_lb_stats(struct lb_env *env, struct sd_lb_stats *sd
>>  		env->fbq_type = fbq_classify_group(&sds->busiest_stat);
>>  
>>  	if (!env->sd->parent) {
> On a large, busy system, load balancing at topmost domain is only
> done once ever couple seconds and keeping the 'avg_nr_running' the
> same for that long seems dodgy.
> 
> An overshoot shouldn't be worse than what it is now, and an
> undershoot would at-least wait till 3 tasks on the CPU so it should
> be fine?
> 
>> +		unsigned int weight = env->sd->span_weight;
>> +
>>  		/* update overload indicator if we are at root domain */
>>  		set_rd_overloaded(env->dst_rq->rd, sg_overloaded);
>>  
>>  		/* Update over-utilization (tipping point, U >= 0) indicator */
>>  		set_rd_overutilized(env->dst_rq->rd, sg_overutilized);
>> +
>> +		/*
>> +		 * We update the average task count at the top-level (root)
>> +		 * domain, as this incurs the least overhead and comes
>> +		 * closest to the ideal scenario. The trade-off is that we
>> +		 * must accept some temporary data inaccuracy.
>> +		 */
>> +		if (weight) {
>> +			unsigned long cur_avg;
>> +			unsigned long old_avg;
>> +
>> +			cur_avg = (sds->total_nr_running << SCHED_CAPACITY_SHIFT) / weight;
>> +			old_avg = READ_ONCE(env->dst_rq->rd->avg_nr_running);
>> +			if (old_avg)
>> +				cur_avg = (old_avg * 7 + cur_avg) >> 3;
> Is this an open-coded update_avg()?

It has been changed to the following:

+               if (weight) {
+                       unsigned long thresh;
+
+                       thresh = max_t(unsigned long,
+                                      (sds->total_nr_running * 3) / weight, 3);
+
+                       if (READ_ONCE(env->dst_rq->rd->overload_nr_threshold) !=
thresh)
+
WRITE_ONCE(env->dst_rq->rd->overload_nr_threshold, thresh);
+               }
        } else if (sg_overutilized) {

>> +
>> +			WRITE_ONCE(env->dst_rq->rd->avg_nr_running, cur_avg);
> Can you do this selectively when old_avg != new_avg?
> 
> Also since you compare it with,
> 
> 	READ_ONCE(rq->nr_running) << SCHED_CAPACITY_SHIFT
> 
> Checking if (cur_avg * 3) >> SCHED_CAPACITY_SHIFT has changed and caching that
> would be better and save on computation during wakeup.
> 
>> +		}
>>  	} else if (sg_overutilized) {
>>  		set_rd_overutilized(env->dst_rq->rd, sg_overutilized);
>>  	}
>> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
>> index e656c7059bf8..4147e043407c 100644
>> --- a/kernel/sched/sched.h
>> +++ b/kernel/sched/sched.h
>> @@ -1077,6 +1077,9 @@ struct root_domain {
>>  	cpumask_var_t		rto_mask;
>>  	struct cpupri		cpupri;
>>  
>> +	/* Scaled (SCHED_CAPACITY_SCALE) average per-CPU rq depth */
>> +	unsigned long		avg_nr_running;
>> +
>>  	/*
>>  	 * NULL-terminated list of performance domains intersecting with the
>>  	 * CPUs of the rd. Protected by RCU.

-- 
thanks,
kunyu