Re: [PATCH 06/11] sched_ext: Split curr|donor references properly

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing TICK_DEP_BIT_SCHED protection for cross-class proxy execution causes remote NOHZ tick warnings and accounting corruption.
- [Medium] The ops.running() callback is never invoked for the physical lock owner under proxy execution, violating the documented API contract.
--

--- Patch [6]: [PATCH 06/11] sched_ext: Split curr|donor references properly ---
commit 4630a7edb1e1687e72cb21f49d2599ffe928b4de
Author: Andrea Righi <[email protected]>

sched_ext: Split curr|donor references properly

This commit prepares sched_ext for proxy execution by differentiating
between the scheduling context provider (rq->donor) and the physically
executing task (rq->curr). It adjusts tick management, task selection,
and BPF helper functions to use the donor where appropriate.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c

[ ... ]

> @@ -4370,14 +4381,14 @@ static void run_deferred(struct rq *rq)
>  #ifdef CONFIG_NO_HZ_FULL
>  bool scx_can_stop_tick(struct rq *rq)
>  {
> -	struct task_struct *p = rq->curr;
> +	struct task_struct *p = rq->donor;
>  	struct scx_sched *sch = scx_task_sched(p);
>  
>  	if (p->sched_class != &ext_sched_class)
>  		return true;

[Severity: High]
Does this missing check cause remote NOHZ tick warnings during cross-class
proxy execution?

If a FAIR task donates to an EXT task, rq->donor is FAIR but rq->curr is
EXT. When scx_can_stop_tick() is called, it checks p->sched_class (which
is FAIR) and immediately returns true. If the FAIR donor isn't bandwidth
constrained, this allows the tick to stop.

However, stopping the tick when rq->curr != rq->donor violates core NOHZ
assumptions and triggers a WARN_ON_ONCE in sched_tick_remote().

Will this also corrupt time bandwidth accounting by charging the EXT owner
instead of the FAIR donor?

[ ... ]

> @@ -9932,10 +9955,14 @@ __bpf_kfunc struct rq *scx_bpf_locked_rq(const struct bpf_prog_aux *aux)
>  }
>  
>  /**
> - * scx_bpf_cpu_curr - Return remote CPU's curr task
> + * scx_bpf_cpu_curr - Return remote CPU's current scheduling context
>   * @cpu: CPU of interest
>   * @aux: implicit BPF argument to access bpf_prog_aux hidden from BPF progs
>   *
> + * Under proxy execution, this returns the donor, which supplies the scheduling
> + * policy and runtime budget, rather than the task whose code is physically
> + * executing. ops.running() continues to report physical task execution.
> + *

[Severity: Medium]
Is ops.running() actually invoked for the physical lock owner as documented
here?

When a task donates its context, the core scheduler calls
set_next_task_scx() for the blocked donor. However, set_next_task_scx()
explicitly skips ops.running() for blocked tasks:

set_next_task_scx() {
    ...
    if ((p->scx.flags & SCX_TASK_QUEUED) && !p->is_blocked) {
        if (SCX_HAS_OP(sch, running))
            SCX_CALL_OP_TASK(sch, running, rq, p);
    }
    ...
}

Since the core scheduler context-switches to the physical lock owner
without invoking its scheduling class callbacks, does this leave the BPF
scheduler completely blind to the physical task execution?

>   * Callers must hold RCU read lock (KF_RCU).
>   */
>  __bpf_kfunc struct task_struct *scx_bpf_cpu_curr(s32 cpu, const struct bpf_prog_aux *aux)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6