Re: [PATCH] sched/fair: Fix the runtime check in preempt_sync()
From: Peng Wang
Date: Fri Nov 06 2026 - 07:21:01 EST
On Fri, Nov 06, 2026 at 11:31:40AM +0100, Peter Zijlstra wrote:
> On Fri, Nov 06, 2026 at 04:50:00PM +0800, Peng Wang wrote:
> > With NEXT_BUDDY enabled, preempt_sync() only lets a WF_SYNC wakeup
> > force a reschedule once the current task has run long enough. It
> > computes the runtime as rq_clock_task() - se->exec_start, but
> > wakeup_preempt_fair() has already called update_curr_fair() at that
> > point, which sets se->exec_start to rq_clock_task(). The result is
> > always 0.
> >
> > Use sum_exec_runtime - prev_sum_exec_runtime instead, which is the time
> > the task has run since it was picked, and drop the now unused rq
> > argument.
> >
> > Fixes: e837456fdca8 ("sched/fair: Reimplement NEXT_BUDDY to align with EEVDF goals")
> > Signed-off-by: Peng Wang <peng_wang@xxxxxxxxxxxxxxxxx>
> > ---
> > kernel/sched/fair.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index cfc9822..6164cb7 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -10090,8 +10090,8 @@ static inline bool set_short_buddy(struct cfs_rq *cfs_rq, struct sched_entity *p
> > * multiple tasks must be woken up.
> > */
> > static inline enum preempt_wakeup_action
> > -preempt_sync(struct rq *rq, int wake_flags,
> > - struct sched_entity *pse, struct sched_entity *se)
> > +preempt_sync(int wake_flags, struct sched_entity *pse,
> > + struct sched_entity *se)
> > {
> > u64 threshold, delta;
> >
> > @@ -10102,7 +10102,7 @@ preempt_sync(struct rq *rq, int wake_flags,
> > WARN_ON_ONCE(!(wake_flags & WF_TTWU));
> >
> > threshold = sysctl_sched_migration_cost;
> > - delta = rq_clock_task(rq) - se->exec_start;
> > + delta = se->sum_exec_runtime - se->prev_sum_exec_runtime;
> > if ((s64)delta < 0)
> > delta = 0;
> >
> > @@ -10226,7 +10226,7 @@ static void wakeup_preempt_fair(struct rq *rq, struct task_struct *p, int wake_f
> > * waker and less likely to be cache hot.
> > */
> > if (wake_flags & WF_SYNC)
> > - preempt_action = preempt_sync(rq, wake_flags, pse, se);
> > + preempt_action = preempt_sync(wake_flags, pse, se);
> > }
> >
> > switch (preempt_action) {
>
> Well yes. Would you also happen to have some number on how this fix
> affects performance of WF_SYNC workloads?
> Well yes. Would you also happen to have some number on how this fix
> affects performance of WF_SYNC workloads?
Yes. I compared v7.4-rc1 with and without the fix on a bare-metal
4-socket Xeon Platinum 8369HB system. Both kernels had NEXT_BUDDY
and HRTICK enabled, with PREEMPT_FULL and HZ=1000.
The workloads were free to migrate across eight physical cores on one
NUMA node, with memory bound to that node. SMT remained enabled.
I used four boots in baseline/patched/baseline/patched order, with
three runs per workload per boot. The table shows the two boot medians
for each kernel; changes are calculated from their mean.
Hackbench exercises sync wakeups through pipe and UNIX-socket I/O.
Light used -g 1 -f 2 -l 1000000; heavy used -g 4 -f 20 -l 25000.
hackbench, seconds base fixed change
-P pipe light 1.244 1.250 1.252 1.225 -0.7%
-P pipe heavy 3.552 3.454 3.408 3.378 -3.1%
-P socket light 1.411 1.435 1.340 1.355 -5.3%
-P socket heavy 5.644 5.695 5.673 5.686 +0.2%
-T pipe light 1.346 1.332 1.333 1.299 -1.7%
-T pipe heavy 3.835 3.731 3.851 3.732 +0.2%
-T socket light 1.406 1.419 1.480 1.386 +1.5%
-T socket heavy 6.006 6.003 5.988 5.997 -0.2%
I also ran loopback TCP_RR with 1-byte requests and replies for
10 seconds per run. The eight-client rate is the sum of the
concurrent clients' rates.
TCP_RR, trans/s base fixed change
1 client 87676 88581 88479 87534 -0.1%
8 clients 812475 838587 815175 801202 -2.1%
Eight-client TCP_RR was 2.1% lower overall. The two successive A/B
comparisons were +0.33% and -4.46%, so the decrease was not consistent
across the two pairs.
Thanks,
Peng