Re: [PATCH-for-4.17] xen/sched: fix race in RTDS scheduler

[Dario Faggioli <dfaggioli@xxxxxxxx>]

On Fri, 2022-10-21 at 08:10 +0200, Juergen Gross wrote:
> When a domain gets paused the unit runnable state can change to "not
> runnable" without the scheduling lock being involved. This means that
> a specific scheduler isn't involved in this change of runnable state.
> 
> In the RTDS scheduler this can result in an inconsistency in case a
> unit is losing its "runnable" capability while the RTDS scheduler's
> scheduling function is active. RTDS will remove the unit from the run
> queue, but doesn't do so for the replenish queue, leading to hitting
> an ASSERT() in replq_insert() later when the domain is unpaused
> again.
> 
> Fix that by removing the unit from the replenish queue as well in
> this
> case.
> 
Ah, ok... So, all is fine until what could happen during rt_schedule(),
was "just" that the currently running task, not only is descheduled,
but it also became !runnable.

In fact, in this case, the unit itself is not in the runq, but it can
be in the replq. However, since it still has the RTDS_scheduled flag
set, either:
1) we reach rt_context_saved(), which remove it from replq, before any 
   replq_insert;
2) rt_unit_wake() is called, but due to RTDS_scheduled, it may only do 
   replq_reinsert(), which is fine with the unit being already there.

However, what can also happen in rt_schedule() is that we remove from
the runq an unit that was not running, and hence does not have the
RTDS_scheduled flat set. In which case, rt_context_saved() doesn't do
anything to it (of course!). And as soon as rt_unit_wake() happens, it
does replq_insert(), which is not fine with finding the replenishment
event in the queue already.

So, yes... And good catch! :-P


> Fixes: 7c7b407e7772 ("xen/sched: introduce unit_runnable_state()")
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>
Acked-by: Dario Faggioli <dfaggioli@xxxxxxxx>

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

Attachment: signature.asc
Description: This is a digitally signed message part