Re: [PATCH v13 08/13] sched/core: Push current task from non preferred CPU

[Shrikanth Hegde <sshegde@linux.ibm.com>]

Hi Peter.

On 9/25/26 1:19 PM, Peter Zijlstra wrote:
> On Wed, Sep 09, 2026 at 07:26:12PM +0530, Shrikanth Hegde wrote:
>> Actively push out the current running task on a non-preferred CPU. Since
>> the task is currently running, a stopper thread must be queued to push the
>> task out. However, if the task is pinned only to non-preferred CPUs,
>> it will continue running there. This helps to maintain userspace
>> affinities, unlike CPU hotplug or isolated cpusets.
>>
>> Though the code is similar to __balance_push_cpu_stop and quite close to
>> push_cpu_stop, it is kept separate as it provides a cleaner
>> implementation specifically for CONFIG_PREFERRED_CPU.
>

> I would have preferred the code looking more like
> __balance_push_cpu_stop(). Now you're written more or less the same, but
> visually different for no reason. This makes comparing them unnecessarily
> hard.
> 

Yes. I will keep your version at the end and respin v14. Hopefully by Monday.

>> Add the npc_push_work_pending flag to protect the work buffer.
> 
> I can't help but read NPC as Non-Playing-Character. Too much RPGs, and
> that ain't Rocket Propelled Grenades. TLA are such fun :-)
> 

hehe. one more into the mix.
> 
>> +#ifdef CONFIG_PREFERRED_CPU
>> +static DEFINE_PER_CPU(struct cpu_stop_work, npc_push_task_work);
>> +
>> +static int sched_non_preferred_cpu_push_stop(void *arg)
>> +{
>> +	struct task_struct *p = arg;
>> +	struct rq *rq = this_rq();
>> +	struct rq_flags rf;
>> +	int cpu;
>> +
>> +	if (cpu_preferred(rq->cpu)) {
>> +		scoped_guard(rq_lock_irqsave, rq)
>> +			rq->npc_push_work_pending = false;
>> +		put_task_struct(p);
>> +		return 0;
>> +	}
>> +
>> +	raw_spin_lock_irq(&p->pi_lock);
>> +
>> +	/*
>> +	 * select_fallback_rq() may acquire the rq lock in case of fallback.
>> +	 * So call it before grabbing rq lock. If the task migrates to
>> +	 * another CPU before the rq lock is acquired, subsequent validation
>> +	 * of task's current rq will help to safely bail out.
>> +	 */
>> +	cpu = select_fallback_rq(rq->cpu, p);
>> +	rq_lock(rq, &rf);
>> +	rq->npc_push_work_pending = false;
>> +	update_rq_clock(rq);
>> +
>> +	context_unsafe_alias(rq);
>> +
>> +	if (task_rq(p) == rq && task_on_rq_queued(p) &&
>> +	    !is_migration_disabled(p))
>> +		rq = __migrate_task(rq, &rf, p, cpu);
>> +
>> +	rq_unlock(rq, &rf);
>> +	raw_spin_unlock_irq(&p->pi_lock);
>> +	put_task_struct(p);
>> +
>> +	return 0;
>> +}
> 
> Anyway, I ended up with:
> 
>    static int sched_non_preferred_cpu_push_stop(void *arg)
>    {
> 	  struct task_struct *p = arg;
> 	  struct rq *rq = this_rq();
> 	  struct rq_flags rf;
> 	  int cpu;
> 
> 	  if (cpu_preferred(rq->cpu)) {
> 		  scoped_guard (rq_lock_irqsave, rq)
> 			  rq->npc_push_work_pending = false;
> 		  put_task_struct(p);
> 		  return 0;
> 	  }
> 
> 	  scoped_guard (raw_spinlock_irq, &p->pi_lock) {
> 		  /*
> 		   * select_fallback_rq() may acquire the rq lock in case of
> 		   * fallback. So call it before grabbing rq lock. If the task
> 		   * migrates to another CPU before the rq lock is acquired,
> 		   * subsequent validation of task's current rq will help to
> 		   * safely bail out.
> 		   */
> 		  cpu = select_fallback_rq(rq->cpu, p);
> 
> 		  rq_lock(rq, &rf);
> 		  rq->npc_push_work_pending = false;
> 		  update_rq_clock(rq);
> 
> 		  context_unsafe_alias(rq);
> 
> 		  if (task_rq(p) == rq && task_on_rq_queued(p) &&
> 		      !is_migration_disabled(p))
> 			  rq = __migrate_task(rq, &rf, p, cpu);
> 		  rq_unlock(rq, &rf);
> 	  }
> 
> 	  put_task_struct(p);
> 
> 	  return 0;
>    }
> 

Alright. Will keep the above. (without the additional migrate check)

> And note how you have an extra !is_migration_disabled() vs
> __balance_push_cpu_stop(). Either yours is superfluous or
> __balance_push_cpu_stop() should have one. Which is it?
> 

yes. It is superfluous. Queue side check is sufficient.
Will remove it.
1. Even if task got pulled by remote CPU before stopper got to run,
    the task_rq check will fail and bail out.
2. Since the policy is currently for FAIR tasks only, it can't run
    before stopper.