RE: [PATCH v3 1/3] sched_ext: Fix pnt_seq calculation

[liuwenfang <liuwenfang@honor.com>]

> Hello,
> 
> On Mon, Aug 11, 2025 at 02:03:16PM -1000, 'Tejun Heo' wrote:
> ...
> > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index
> > > 0fb9bf995..50d757e92 100644
> > > --- a/kernel/sched/fair.c
> > > +++ b/kernel/sched/fair.c
> > > @@ -8887,6 +8887,9 @@ pick_next_task_fair(struct rq *rq, struct
> > > task_struct *prev, struct rq_flags *rf
> > >
> > >  	__put_prev_set_next_dl_server(rq, prev, p);
> > >
> > > +	if (scx_enabled())
> > > +		scx_put_prev_set_next(rq, prev, p);
> > > +
> > >  	/*
> > >  	 * Because of the set_next_buddy() in dequeue_task_fair() it is rather
> > >  	 * likely that a next task is from the same cgroup as the current.
> > > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index
> > > 47972f34e..bcb7f175c 100644 @@ -2465,6 +2470,9 @@ static inline void
> > > put_prev_set_next_task(struct rq *rq,
> > >
> > >  	__put_prev_set_next_dl_server(rq, prev, next);
> > >
> > > +	if (scx_enabled())
> > > +		scx_put_prev_set_next(rq, prev, next);
> > > +
> > >  	if (next == prev)
> > >  		return;
> >
> > I'm not sure these are the best spots to call this function. How about
> > putting it in the CONFIG_SCHED_CLASS_EXT section in prev_balance()?
> > The goal of the seq counter is to wait for scheduler path to be
> > entered, so that's good enough a spot and there already is scx
> > specific section, so it doesn't add too much noise.
> 
> Strike that. I see that we need a hook after task is picked to resolve the bug
> around cpu_released. Can you please move scx_enabled() test into
> scx_put_prev_set_next() and add a helper which calls both
> __put_prev_set_next_dl_server() and scx_put_prev_set_next() so that the call
> doesn't have to be added to two places?
Thanks for your feedback.

__put_prev_set_next is added here as the helper, the fixed function is:

+void scx_put_prev_set_next(struct rq *rq, struct task_struct *prev,
+                          struct task_struct *next)
+{
+       if (!scx_enabled())
+               return;
+
+#ifdef CONFIG_SMP
+       /*
+        * Pairs with the smp_load_acquire() issued by a CPU in
+        * kick_cpus_irq_workfn() who is waiting for this CPU to perform a
+        * resched.
+        */
+       smp_store_release(&rq->scx.pnt_seq, rq->scx.pnt_seq + 1);
+#endif
+}

+static inline void __put_prev_set_next(struct rq *rq,
+                                      struct task_struct *prev,
+                                      struct task_struct *next)
+{
+       __put_prev_set_next_dl_server(rq, prev, next);
+       scx_put_prev_set_next(rq, prev, next);
+}
+
 static inline void put_prev_set_next_task(struct rq *rq,
                                          struct task_struct *prev,
                                          struct task_struct *next)
 {
        WARN_ON_ONCE(rq->curr != prev);

-       __put_prev_set_next_dl_server(rq, prev, next);
+       __put_prev_set_next(rq, prev, next);

        if (next == prev)

pick_next_task_fair(struct rq *rq, struct task_struct *prev, struct rq_flags *rf
        if (prev->sched_class != &fair_sched_class)
                goto simple;

-       __put_prev_set_next_dl_server(rq, prev, p);
+       __put_prev_set_next(rq, prev, p);

Any suggestions will be appreciated and a formal patch will be sent out later.
> 
> Thanks.
> 
> --
> Tejun
Thanks.

Wenfang