Re: [PATCH 1/2] xenalyze: Handle start-of-day ->RUNNING transitions

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Mon, Mar 27, 2023 at 05:13:25PM +0100, George Dunlap wrote:
> A recent xentrace highlighted an unhandled corner case in the vcpu
> "start-of-day" logic, if the trace starts after the last running ->
> non-running transition, but before the first non-running -> running
> transition.  Because start-of-day wasn't handled, vcpu_next_update()
> was expecting p->current to be NULL, and tripping out with the
> following error message when it wasn't:
> 
> vcpu_next_update: FATAL: p->current not NULL! (d32768dv$p, runstate 
> RUNSTATE_INIT)
> 
> where 32768 is the DEFAULT_DOMAIN, and $p is the pcpu number.
> 
> Instead of calling vcpu_start() piecemeal throughout
> sched_runstate_process(), call it at the top of the function if the
> vcpu in question is still in RUNSTATE_INIT, so that we can handle all
> the cases in one place.
> 
> Sketch out at the top of the function all cases which we need to
> handle, and what to do in those cases.  Some transitions tell us where
> v is running; some transitions tell us about what is (or is not)
> running on p; some transitions tell us neither.
> 
> If a transition tells us where v is now running, update its state;
> otherwise leave it in INIT, in order to avoid having to deal with TSC
> skew on start-up.
> 
> If a transition tells us what is or is not running on p, update
> p->current (either to v or NULL).  Otherwise leave it alone.
> 
> If neither, do nothing.
> 
> Reifying those rules:
> 
> - If we're continuing to run, set v to RUNNING, and use p->first_tsc
>   as the runstate time.
> 
> - If we're starting to run, set v to RUNNING, and use ri->tsc as the
>   runstate time.
> 
> - If v is being deschedled, leave v in the INIT state to avoid dealing
>   with TSC skew; but set p->current to NULL so that whatever is
>   scheduled next won't trigger the assert in vcpu_next_update().
> 
> - If a vcpu is waking up (switching from one non-runnable state to
>   another non-runnable state), leave v in INIT, and p in whatever
>   state it's in (which may be the default domain, or some other vcpu
>   which has already run).
> 
> While here, fix the comment above vcpu_start; it's called when the
> vcpu state is INIT, not when current is the default domain.
> 
> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxx>

Acked-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

-- 
Anthony PERARD