[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: 'Jan Beulich' <JBeulich@xxxxxxxx>
  • From: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
  • Date: Mon, 18 Mar 2019 17:06:12 +0000
  • Accept-language: en-GB, en-US
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wei.liu2@xxxxxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>, "Tim \(Xen.org\)" <tim@xxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxx>, Julien Grall <julien.grall@xxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>
  • Delivery-date: Mon, 18 Mar 2019 17:06:27 +0000
  • Ironport-data: A9a23:6cQSwa9KIu7LzqDOo5m2DrVtTnXEKcPSOkUsrf3CRtIx9vdTUMd/yF bDzn0lg3Zu1s/9EnKu3S+rwFHeSs087hS4j/rM5nJyTAWKY4fYmlUiAUUTb6YgHPkMkZlg7S pJQFwMqw+PcoXxU6t27ak0fY/bxJy3QU64gjGUCARcOLRoCscxyhiSN9NFTUzll3sUKhIyTA 0kLdNchZCZAD4GdxxOIS34RgRrWWgEP78JSaa/b66cQqMQmdbKYyMOqCWsDfoUZkMYQwU8eo YM7NEzy9CJKOozxRcxwOPefba7qBgEI1URH0Q3muKI+WL5y81/n2c85UuijIPI6SudQeP+9R Irj9Gc84f9sMJ7OjYyfeuUxT5j1kWETq3BE71Hw79wU8VwS6NcoV3VODCHNBjCrnpROJCqW6 1T1SxfnIpIOcmeRa9PXCtSIGuuGQ4CfsnUIxaex2MsqD2LfvRIr7eZ0MvYbfjL+T52rEdYqk /gRn+po3hj3OLCMZgF2edsGVo/uOoxiOmPgB+i/ez0dI//A0XLmu3TRShguFzY2BFwSVMLGJ FYDBvvMRL376cYSy/tnEMdLiMU98eaFVywqhtQ3dLXFLGnfbDqujCtycUiMEZe769ozJ5DEY 9CUt4eHIcKkh6A71WvrjEBTFDfXjONnqnCsYgEDCX+DDZKJh33NPypIWuQHw7AB1k4oaBz/+ HZwvEpqCg9R/pvpKa7w2h/iY+PRTgQ+Bvuti1IMjP4jToQY47zB1BY00OhsBddzphRsseldG VRiT78oWq8IkFKxrFqiCZAlLZaRDdPj96Ac9PjK5IcGNap0fUcIKukYDsOr7ST4kGH22/yec jnLJ1vAz6sd9MLIaoU23udrzAPPdZxHHwYWfM8xIiNPdoRNWeeGZOh28/zazkUIR+l5js4Kp cS+bxjoPz45pdylwZgZoBiEZAJqQ+SmdQhDswDwPei976NUGwU9JE06UV8rukFNdEhCEArzt 3gF7bZ7sm6PsBsWJ8U3cMOfezLHDl0tuiiex7Xw8Vhuww7Nmy4Hb7tZ+BucV+jyI/5OOZYQV +LWH3t8ptMiYFohk8TxbUV3X0egiYk4+Guk5b72bk4Sd32856J/B4JWhID47U7kDNY4GXnav vIqis3T/JAOky+q9pKBWDcFErbvXFqtc1fdy80a+8MPSZONhXdXb+JGwwMKCPen0uRxSnZ3g BiTSf7oZiOAKjMYPwsPFI9E3jnjeNCiq93Elixx0xs9P8x4Zg5bK962ZYoau3iIIgj+a86wK 5BQA82gkGXhNMnT6Xhm8Y0FLYqti9bBQfidJ4OYKLPuyV5ojYh3Xhp+WUMWkMnTFHNzayC4W WW6qaSye+JIeVDLta62/hvue2eFnBGeSvKDulEF4c6QgONWfML7tET073L69aiybv8l3CMpZ Tajms7+BKpxy3alwJb9J+sJKUH8bc5H1K0oNGDgXiQ4xOFXym51PFBmENk0dJaTszEa28lyw sXydTiplu8qrR3f8mcFoEKZ/OpIg6GYM/dLr+vEZwuh7n7Vl0ML5LWtDpbG8TEhmSaBp9QJn OK77mN1HWiTYRMoK/KlmU+ASClO9kP08q1szaFARMEdASbdXjlFAQ07Z47t4GwoX2rorY6Cw y9c7otKaIDf0ot4VTnnO5nitzYPCvW76z7s+hHI/ANo13kRlCh34hooWNU369GspNowDPkDx 1CMNQ3uf46mKAX0k/WmcDSuuOdIdAfLJeU2WPPIe/FnFkekTBfmLsTnOP4d4UIBbdjB6n8+1 1Rxpxxp2NS/lJEuZ+P4k004Gs22sxjG5pljeZu8KqahQdn2yqCHzNhVB3KVJqTBk3SJDuntq DjafJhjFeAo7EAWEoh9ZeHZj9dv99Hdz1CfK4+DCdT+Qu4qK77Pfov7cuDzVvrw19amdftzA DA1v0QRKWPFWn+1Wk6UmLZWSU8ARsXMkV5yrZkkzezGdKPKM/sYGKkC55JVImUUXGCAEhkb6 KjIRpOv2ghW27LRH6zet/oDJvgzUDRr1SC/ZLbIXxa8GvSJeqUGhc90itiR4CtLYyKA4izTw uPsXi3p4Q=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHU3X2WVMvWAzv7yUWT06FGctY0sqYRYLYAgAATJKCAABN4MP//+oUAgAARHMD///gyAIAAEO9w
  • Thread-topic: [Xen-devel] [PATCH v8 10/11] viridian: add implementation of synthetic timers

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 18 March 2019 16:56
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>; Andrew Cooper 
> <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
> <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Roger Pau 
> Monne
> <roger.pau@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Stefano Stabellini 
> <sstabellini@xxxxxxxxxx>;
> xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>; Konrad Rzeszutek Wilk 
> <konrad.wilk@xxxxxxxxxx>; Tim
> (Xen.org) <tim@xxxxxxx>
> Subject: Re: [Xen-devel] [PATCH v8 10/11] viridian: add implementation of 
> synthetic timers
> 
> >>> 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.

Oh, it looked to me like the locking would ensure the timer was deactivated and 
would not run... but I guess I may have misunderstood the code. Still even if 
both occurrences make it past the test of config.enabled all they'll do is both 
set the pending bit, as the prior version of the patch did. Although I guess 
there's now a possibility that, for one occurrence, the poll could occur before 
config.enabled is cleared and thus the timer may be rescheduled and immediately 
expire again. I'm not sure whether this is really a problem or not.

  Paul

> 
> Jan
> 


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.