Re: [Xen-devel] [PATCH v2] xen/sched: rework and rename vcpu_force_reschedule()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.09.2019 08:42, Juergen Gross wrote:
> vcpu_force_reschedule() is only used for modifying the periodic timer
> of a vcpu. Forcing a vcpu to give up the physical cpu for that purpose
> is kind of brutal.
> 
> So instead of doing the reschedule dance just operate on the timer
> directly. By protecting periodic timer modifications against concurrent
> timer activation via a per-vcpu lock it is even no longer required to
> bother the target vcpu at all for updating its timer.
> 
> Rename the function to vcpu_set_periodic_timer() as this now reflects
> the functionality.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

I continue to be unhappy about there being no word at all about ...

> @@ -724,24 +725,6 @@ static void vcpu_migrate_finish(struct vcpu *v)
>      vcpu_wake(v);
>  }
>  
> -/*
> - * Force a VCPU through a deschedule/reschedule path.
> - * For example, using this when setting the periodic timer period means that
> - * most periodic-timer state need only be touched from within the scheduler
> - * which can thus be done without need for synchronisation.
> - */
> -void vcpu_force_reschedule(struct vcpu *v)

... the originally intended synchronization-free handling. Forcing
the vCPU through the scheduler may seem harsh (and quite some
overhead), yes, but I don't think the above was written (and
decided) without consideration. One effect of this can be seen by
you ...

> +void vcpu_set_periodic_timer(struct vcpu *v, s_time_t value)
> +{
> +    spin_lock(&v->periodic_timer_lock);
> +
> +    stop_timer(&v->periodic_timer);

... introducing a new stop_timer() here, i.e. which doesn't replace
any existing one. The implication is that other than before the
periodic timer may now not run (for a brief moment) despite it
being supposed to run - after all it has been active so far
whenever a vCPU was running.

Then again, looking at the involved code paths yet again, I wonder
whether this has been working right at all: There's an early exit
from schedule() when prev == next, which bypasses
vcpu_periodic_timer_work(). And I can't seem to be able to spot
anything on the vcpu_force_reschedule() path which would guarantee
this shortcut to not be taken.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel