From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: Re: [PATCH XEN v6 25/32] tools/libs/gnttab: Extensive updates to API documentation. Date: Wed, 9 Dec 2015 13:37:15 +0000 Message-ID: <20151209133715.GO23818@citrix.com> References: <1449141675.4424.125.camel@citrix.com> <1449141749-14940-1-git-send-email-ian.campbell@citrix.com> <1449141749-14940-26-git-send-email-ian.campbell@citrix.com> <20151209124112.GM23818@citrix.com> <1449666034.16124.220.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <1449666034.16124.220.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: ian.jackson@eu.citrix.com, Daniel De Graaf , Wei Liu , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Wed, Dec 09, 2015 at 01:00:34PM +0000, Ian Campbell wrote: > On Wed, 2015-12-09 at 12:41 +0000, Wei Liu wrote: > > > + */ > > > + > > > +/* > > > =A0 * Grant Table Interface (making use of grants from other domains) > > > =A0 */ > > > =A0 > > > =A0typedef struct xengntdev_handle xengnttab_handle; > > > =A0 > > > =A0/* > > > - * Note: > > > - * After fork a child process must not use any opened xc gnttab > > > - * handle inherited from their parent. They must open a new handle if > > > - * they want to interact with xc. > > > + * Returns a handle onto the grant table driver.=A0=A0Logs errors. > > > + * > > > + * Note: After fork a child process must not use any opened gnttab > > > + * handle inherited from their parent, more access any grant mapped > > > + * areas associated with that handle. > > > + * > > = > > And this could use the same cloexec trick as you do in other patch for > > privcmd. > = > I think this statement "do not use the handle" still stands regardless of > the underlying fd being cloexec'd. > = Yes. > However, this did make me think about a comment elsewhere, specifically t= he > various instances of: > =A0* This is the only function which may be safely called on a > =A0* xen_handle in a child after a fork. > for several xen_close() functions. > = > This is not really true if the fd is cloexec, since then _close() will > either fail (EBADF) or, worse, close some other freshly opened file > descriptor. > = > There seems to be two choices here, either require all osdep backends to > make their fds O_CLOEXEC (which might involve tolerating a racy > fcntl+FD_CLOEXEC pattern after open on some platforms) or don't set > O_CLOEXEC ever and declare that the application is responsible for closing > after fork, and for taking care of the corner cases themselves in > multithreaded applications. > = > The former seems friendlier to me, even if some platforms need to use > FD_CLOEXEC. > = > Hrm, maybe I can extend the atfork trick to cover the open+fcntl bits. > Hopefully there is no issue with using pthreads from each of the affected > libraries. > = > So, I think the advice in the comment would then be: > = > If you fork and then exec then you must not (and need not) call _clos= e() > or any other function on the handle. > = > If you fork but do not exec then it is permissible to call _close(), = but > it is not permissible to call any other function on the handle. > = > Need to think about that wording. > = Looks sensible to me FWIW. > Ian.