linux-kernel - RE: [PATCH v3 1/3] sched_ext: Fix pnt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b1898ea1365d460e89b64989304ea0f7@honor.com>
Date: Mon, 18 Aug 2025 10:45:55 +0000
From: liuwenfang <liuwenfang@...or.com>
To: 'Tejun Heo' <tj@...nel.org>
CC: 'David Vernet' <void@...ifault.com>, 'Andrea Righi' <arighi@...dia.com>,
	'Changwoo Min' <changwoo@...lia.com>, 'Ingo Molnar' <mingo@...hat.com>,
	'Peter Zijlstra' <peterz@...radead.org>, 'Juri Lelli'
	<juri.lelli@...hat.com>, 'Vincent Guittot' <vincent.guittot@...aro.org>,
	'Dietmar Eggemann' <dietmar.eggemann@....com>, 'Steven Rostedt'
	<rostedt@...dmis.org>, 'Ben Segall' <bsegall@...gle.com>, 'Mel Gorman'
	<mgorman@...e.de>, 'Valentin Schneider' <vschneid@...hat.com>,
	"'linux-kernel@...r.kernel.org'" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v3 1/3] sched_ext: Fix pnt_seq calculation

> 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