Re: [PATCH v2 1/2] sched/numa: Drive NUMA task tick from execution context

[Tim Chen]

On Sat, 2026-09-05 at 00:10 +0800, Chen Yu wrote:
> On Thu, Sep 03, 2026 at 02:30:25PM -0700, Tim Chen wrote:
> > On Thu, 2026-09-03 at 20:41 +0800, Chen, Yu C wrote:
> 
> [ ... ]
> 
> > > But I also see that in task_tick_core(), the sum_exec_runtime is also 
> > > leveraged
> > > to calculate the delta "wall time" via __entity_slice_used():
> > > se->sum_exec_runtime - se->prev_sum_exec_runtime
> > > does it mean task_tick_core() also needs to be bring one level up to 
> > > sched_tick()
> > > and passed with rq->curr?
> > 
> > I think task_tick_core() needs to stay with the donor's context
> > as it is the scheduling context.
> > 
> > task_tick_core() is not about the execution context -- 
> > it decides whether the current scheduling context has used
> > up enough of its slice to let a force-idled SMT sibling run. That
> > slice belongs to the donor, so the donor is the right task to pass.
> > 
> > There is a separate issue lurking here, task_tick_core() measures consumed slice as
> > se->sum_exec_runtime - se->prev_sum_exec_runtime. Under proxy the
> > donor's sum_exec_runtime does not advance (update_se() charges the
> > runtime to rq->curr instead), so that delta stays near zero and the
> > force-idle resched may never trigger. 
> > 
> > Passing rq->curr does not fix it either. __entity_slice_used() takes
> > the runtime and the slice from the same entity, so passing rq->curr
> > just compares the running task against its own slice. But this check
> > is about the donor: it asks whether the scheduling context that owns
> > the CPU has used up its slice. The running task is only borrowing the
> > CPU through proxy, so its slice is not the one we care about here.
> > 
> 
> Got it, I see.
> 
> > Maybe something like below (only compile tested) to fix the issue.
> > That said, this is somewhat orthogonal to the issue that the execution context
> > series is trying to solve. It should be fixed separately.
> > 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 8dff37059faf..cd240bf52d03 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -14748,10 +14748,29 @@ static void rq_offline_fair(struct rq *rq)
> >  static inline bool
> >  __entity_slice_used(struct sched_entity *se, int min_nr_tasks)
> >  {
> > -	u64 rtime = se->sum_exec_runtime - se->prev_sum_exec_runtime;
> > -	u64 slice = se->slice;
> > +	u64 vslice, vused;
> >  
> > -	return (rtime * min_nr_tasks > slice);
> > +	/*
> > +	 * @se is the scheduling context (rq->donor). Under proxy execution
> > +	 * it need not be the task executing on the CPU, so its
> > +	 * sum_exec_runtime is not advanced and cannot be used to tell how
> > +	 * much of its slice it has consumed. Its vruntime, however, is
> > +	 * advanced by update_curr() with the proxy runtime, and its EEVDF
> > +	 * deadline reflects the granted slice, so measure the consumed
> > +	 * fraction in virtual time instead.
> > +	 *
> > +	 * This is equivalent to the previous real-time comparison in the
> > +	 * non-proxy case: both @vused and @vslice are scaled by the same
> > +	 * weight factor, so the ratio (and thus the min_nr_tasks test) is
> > +	 * unchanged.
> > +	 */
> > +	if (vruntime_cmp(se->vruntime, ">=", se->deadline))
> > +		return true;
> > +
> > +	vslice = calc_delta_fair(se->slice, se);
> > +	vused = vslice - (se->deadline - se->vruntime);
> > +
> > +	return (vused * min_nr_tasks > vslice);
> >  }
> 
> This fix looks good to me. And just one minor question that I'm
> trying to figure out:
> 
> Consider that there is only one running task p on one of the SMT siblings.
> The original comparison is between:
> se->sum_exec_runtime - se->prev_sum_exec_runtime vs slice,
> and since there is only one runnable task, p continues to run
> without any preemption, so se->prev_sum_exec_runtime remains
> unchanged, while se->sum_exec_runtime moves forward. Therefore,
> the duration delta of se->sum_exec_runtime - se->prev_sum_exec_runtime
> could expand to many slices in theory.
> After switching to the vruntime-based comparison, even
> if p has not been preempted, se->deadline together with se->vruntime
> will move forward by update_dealine(). That is to say, we now only
> consider the delta within one slice. 
> 

You raised a good point. And coupled with Hui's comment make me realize
that the deadline advancement in each slice means that I am only checking
whether I am using up my quota in the current slice.

I should check the run time since the task was picked to run to
see if the donor has exceeded its allotment if it keeps running
across slices in the lone task case.

I replied to Hui's email with a new proposal.

Tim

> This seems to tighten the
> restriction for task_tick_core() to trigger a force reschedule.
> But Overall I think it should not be a good deal to check
> within a slice period.
> 
> thanks,
> Chenyu