Re: [PATCH 10/23] libxl: events: Provide libxl__ev_evtchn*

[Ian Campbell <Ian.Campbell@citrix.com>]

On Thu, 2013-12-19 at 17:47 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH 10/23] libxl: events: Provide libxl__ev_evtchn*"):
> > > [...]
> > > +struct libxl__ev_evtchn {
> > > +    /* caller must fill these in, and they must all remain valid */
> > > +    libxl__ev_evtchn_callback *callback;
> > > +    int port;
> > 
> > evtchn_port_t?
> 
> Leaving it as "int" means that the caller can put -1 in it if the
> struct isn't in use or the port not allocated.  That avoids the caller
> needing to have a separate copy of the value, or a separate boolean.
> And later, we do.

I suppose that's vaguely the same sort of thing as
evtchn_port_or_error_t, but not quite.

> 
> > > @@ -343,6 +354,10 @@ struct libxl__ctx {
> > >      uint32_t watch_counter; /* helps disambiguate slot reuse */
> > >      libxl__ev_fd watch_efd;
> > >  
> > > +    xc_evtchn *xce; /* for waiting use only libxl__ev_evtchn* */
> > 
> > The comment means "don't use directly, use libxl__ev-evtchn"?
> >
> > Or does it imply that for uses other than waiting you may use it
> > directly?
> 
> The latter.  I'm open to suggestions for improved wording.

"waiting must only be done via libxl__ev_..." not much better though.

> 
> > > +    LIBXL_LIST_HEAD(, libxl__ev_evtchn) evtchns_waiting;
> > 
> > Are there any locking requirements relating to this list?
> 
> They are the same as all the other event-related data structures in
> the ctx: the ctx mutex protects them.  This isn't particularly
> explicit here but I think it's fairly obvious.  Only the event code
> looks at this list anyway and it all takes a gc.

OK.

> > > + * You must not wait on the same port twice at once (that is, with
> > > + * two separate libxl__ev_evtchn's).
> > 
> > Doing so would require per wait xce_handles? Worth avoiding.
> 
> It would be possible to improve the data structure and event code to
> support this.  The obvious way would be to keep scanning the
> evtchns_waiting list after we found one waiter, but you'd have to do
> something fiddly to avoid reentering the same waiter again right away.

Yes, easiest to avoid until the requirement occurs I think.

Ian.