From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <Ian.Campbell@citrix.com>
Subject: Re: [PATCH 10/23] libxl: events: Provide
	libxl__ev_evtchn*
Date: Thu, 19 Dec 2013 17:51:36 +0000
Message-ID: <1387475496.17289.15.camel@kazak.uk.xensource.com>
References: <1387305337-15355-1-git-send-email-ian.jackson@eu.citrix.com>
	<1387305337-15355-11-git-send-email-ian.jackson@eu.citrix.com>
	<1387460612.9925.75.camel@kazak.uk.xensource.com>
	<21171.12604.288109.290100@mariner.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <21171.12604.288109.290100@mariner.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Shriram Rajagopalan <rshriram@cs.ubc.ca>, George Dunlap <George.Dunlap@eu.citrix.com>, xen-devel@lists.xensource.com, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
List-Id: xen-devel@lists.xenproject.org

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.