Re: [Xen-devel] [PATCH 8/9] libxl: Introduce libxl__ev_devstate

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [Xen-devel] [PATCH 8/9] libxl: Introduce 
libxl__ev_devstate"):
> On Fri, 2012-01-13 at 19:25 +0000, Ian Jackson wrote:
> > Provide a new-style asynchronous facility for waiting for device
> > states on xenbus.  This will replace libxl__wait_for_device_state,
> > after the callers have been updated in later patches.
> 
> Is event-with-timeout likely to be a useful/common enough pattern to be
> worth baking into the infrastructure/helpers rather than implementing
> just for this one event type? (if yes then, "I will refactor for the
> second user is a valid response").

I'm not convinced.  I thought of this but I think it would result in
flabby code - all the libxl__ev_register functions would gain a new
timeout parameter (and note that the timeout machinery has both
absolute and relative timeouts...)

I think when we have a second user it might be worth seeing if some
commonality could be extracted but TBH I doubt it would make the code
smaller or simpler.

> > +static void devstate_timeout(libxl__egc *egc, libxl__ev_time *ev,
> > +                             const struct timeval *requested_abs)
> > +{
> > +    EGC_GC;
> > +    libxl__ev_devstate *ds = CONTAINER_OF(ev, *ds, timeout);
> > +    LIBXL__LOG(CTX, LIBXL__LOG_DEBUG, "backend %s wanted state %d "
> > +               " timed out", ds->watch.path, ds->wanted);
> > +    libxl__ev_devstate_cancel(gc, ds);
> 
> What prevents racing here with the watch happening? Might the caller see
> two callbacks?

  static inline void libxl__ev_devstate_cancel(libxl__gc *gc,
                                               libxl__ev_devstate *ds)
  {
      libxl__ev_time_deregister(gc,&ds->timeout);
      libxl__ev_xswatch_deregister(gc,&ds->watch);
  }

So, no.  When the timeout happens, the ev xswatch is deregistered and
can thereafter no longer generate callbacks.  If there are any
xenstore watch events in the pipeline for deregistered ev_xswatch's,
they're discarded by watchfd_callback.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel