Re: [PATCH] sched/rtds: refill cur_budget when extratime is toggled on a depleted vCPU

[Juergen Gross <jgross@xxxxxxxx>]

On 20.05.26 15:57, Oleksii Moisieiev wrote:
> XEN_DOMCTL_SCHEDOP_putvcpuinfo can flip the RTDS_extratime bit on a vCPU
> that is currently depleted (cur_budget == 0, possibly sitting on the
> depleted queue). rt_dom_cntl() touches only svc->flags; cur_budget is
> left unchanged. As a result the next code path that calls runq_insert()
> on this vCPU - rt_unit_wake() after a domain_unpause(),
> rt_context_saved() following a delayed runq add, or repl_timer_handler()
> after a replenishment - places the vCPU on the run queue, because
> has_extratime(svc) is now true and runq_insert() admits extratime units
> regardless of cur_budget:
>
>      /* add svc to runq if svc still has budget or its extratime is set */
>      if ( svc->cur_budget > 0 ||
>           has_extratime(svc) )
>          deadline_runq_insert(svc, &svc->q_elem, runq);
>      else
>          list_add(&svc->q_elem, &prv->depletedq);
>
> The very next rt_schedule() iterates the run queue from runq_pick()
> and trips the ASSERT(iter_svc->cur_budget > 0) at the bottom of the
> loop, panicking the host. Observed trace:
>
>      Assertion 'iter_svc->cur_budget > 0' failed at common/sched/rt.c:1035
>      ----[ Xen-4.22-unstable  arm64  debug=y ubsan=y  Not tainted ]----
>      [<...>] rt.c#rt_schedule+0x1558/0x33e0 (PC)
>      [<...>] core.c#do_schedule+0x2e4/0x15b4
>      [<...>] core.c#schedule+0xb14/0xe50
>      [<...>] softirq.c#__do_softirq+0x20c/0x3d4
>      [<...>] do_softirq+0x14/0x1c
>      [<...>] domain.c#idle_loop+0x194/0x558
>
> Minimal reproducer: pin a single-vCPU domU to a pCPU, program RTDS with
> extratime off and a low utilisation (e.g. budget = 10ms / period = 100ms)
> so the vCPU spends most of its time in the depleted queue, pause the
> domain, issue a putvcpuinfo that sets XEN_DOMCTL_SCHEDRT_extra, then
> unpause. As soon as the schedule softirq fires on the pCPU, the BUG
> hits. The same sequence is reachable without an explicit pause: any
> window in which rt_dom_cntl() runs between burn_budget()'s budget
> exhaustion and rt_context_saved()'s runq_insert() also closes onto the
> same broken state, because the per-scheduler lock is dropped between
> those two points.
>
> The semantics for "extratime gets exhausted budget refilled" already
> live in burn_budget():
>
>      if ( has_extratime(svc) )
>      {
>          svc->priority_level++;
>          svc->cur_budget = svc->budget;
>      }
>
> Apply the same priority-demotion-and-refill in rt_dom_cntl() when the
> flag transitions from off to on while the vCPU is depleted, clear
> RTDS_depleted to match, and - if the vCPU is currently on the depleted
> queue - move it to the run queue using the same q_remove() +
> runq_insert() pattern already used by repl_timer_handler(). The vCPU
> remains on the replenishment queue throughout, so its normal
> replenishment cadence is preserved.
>
> The complementary transition (on -> off) is already safe: clearing the
> flag only narrows the runq_insert() admission condition, so subsequent
> depleted insertions correctly route to the depleted queue.
>
> No other call sites need changes: with cur_budget restored before the
> flag is observable to runq_insert(), runq_pick()'s long-standing
> invariant (every run-queue entry has cur_budget > 0) is preserved.
This is missing

Fixes: 463b95831778 ("xen:rtds: towards work conserving RTDS")

> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@xxxxxxxx>
With the "Fixes:" tag added:

Reviewed-by: Juergen Gross <jgross@xxxxxxxx>


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature