Re: [PATCH v2 06/21] sched/deadline: Do not lose a throttled task switched out and back

[Juri Lelli]

Hello,

On 04/11/26 16:11, Mathieu Desnoyers wrote:
> A SCHED_DEADLINE task that is throttled, switched to another policy and
> switched back before its 0-lag time can be left throttled with nothing
> to replenish it: runnable, queued as far as the core is concerned, but
> on no dl_rq and with no timer pending. It never runs again, and a fatal
> signal cannot make it run; only another switch out of SCHED_DEADLINE
> requeues it.
> 
>  1. the task exhausts its runtime: dl_throttled is set, and the
>     replenishment timer armed for the start of its next period;
>  2. sched_setscheduler() switches it to SCHED_OTHER: switched_from_dl()
>     calls task_non_contending(), which arms the inactive timer for the
>     0-lag time; the parameters are kept until then, so that leaving and
>     reentering SCHED_DEADLINE gains no bandwidth;
>  3. the replenishment timer fires: dl_task_timer() finds a task that is
>     not a deadline task and returns, dl_throttled still set;
>  4. sched_setattr() switches it back before the 0-lag time:
>     switched_to_dl() cancels the inactive timer, and enqueue_dl_entity()
>     leaves a throttled task to its replenishment timer, which has fired
>     already.
> 
> The window between the deadline and the 0-lag time is the overrun
> scaled by period/runtime: about 100us for an overrun of 20us at 20%
> bandwidth. The replenishment timer cleared the parameters, dl_throttled
> included, of a task it found out of SCHED_DEADLINE until
> commit 209a0cbda7a0 ("sched/deadline: Improve the tracking of active
> utilization") moved that reset to the inactive timer, at the 0-lag
> time.
> 
> Give the task the replenishment the timer owes it, as the timer already
> does for one that is throttled and not queued: the overrun comes off the
> periods that follow, the deadline moves past it, and dl_throttled and
> dl_yielded are cleared. A return before the 0-lag time then finds an
> entity it can run on, and a return after it finds the parameters reset
> by the inactive timer, as before.
> 
> Clearing dl_throttled alone would leave the overrun in the runtime, for
> the new period of the enqueue to forgive, the deadline behind, for
> dl_check_constrained_dl() to throttle the task on again, and a yield of
> that period set, for update_curr_dl_se() to throttle it at once. The
> first version of this patch did, and the Sashiko AI review of the first
> posting of this series pointed the three out.
> 
> The selftest added two patches later fails its throttled_round_trip
> case 8 times out of 8, on v7.3 as on v7.4-rc1 with the four deadline
> fixes of sched/urgent, and passes it 8 times out of 8 with this change.
> It was found by a cgroup selftest under development,
> test_concurrency_deadline, which hung about once in 1350 runs.
> 
> Fixes: 209a0cbda7a0 ("sched/deadline: Improve the tracking of active utilization")
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
> ---
>  kernel/sched/deadline.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 30a52e42c26d..b646063a7b0d 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -1232,10 +1232,23 @@ static enum hrtimer_restart dl_task_timer(struct hrtimer *timer)
>  
>  	/*
>  	 * The task might have changed its scheduling policy to something
> -	 * different than SCHED_DEADLINE (through switched_from_dl()).
> +	 * different than SCHED_DEADLINE (through switched_from_dl()). It
> +	 * keeps its parameters until its 0-lag time and may come back to
> +	 * SCHED_DEADLINE before then, switched_to_dl() cancelling the
> +	 * inactive timer: give it the replenishment this timer owes it, as
> +	 * the !queued case below does, and leave the enqueue to its return.
> +	 * A throttle left behind would never be lifted; an overrun left in
> +	 * its runtime would be forgiven by the new period its enqueue
> +	 * starts; and a yield left set would throttle it again at once.
>  	 */
> -	if (!dl_task(p))
> +	if (!dl_task(p)) {
> +		if (dl_se->dl_throttled && !is_dl_boosted(dl_se)) {
> +			sched_clock_tick();
> +			update_rq_clock(rq);
> +			replenish_dl_entity(dl_se);
> +		}
>  		goto unlock;
> +	}

This should fix the subject issue. Sashiko points out a follow-up
re-entry case that looks problematic (and I think I'm now seeing more),
but that should all only concern limited bandwidth overruns. This change
fix an actual hang, so we might want to take it and work on the
remaining issues separately.

Thanks,
Juri