Re: [PATCH 2/6] sched/numa: Leave the placement of a BPF-scheduled task to its scheduler

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Oct 27, 2026 at 12:09:33PM +0100, Andrea Righi wrote:
> Hi Peter,
> 
> On Tue, Oct 27, 2026 at 11:43:17AM +0100, Peter Zijlstra wrote:
> > On Tue, Oct 27, 2026 at 05:13:31AM +0100, Andrea Righi wrote:
> > > A task under a BPF scheduler can take NUMA hinting faults via
> > > task_numa_fault() and then reach numa_migrate_preferred(), which may
> > > migrate the task to its preferred node through task_numa_migrate().
> > > 
> > > The migration of such a task should always be driven by its BPF
> > > scheduler, which owns the placement of its tasks.
> > > 
> > > Keep collecting the statistics, which is what makes
> > > p->numa_preferred_nid worth reading, but skip the migration for a task
> > > owned by a BPF scheduler. The scheduler can read the preferred node and
> > > decide for itself whether moving the task is worth the cost.
> > > 
> > > For the same reason, never pick a task owned by a BPF scheduler as the
> > > swap partner of another task that NUMA balancing migrates, e.g., a fair
> > > task when the BPF scheduler runs in partial mode. Such a task is
> > > treated like a kthread on the destination CPU: the migrating task can
> > > still move there alone, but the BPF-scheduled task is left in place.
> > > 
> > > Reported-by: Vladimir Vdovin <deliran@verdict.gg>
> > > Link: https://lore.kernel.org/r/20261002124559.10367-1-deliran@verdict.gg
> > > Signed-off-by: Andrea Righi <arighi@nvidia.com>
> > > ---
> > >  kernel/sched/fair.c | 15 ++++++++++++++-
> > >  1 file changed, 14 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > > index 36df9a4dde534..5064c7b707f4d 100644
> > > --- a/kernel/sched/fair.c
> > > +++ b/kernel/sched/fair.c
> > > @@ -3267,8 +3267,13 @@ static bool task_numa_compare(struct task_numa_env *env,
> > >  
> > >  	rcu_read_lock();
> > >  	cur = rcu_dereference_all(dst_rq->curr);
> > > +	/*
> > > +	 * Never pick a task under a BPF scheduler as the swap partner: its
> > > +	 * placement is left to that scheduler, see numa_migrate_preferred().
> > > +	 * Moving the faulting task alone to the destination is still allowed.
> > > +	 */
> > >  	if (cur && ((cur->flags & (PF_EXITING | PF_KTHREAD)) ||
> > > -		    !cur->mm))
> > > +		    !cur->mm || task_on_scx(cur)))
> > >  		cur = NULL;
> > 
> > Rather than check task_on_scx(); your argument with Prateek seems to
> > highlight that the actual check you're looking for should be something
> > like: cur->class != &fair_sched_class or somesuch, right?
> 
> Right, the actual condition is "cur is not a fair task". I used
> task_on_scx() only to avoid changing the behavior of RT/DL tasks, but
> the same arguments apply to them: their se.avg isn't updated either, so
> task_h_load(cur) is stale, and their placement is driven by their own
> class (push/pull), so fair shouldn't move them around.
> 
> So yeah... "task not in the fair class" seems the right thing to do.
> 
> > 
> > >  	/*
> > > @@ -3645,6 +3650,14 @@ static void numa_migrate_preferred(struct task_struct *p)
> > >  	if (task_node(p) == p->numa_preferred_nid)
> > >  		return;
> > >  
> > > +	/*
> > > +	 * A task under a BPF scheduler is placed by that scheduler. Keep the
> > > +	 * statistics coming, which is what makes p->numa_preferred_nid worth
> > > +	 * reading, but leave the placement alone.
> > > +	 */
> > > +	if (task_on_scx(p))
> > > +		return;
> > > +
> > >  	/* Otherwise, try migrate to a CPU on the preferred node */
> > >  	task_numa_migrate(p);
> > >  }
> > 
> > I am somewhat confused on all this still, probably because I've not
> > looked at it for more than a couple of minutes at this point :-)
> > 
> > But rather than putting a callback in sched_setnuma() should you not put
> > a class call here?
> > 
> > That is; have this be:
> > 
> > 	p->class->task_numa_migrate(p);
> > 
> > and have fair set task_numa_migrate_fair(), which is the current
> > function, and then ext can implement the BPF thing.
> 
> Yeah, that'd be a much cleaner approach than special-casing ext in fair.
> But I think it gives a different signal than the sched_setnuma()
> callback.
> 
> numa_migrate_preferred() is called from task_numa_fault(), in the
> context of the faulting task, at most every numa_migrate_retry, and it
> only gets to this point when the task isn't on its preferred node. So a
> class call here means "this task is misplaced, migrate it if you want",
> repeated at each retry, even if the preferred node remains the same.

This is an important signal; it is fundamental to the whole controller.
Without a force to keep a task on its preferred node, the memory will
keep migrating around for no purpose.

A task really should be moved 'back' to its preferred node it at all
possible, and it remaining to run outside of it had better have a good
reason (like imbalance). Of course, if a task is ran elsewhere, then
eventually memory will migrate to the 'new' place and then a new
preferred node will be selected.

The new place is typically constrained by the numa distance, when its
not possible to run on the preferred node, it tries to run near, which
then limits the possible nodes and will have it converge on a new node.

The recently added cache-aware will help here, since that is a short
term signal to stick to one place, ensuring the memory migration is
also likely to be more stable and the whole thing will converge on a new
node more quickly.

> The sched_setnuma() callback instead reports when the preferred node
> changes, which is what a BPF scheduler needs to use the preferred node
> as a placement hint.
> 
> So I think the two are complementary: the class call would replace the
> task_on_scx() check here (ext can just skip the migration, or expose it
> to BPF if we think it's useful), while the notification would stay in
> sched_setnuma().

I am still not quite sure I understand why sched_ext needs the
sched_setnuma() hint for placement.

> One thing to consider: RT/DL tasks would also stop being migrated here
> (they can take hinting faults too), which seems more correct honestly:
> task_numa_migrate() decides the move based on task_h_load(p), which is
> stale for them, and their faults only come from pages scanned by other
> fair threads of the same mm, since they don't drive the scanning
> themselves.
> 
> Would that work for you?

RT/DL should definitely be ignored. 

> > Anyway, if you want to do this task_on_scx() thing like you did, its in
> > the wrong spot, that condition right above it is irrelevant if you're
> > not going to be calling task_numa_migrate(). 
> 
> Right, I'll move it before that check. It still needs to stay after the
> p->numa_migrate_retry update though, otherwise the time_after() check in
> task_numa_fault() would be always true for scx tasks and
> task_numa_placement() would be called at every hinting fault.

Just so.