Re: [Xen-devel] [PATCH v8 10/11] viridian: add implementation of synthetic timers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.03.19 at 17:26, <Paul.Durrant@xxxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 18 March 2019 16:23
>> 
>> >>> On 18.03.19 at 16:46, <Paul.Durrant@xxxxxxxxxx> wrote:
>> >> > > +    {
>> >> > > +        expiration = vs->count;
>> >> > > +        if ( expiration - now <= 0 )
>> >> > > +        {
>> >> > > +            vs->expiration = expiration;
>> >> > > +            stimer_expire(vs);
>> >> >
>> >> > Aren't you introducing a risk for races by calling the timer function
>> >> > directly from here? start_timer(), after all, gets called from quite a
>> >> > few places.
>> >>
>> >> In practice I don't think there should be any problematic race, but I'll 
>> >> check again.
>> >
>> > I think the 'periodic' name might be confusing things... The Xen timers are
>> > all single-shot, it's just that start_stimer() is re-called after a
>> > successful poll if the viridian timer is configured to be periodic. So I
>> > don't think there is case where the underlying Xen timer could actually be
>> > running when we enter start_stimer().
>> 
>> One of the callers of the function is the WRMSR handler. Why would
>> it be guaranteed that the timer isn't active when such a WRMSR
>> occurs?
> 
> It's not guaranteed on entry, but the WRMSR handler always calls 
> stop_stimer() before calling start_stimer() which AFAICT should guarantee the 
> timer is not running when start_stimer() is called.

I've looked only briefly, but the stop_timer() -> deactivate_timer() call
chain doesn't look to wait for the timer handler to not be active anymore
on another CPU before returning.

Jan



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