Re: [PATCH 4/6] sched_ext: Report preferred NUMA nodes through ops.set_numa_node()

[Andrea Righi]

On Tue, Oct 27, 2026 at 11:49:01AM +0100, Peter Zijlstra wrote:
> On Tue, Oct 27, 2026 at 05:13:33AM +0100, Andrea Righi wrote:
> 
> > @@ -4528,6 +4543,15 @@ static void __scx_enable_task(struct scx_sched *sch, struct task_struct *p)
> >  
> >  	if (SCX_HAS_OP(sch, set_weight))
> >  		SCX_CALL_OP_TASK(sch, set_weight, rq, p, p->scx.weight);
> > +
> > +	if (SCX_HAS_OP(sch, set_numa_node)) {
> > +		s32 nid = NUMA_NO_NODE;
> > +
> > +#ifdef CONFIG_NUMA_BALANCING
> > +		nid = p->numa_preferred_nid;
> > +#endif
> > +		SCX_CALL_OP_TASK(sch, set_numa_node, rq, p, nid);
> > +	}
> >  }
> >  
> >  void scx_enable_task(struct scx_sched *sch, struct task_struct *p)
> > @@ -4834,6 +4858,20 @@ void sched_ext_dead(struct task_struct *p)
> >  	}
> >  }
> >  
> > +#ifdef CONFIG_NUMA_BALANCING
> > +static void numa_node_changed_scx(struct rq *rq, struct task_struct *p, int nid)
> > +{
> > +	struct scx_sched *sch = scx_task_sched(p);
> > +
> > +	if (task_dead_and_done(p) ||
> > +	    scx_get_task_state(p) != SCX_TASK_ENABLED)
> > +		return;
> > +
> > +	if (SCX_HAS_OP(sch, set_numa_node))
> > +		SCX_CALL_OP_TASK(sch, set_numa_node, rq, p, nid);
> > +}
> > +#endif
> 
> I dont get this; why are you calling set_numa_mode() on enable? The
> signal was sched_setnuma(), but now you're doing more.
> 
> Should not the existing enable method check p->numa_preferred_nid rather
> than injecting this weird extra call?

This follows what sched_ext already does with the other "set" callbacks:
ops.set_weight() and ops.set_cpumask() are also invoked at enable with
the initial value, right after ops.enable(), so that a BPF scheduler can
track that property in a single callback, instead of reading the initial
value in ops.enable() and then tracking the changes in a separate
callback.

That said, ops.enable() can also read the initial value directly with
scx_bpf_task_numa_nid() (introduced in patch 5), so if we think it's
cleaner I can drop the call at enable and document that ops.enable()
should do that.

Thanks,
-Andrea