Re: [PATCH v3 04/10] sched/fair: Credit nominated next-buddy in yield_to_task_fair()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Skipping `update_rq_clock()` for the same-runqueue case in `yield_to_task_fair()` unconditionally triggers a `WARN_ON_ONCE` in `assert_clock_updated()` via `update_curr()`.
--

commit 18fe3f24f01f55a4b16680e6b460846c6d5e6260
Author: Wanpeng Li <wanpengli@tencent.com>

sched/fair: Credit nominated next-buddy in yield_to_task_fair()

This patch credits bounded EEVDF lag to nominated targets during
yield_to(). It walks the sched_entity hierarchy to provide the credit,
avoiding forfeiture of the yield hint when a group entity isn't yet
eligible.

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index c28682fedb362..48f65a4f19230 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c

[ ... ]

> @@ -9496,6 +9497,43 @@ static bool yield_to_task_fair(struct rq *rq, struct task_struct *p)
>  	/* Tell the scheduler that we'd really like se to run next. */
>  	set_next_buddy(se);
>  
> +	/* Without lag credit, keep the existing forfeit-based yield. */
> +	if (!sched_feat(YIELD_TO_LAG_CREDIT)) {
> +		yield_task_fair(rq);
> +		return true;
> +	}
> +
> +	/*
> +	 * Walk the ancestor chain set_next_buddy() just nominated and credit
> +	 * bounded lag to each not-yet-eligible level so pick_eevdf() returns
> +	 * it. yield_to() holds both rq locks via double_rq_lock(), so touching
> +	 * p's cfs_rqs (possibly on another CPU) is safe; the primitive is
> +	 * idempotent, so no rate limiting is needed.
> +	 *
> +	 * Only refresh p_rq's clock when it differs from the local rq. A
> +	 * remote p_rq must be refreshed so the per-level update_curr() is
> +	 * accurate. In the same-rq case we skip it: the credit is a
> +	 * best-effort hint and the rq clock is recent enough, while the
> +	 * trailing yield_task_fair() would otherwise make this a second
> +	 * update_rq_clock() on the same rq and trip
> +	 * SCHED_WARN_ON(WARN_DOUBLE_CLOCK).
> +	 */
> +	if (rq != p_rq)
> +		update_rq_clock(p_rq);
> +
> +	for_each_sched_entity(se) {
> +		struct cfs_rq *cfs_rq = cfs_rq_of(se);
> +
> +		if (cfs_rq->next != se)
> +			break;
> +		if (se->sched_delayed)
> +			break;
> +		if (throttled_hierarchy(cfs_rq))
> +			break;
> +
> +		eevdf_credit_entity_vlag(cfs_rq, se);

[Severity: High]
Does skipping update_rq_clock() here trigger a WARN_ON_ONCE() when the target
task is on the same runqueue?

Since yield_to() acquires locks via double_rq_lock(), the clock update flag
is cleared. By skipping update_rq_clock(p_rq) for local tasks, the call to
eevdf_credit_entity_vlag() will invoke update_curr() with an un-updated clock:

eevdf_credit_entity_vlag()
  update_curr()
    update_se()
      rq_clock_task()
        assert_clock_updated() -> WARN_ON_ONCE(rq->clock_update_flags < RQCF_ACT_SKIP)

Can this reliably trigger a warning when yield_to() targets a task on the
same CPU (for instance, via KVM vCPU yielding)?

> +	}
>  
>  	yield_task_fair(rq);
>  
>  	return true;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612013355.59231-1-kernellwp@gmail.com?part=4