[PATCH] sched/fair: Fix the runtime check in preempt_sync()

[PATCH] sched/fair: Fix the runtime check in preempt_sync()

[Peng Wang]

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@linux.alibaba.com>
---
 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) {
-- 
2.47.0

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4

Re: [PATCH] sched/fair: Fix the runtime check in preempt_sync()

[Peter Zijlstra]

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@linux.alibaba.com>
> ---
>  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?

Re: [PATCH] sched/fair: Fix the runtime check in preempt_sync()

[Peng Wang]

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@linux.alibaba.com>
> > ---
> >  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