Re: [PATCH 3/6] sched/numa: Notify the scheduling class of preferred node changes

[Andrea Righi <arighi@nvidia.com>]

Hi Prateek,

On Tue, Oct 27, 2026 at 11:00:17AM +0530, K Prateek Nayak wrote:
> Hello Andrea,
> 
> On 10/27/2026 9:43 AM, Andrea Righi wrote:
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -8331,8 +8331,13 @@ int migrate_task_to(struct task_struct *p, int target_cpu)
> >  void sched_setnuma(struct task_struct *p, int nid)
> >  {
> >  	guard(task_rq_lock)(p);
> > -	scoped_guard (sched_change, p, DEQUEUE_SAVE)
> > +	scoped_guard (sched_change, p, DEQUEUE_SAVE) {
> > +		bool changed = p->numa_preferred_nid != nid;
> 
> Don't the callers of sched_setnuma() already check for this?
> 
> Only one that doesn't is task_numa_migrate() but
> numa_migrate_preferred() only calls it when
> "task_node(p) == p->numa_preferred_nid" is false and "!sd "case resets
> "p->numa_preferred_nid" to "task_node(p)" when the task cannot be
> migrated.
> 
> Afaict, "changed" is always true here. Is there a scenario that I might
> be missing?

With the current callers it's always true for the tasks that can reach
the callback.

 added the check just because ->numa_node_changed() is a generic
sched_class callback and its contract is that it's invoked only when the
preferred node actually changes. Checking it in sched_setnuma(), under
the rq lock we're already holding, makes sure that the contract is
honored independently of what the callers of sched_setnuma() are doing,
now and in the future, instead of relying on each of them to check it.

That said, I don't have a strong opinion on that, if we prefer to keep
sched_setnuma() minimal, I can drop the check and document that the
callers must only invoke it on a change.

Thanks,
-Andrea

> 
> > +
> >  		p->numa_preferred_nid = nid;
> > +		if (changed && p->sched_class->numa_node_changed)
> > +			p->sched_class->numa_node_changed(task_rq(p), p, nid);
> > +	}
> >  }
> >  #endif /* CONFIG_NUMA_BALANCING */
> 
> -- 
> Thanks and Regards,
> Prateek
>