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

[Peter Zijlstra <peterz@infradead.org>]

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.

> 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 :-)


> +#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;
  }

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?