Re: [PATCH sched_ext/for-7.3] tools/sched_ext: scx_pair: Convert to sched_switch TP

Hi Andrea, thanks for the review :)

On Tue, Jul 21, 2026 at 09:37:50PM +0200, Andrea Righi wrote:
> Hi Cheng-Yang,
> 
> On Sun, Jul 19, 2026 at 10:28:45PM +0800, Cheng-Yang Chou wrote:
> > ops.cpu_acquire/release() are deprecated in favor of tracking CPU
> > preemption from a sched_switch tracepoint, see
> > commit a3f5d4822253 ("sched_ext: Allow scx_bpf_reenqueue_local() to
> > be called from anywhere"). Loading scx_pair currently emits a
> > deprecation warning.
> > 
> > Replace the pair_cpu_acquire/release() callbacks with a
> > tp_btf/sched_switch program that edge-detects the same transitions the
> > core used to deliver: a release when a running SCX task loses its CPU
> > to a higher-priority class, and an acquire when the CPU switches back
> > to an SCX task or idle while marked preempted.
> > 
> > Tasks are classified by effective priority (p->prio) rather than by
> > policy: rt_mutex_setprio() boosts a PI beneficiary into the rt/dl
> > classes while leaving its policy untouched, so a policy test would
> > both miss the release when a boosted task takes the CPU and fire a
> > spurious acquire when a boosted task replaces a real rt task.
> > 
> > A switch from idle straight to a higher-priority task is deliberately
> > not treated as a release. The CPU was not running an SCX task, so
> > there is nothing to drain, and kicking SCX_KICK_PREEMPT |
> > SCX_KICK_WAIT on every rt wakeup would make the pair CPU wait out rt
> > bursts it was never coupled to. The old callbacks behaved the same
> > way, firing ops.cpu_release() only from switch_class() when an SCX
> > task was put for a higher class.
> > 
> > The tracepoint runs on every context switch in the system, so the
> > common no-transition case is filtered before taking the pair-shared
> > lock. This is safe because a CPU's own preempted_mask bit is only ever
> > written by this tracepoint running on that CPU.
> > 
> > sched_setscheduler() on a running task changes class in place without
> > a context switch, so such transitions are only observed at the task's
> > next switch. The old callbacks had the same blind spot in
> > switch_class(), and try_dispatch() already bounds the resulting wait.
> > 
> > Verified in virtme-ng with the script below. The scheduler must load
> > without the deprecation warning, stay enabled through the rt churn and
> > the idle soak (the watchdog would otherwise abort it with "runnable
> > task stall"), keep its preemption counter advancing, and unregister
> > cleanly at the end. A PI rt-mutex churn that repeatedly boosts
> > SCX tasks into the rt class was exercised separately:
> > 
> >   #!/bin/bash
> >   # vng --verbose --cpus 8 -m 4G --user root -- ./verify.sh
> >   # FIFO harness: survives even if all SCHED_NORMAL tasks stall
> >   [ "${RT:-0}" = 1 ] || exec chrt -f 5 env RT=1 "$0"
> > 
> >   chrt -o 0 ./tools/sched_ext/build/bin/scx_pair &
> >   PAIR=$!
> >   sleep 3
> >   for round in $(seq 10); do
> >     pids=""
> >     for i in 0 1 2 3; do  # SCHED_FIFO churn
> >       chrt -f 10 bash -c \
> >         'e=$((SECONDS+1)); while [ $SECONDS -lt $e ]; do :; done' &
> >       pids="$pids $!"
> >     done
> >     for i in 0 1; do      # SCHED_NORMAL load under scx
> >       chrt -o 0 bash -c \
> >         'n=0; while [ $n -lt 200000 ]; do n=$((n+1)); done' &
> >       pids="$pids $!"
> >     done
> >     wait $pids            # explicit pids, not the scx_pair job
> >   done
> >   sleep 300               # idle soak
> >   kill -INT $PAIR         # expect clean unregister in dmesg
> > 
> > Signed-off-by: Cheng-Yang Chou <[email protected]>
> 
> This looks good to me.
> 
> Reviewed-by: Andrea Righi <[email protected]>
> 
> Thanks,
> -Andrea
> 
> > ---
> >  tools/sched_ext/scx_pair.bpf.c | 136 ++++++++++++++++++++++-----------
> >  1 file changed, 90 insertions(+), 46 deletions(-)
> > 
> > diff --git a/tools/sched_ext/scx_pair.bpf.c b/tools/sched_ext/scx_pair.bpf.c
> > index 267011b57cba..0d61b7b812db 100644
> > --- a/tools/sched_ext/scx_pair.bpf.c
> > +++ b/tools/sched_ext/scx_pair.bpf.c
> > @@ -93,12 +93,13 @@
> >   * -----------------------
> >   *
> >   * SCX is the lowest priority sched_class, and could be preempted by them at
> > - * any time. To address this, the scheduler implements pair_cpu_release() and
> > - * pair_cpu_acquire() callbacks which are invoked by the core scheduler when
> > - * the scheduler loses and gains control of the CPU respectively.
> > + * any time. To address this, the scheduler watches every sched_switch from
> > + * a tracepoint and edge-detects when a CPU leaves and returns to SCX
> > + * control.
> >   *
> > - * In pair_cpu_release(), we mark the pair_ctx as having been preempted, and
> > - * then invoke:
> > + * When a higher-priority class takes a CPU away from a running SCX task -
> > + * a sched_switch from an SCX task to a higher-priority task - we mark the
> > + * pair_ctx as having been preempted and then invoke:
> >   *
> >   * scx_bpf_kick_cpu(pair_cpu, SCX_KICK_PREEMPT | SCX_KICK_WAIT);
> >   *
> > @@ -107,9 +108,19 @@
> >   * sched_class that preempted our scheduler does not schedule a task
> >   * concurrently with our pair CPU.
> >   *
> > - * When the CPU is re-acquired in pair_cpu_acquire(), we unmark the preemption
> > - * in the pair_ctx, and send another resched IPI to the pair CPU to re-enable
> > - * pair scheduling.
> > + * When the CPU returns to SCX or idle, we unmark the preemption in the
> > + * pair_ctx and send another resched IPI to the pair CPU to re-enable pair
> > + * scheduling.
> > + *
> > + * A switch from idle straight to a higher-priority task is not a release:
> > + * the CPU was not running an SCX task, so there is nothing to drain and no
> > + * reason to make the pair wait. Kicking SCX_KICK_WAIT on every such wakeup
> > + * would stall the pair CPU behind rt bursts it was never coupled to.
> > + *
> > + * Note that sched_setscheduler() on a running task changes its class in
> > + * place without a context switch, so such transitions are only observed at
> > + * the task's next switch. Until then the stale active_mask bit makes the
> > + * pair wait in try_dispatch(), which is bounded by that next switch.
> >   *
> >   * Copyright (c) 2022 Meta Platforms, Inc. and affiliates.
> >   * Copyright (c) 2022 Tejun Heo <[email protected]>
> > @@ -118,6 +129,8 @@
> >  #include <scx/common.bpf.h>
> >  #include "scx_pair.h"
> >  
> > +#define MAX_RT_PRIO	100
> > +
> >  char _license[] SEC("license") = "GPL";
> >  
> >  /* !0 for veristat, set during init */
> > @@ -308,6 +321,40 @@ static int lookup_pairc_and_mask(s32 cpu, struct pair_ctx **pairc, u32 *mask)
> >  	return 0;
> >  }
> >  
> > +/*
> > + * A task is above SCX whenever its effective priority is in the rt/dl
> > + * range. Test p->prio rather than p->policy: rt_mutex_setprio() boosts
> > + * a PI beneficiary into the rt/dl classes with its policy left
> > + * untouched, so a policy test would misclassify boosted tasks in both
> > + * directions. p->prio follows the boost and the deboost.
> > + *
> > + * This still cannot tell fair and SCX tasks apart. It is complete only
> > + * because scx_pair runs in switch-all mode, where no fair class task
> > + * exists; in partial mode fair is also above SCX and can take the CPU.
> > + */
> > +static bool pair_task_is_highpri(struct task_struct *p)
> > +{
> > +	return p->prio < MAX_RT_PRIO;
> > +}
> > +
> > +static void pair_cpu_acquire_locked(struct pair_ctx *pairc, u32 in_pair_mask,
> > +					u32 *kick_flags)
> > +{
> > +	pairc->preempted_mask &= ~in_pair_mask;
> > +	/* Kick the pair CPU, unless it was also preempted. */
> > +	*kick_flags = !pairc->preempted_mask ? SCX_KICK_PREEMPT : 0;
> > +}
> > +
> > +static void pair_cpu_release_locked(struct pair_ctx *pairc, u32 in_pair_mask,
> > +					u32 *kick_flags)
> > +{
> > +	pairc->preempted_mask |= in_pair_mask;
> > +	pairc->active_mask &= ~in_pair_mask;
> > +	/* Kick the pair CPU if it's still running. */
> > +	*kick_flags = pairc->active_mask ? SCX_KICK_PREEMPT | SCX_KICK_WAIT : 0;
> > +	pairc->draining = true;
> > +}
> > +
> >  __attribute__((noinline))
> >  static int try_dispatch(s32 cpu)
> >  {
> > @@ -500,61 +547,60 @@ void BPF_STRUCT_OPS(pair_dispatch, s32 cpu, struct task_struct *prev)
> >  	}
> >  }
> >  
> > -void BPF_STRUCT_OPS(pair_cpu_acquire, s32 cpu, struct scx_cpu_acquire_args *args)
> > +SEC("tp_btf/sched_switch")
> > +int BPF_PROG(pair_sched_switch, bool preempt, struct task_struct *prev,
> > +	     struct task_struct *next, unsigned int prev_state)
> >  {
> >  	int ret;
> > +	s32 cpu = bpf_get_smp_processor_id();
> >  	u32 in_pair_mask;
> >  	struct pair_ctx *pairc;
> > -	bool kick_pair;
> > +	u32 kick_flags = 0;
> > +	bool preempted;
> > +	bool release, acquire;
> >  
> >  	ret = lookup_pairc_and_mask(cpu, &pairc, &in_pair_mask);
> >  	if (ret)
> > -		return;
> > -
> > -	bpf_spin_lock(&pairc->lock);
> > -	pairc->preempted_mask &= ~in_pair_mask;
> > -	/* Kick the pair CPU, unless it was also preempted. */
> > -	kick_pair = !pairc->preempted_mask;
> > -	bpf_spin_unlock(&pairc->lock);
> > -
> > -	if (kick_pair) {
> > -		s32 *pair = (s32 *)ARRAY_ELEM_PTR(pair_cpu, cpu, nr_cpu_ids);
> > +		return 0;
> >  
> > -		if (pair) {
> > -			__sync_fetch_and_add(&nr_kicks, 1);
> > -			scx_bpf_kick_cpu(*pair, SCX_KICK_PREEMPT);
> > -		}
> > +	/*
> > +	 * This runs on every context switch in the system. A CPU's own
> > +	 * preempted_mask bit is only ever written by this tracepoint
> > +	 * running on that CPU, so the unlocked read is exact and the
> > +	 * pair-shared lock is only taken on actual transitions.
> > +	 */
> > +	preempted = pairc->preempted_mask & in_pair_mask;
> > +	if (next->pid && pair_task_is_highpri(next)) {
> > +		/* an SCX task lost the CPU to a higher-priority class */
> > +		release = !preempted && prev->pid && !pair_task_is_highpri(prev);
> > +		acquire = false;
> > +	} else {
> > +		/* the CPU is back under SCX control (or idle) */
> > +		release = false;
> > +		acquire = preempted;
> >  	}
> > -}
> > -
> > -void BPF_STRUCT_OPS(pair_cpu_release, s32 cpu, struct scx_cpu_release_args *args)
> > -{
> > -	int ret;
> > -	u32 in_pair_mask;
> > -	struct pair_ctx *pairc;
> > -	bool kick_pair;
> > -
> > -	ret = lookup_pairc_and_mask(cpu, &pairc, &in_pair_mask);
> > -	if (ret)
> > -		return;
> > +	if (!release && !acquire)
> > +		return 0;
> >  
> >  	bpf_spin_lock(&pairc->lock);
> > -	pairc->preempted_mask |= in_pair_mask;
> > -	pairc->active_mask &= ~in_pair_mask;
> > -	/* Kick the pair CPU if it's still running. */
> > -	kick_pair = pairc->active_mask;
> > -	pairc->draining = true;
> > +	if (release) {
> > +		pair_cpu_release_locked(pairc, in_pair_mask, &kick_flags);
> > +		__sync_fetch_and_add(&nr_preemptions, 1);
> > +	} else {
> > +		pair_cpu_acquire_locked(pairc, in_pair_mask, &kick_flags);
> > +	}
> >  	bpf_spin_unlock(&pairc->lock);
> >  
> > -	if (kick_pair) {
> > +	if (kick_flags) {
> >  		s32 *pair = (s32 *)ARRAY_ELEM_PTR(pair_cpu, cpu, nr_cpu_ids);
> >  
> >  		if (pair) {
> >  			__sync_fetch_and_add(&nr_kicks, 1);
> > -			scx_bpf_kick_cpu(*pair, SCX_KICK_PREEMPT | SCX_KICK_WAIT);
> > +			scx_bpf_kick_cpu(*pair, kick_flags);
> >  		}
> >  	}
> > -	__sync_fetch_and_add(&nr_preemptions, 1);
> > +
> > +	return 0;
> >  }
> >  
> >  s32 BPF_STRUCT_OPS(pair_cgroup_init, struct cgroup *cgrp)
> > @@ -602,8 +648,6 @@ void BPF_STRUCT_OPS(pair_exit, struct scx_exit_info *ei)
> >  SCX_OPS_DEFINE(pair_ops,
> >  	       .enqueue			= (void *)pair_enqueue,
> >  	       .dispatch		= (void *)pair_dispatch,
> > -	       .cpu_acquire		= (void *)pair_cpu_acquire,
> > -	       .cpu_release		= (void *)pair_cpu_release,
> >  	       .cgroup_init		= (void *)pair_cgroup_init,
> >  	       .cgroup_exit		= (void *)pair_cgroup_exit,
> >  	       .exit			= (void *)pair_exit,
> > -- 
> > 2.43.0
> > 

-- 
Cheers,
Cheng-Yang