Re: [PATCH 02/10] sched/fair: Add rate-limiting and validation helpers

[K Prateek Nayak <kprateek.nayak@amd.com>]

Hello Wanpeng,

On 11/10/2025 9:02 AM, Wanpeng Li wrote:
> +/*
> + * High-frequency yield gating to reduce overhead on compute-intensive workloads.
> + * Returns true if the yield should be skipped due to frequency limits.
> + *
> + * Optimized: single threshold with READ_ONCE/WRITE_ONCE, refresh timestamp on every call.
> + */
> +static bool yield_deboost_rate_limit(struct rq *rq, u64 now_ns)
> +{
> +	u64 last = READ_ONCE(rq->yield_deboost_last_time_ns);
> +	bool limited = false;
> +
> +	if (last) {
> +		u64 delta = now_ns - last;
> +		limited = (delta <= 6000ULL * NSEC_PER_USEC);
> +	}
> +
> +	WRITE_ONCE(rq->yield_deboost_last_time_ns, now_ns);

We only look at local rq so READ_ONCE()/WRITE_ONCE() seems
unnecessary.

> +	return limited;
> +}
> +
> +/*
> + * Validate tasks and basic parameters for yield deboost operation.
> + * Performs comprehensive safety checks including feature enablement,
> + * NULL pointer validation, task state verification, and same-rq requirement.
> + * Returns false with appropriate debug logging if any validation fails,
> + * ensuring only safe and meaningful yield operations proceed.
> + */
> +static bool __maybe_unused yield_deboost_validate_tasks(struct rq *rq, struct task_struct *p_target,
> +					  struct task_struct **p_yielding_out,
> +					  struct sched_entity **se_y_out,
> +					  struct sched_entity **se_t_out)
> +{
> +	struct task_struct *p_yielding;
> +	struct sched_entity *se_y, *se_t;
> +	u64 now_ns;
> +
> +	if (!sysctl_sched_vcpu_debooster_enabled)
> +		return false;
> +
> +	if (!rq || !p_target)
> +		return false;
> +
> +	now_ns = rq->clock;

Brief look at Patch 5 suggests we are under the rq_lock so might
as well use the rq_clock(rq) helper. Also, you have to do a
update_rq_clock() since it isn't done until yield_task_fair().

> +
> +	if (yield_deboost_rate_limit(rq, now_ns))
> +		return false;
> +
> +	p_yielding = rq->curr;
> +	if (!p_yielding || p_yielding == p_target ||
> +	    p_target->sched_class != &fair_sched_class ||
> +	    p_yielding->sched_class != &fair_sched_class)
> +		return false;

yield_to() in syscall.c has already checked for the sched
class matching under double_rq_lock. That cannot change by the
time we are here.

> +
> +	se_y = &p_yielding->se;
> +	se_t = &p_target->se;
> +
> +	if (!se_t || !se_y || !se_t->on_rq || !se_y->on_rq)
> +		return false;
> +
> +	if (task_rq(p_yielding) != rq || task_rq(p_target) != rq)

yield_to() has already checked for this under double_rq_lock()
so this too should be unnecessary.

> +		return false;
> +
> +	*p_yielding_out = p_yielding;
> +	*se_y_out = se_y;
> +	*se_t_out = se_t;

Why do we need these pointers? Can't the caller simply do:

    if (!yield_deboost_validate_tasks(rq, target))
        return;

    p_yielding = rq->donor;
    se_y_out = &p_yielding->se;
    se_t = &target->se;

That reminds me - now that we have proxy execution, you need
to re-evaluate the usage of rq->curr (running context) vs
rq->donor (vruntime context) when looking at all this.

> +	return true;
> +}
> +
>  /*
>   * sched_yield() is very simple
>   */

-- 
Thanks and Regards,
Prateek