Re: [PATCH v4 10/26] sched/deadline: Release the bandwidth of a task that leaves the class asleep

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] sched/deadline: dl_zero_lag() mixes PI donor state with kept
  task params
--

commit 82ae18ee2c32ac7a69958d0da90b2ae54505ac04
Author: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>

sched/deadline: Release the bandwidth of a task that leaves the class asleep

This commit releases or arms the inactive timer for a task's SCHED_DEADLINE
bandwidth in switched_from_dl() when the task leaves the class while not
queued, and clears its deadline parameters once the bandwidth is released.

Link: https://lore.kernel.org/all/20261103-b4-sched-dl-bw-fixes-v1-2-dd761fe005b4@xxxxxxxxxx/

> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index f389620e6fc9a..cb0cf9c15108f 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -390,6 +390,22 @@ static void dl_change_utilization(struct task_struct *p, u64 new_bw)
>  
>  static void __dl_clear_params(struct sched_dl_entity *dl_se);
>  
> +/*
> + * The 0-lag time of @dl_se, relative to the clock of its runqueue: its
> + * utilization counts until then once it blocks or leaves the class.
> + */
> +static s64 dl_zero_lag(struct sched_dl_entity *dl_se)
> +{
> +	struct rq *rq = rq_of_dl_se(dl_se);
> +
> +	/*
> +	 * Using relative times instead of the absolute "0-lag time"
> +	 * allows to simplify the code
> +	 */
> +	return dl_se->deadline - rq_clock(rq) -
> +	       div64_long((dl_se->runtime * dl_se->dl_period), dl_se->dl_runtime);
> +}
[ ... ]
> @@ -3597,15 +3605,69 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p)
>  	dec_dl_tasks_cs(p);
>  
>  	if (!task_on_rq_queued(p)) {
> -		/*
> -		 * Inactive timer is armed. However, p is leaving DEADLINE and
> -		 * might migrate away from this rq while continuing to run on
> -		 * some other class. We need to remove its contribution from
> -		 * this rq running_bw now, or sub_rq_bw (below) will complain.
> -		 */
> -		if (p->dl.dl_non_contending)
> +		bool released = false;
> +
> +		if (p->dl.dl_non_contending) {
> +			/*
> +			 * Inactive timer is armed. However, p is leaving
> +			 * DEADLINE and might migrate away from this rq while
> +			 * continuing to run on some other class. We need to
> +			 * remove its contribution from this rq running_bw now,
> +			 * or sub_rq_bw (below) will complain.
> +			 *
> +			 * inactive_task_timer() will find !dl_task(p) and
> +			 * release total_bw via __dl_sub().
> +			 */
>  			sub_running_bw(&p->dl, &rq->dl);
> +		} else if (p->dl.dl_runtime && !dl_entity_is_special(&p->dl) &&
> +			   dl_zero_lag(&p->dl) >= 0 &&
> +			   !hrtimer_active(&p->dl.inactive_timer)) {
> +			/*
> +			 * No inactive timer armed, the 0-lag time ahead: the
> +			 * task is in flight to the CPU it wakes on, which
> +			 * cancelled the timer (migrate_task_rq_dl()), or it
> +			 * came back to the class asleep, which cancelled it,
> +			 * and leaves again. Arm the timer, as for a queued
> +			 * task: its handler releases total_bw at the 0-lag
> +			 * time, finding the task out of the class. running_bw
> +			 * is off this runqueue already, or never was on it.
> +			 */
> +			task_non_contending(&p->dl, false);
> +		} else if (p->dl.dl_runtime && !dl_entity_is_special(&p->dl)) {

[Severity: Medium]
Can dl_zero_lag(&p->dl) mix a PI donor's dynamic period state with p's own
unreleased reservation parameters?

When a SCHED_DEADLINE task leaves the class before its 0-lag time (or
changes policy away from SCHED_DEADLINE while already PI-boosted), it keeps
its static parameters (dl_runtime, dl_deadline, dl_period, dl_bw) until
0-lag time.  If a SCHED_DEADLINE waiter boosts the task into dl_sched_class
before inactive_timer fires, switched_to_dl() cancels inactive_timer.

During the boost, replenish_dl_entity() skips initializing the donor's
period at first because p->dl.dl_deadline is still non-zero:

kernel/sched/deadline.c:replenish_dl_entity() {
    ...
	if (dl_se->dl_deadline == 0 ||
	    (dl_se->dl_defer_armed && dl_entity_overflow(dl_se, rq_clock(rq)))) {
		dl_se->deadline = rq_clock(rq) + pi_deadline_of(dl_se);
		dl_se->runtime = pi_runtime_of(dl_se);
	}
    ...
}

Once p->dl.deadline and p->dl.runtime are replenished from the donor's
pi_params (on wakeup via replenish_dl_new_period() or after exhausting the
old runtime), p->dl.deadline and p->dl.runtime hold the donor's state while
p->dl.dl_period and p->dl.dl_runtime still hold p's old reservation.  When
p later blocks or is deboosted in switched_from_dl(), dl_zero_lag(&p->dl)
combines the donor's deadline and runtime with p's dl_period and
dl_runtime.  Depending on the donor's parameters, this either releases p's
dl_bw and clears its parameters before p's true 0-lag time, or sees a
positive 0-lag time after p's true 0-lag time has already passed, delaying
the bandwidth release and leaving the donor's deadline and runtime on
p->dl.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107014451.1487825-1-mathieu.desnoyers@xxxxxxxxxxxx?part=10