From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jonathan Creekmore Subject: Re: [PATCH v2] libxenstore: prefer using the character device Date: Thu, 27 Aug 2015 15:34:06 -0500 Message-ID: References: <55D2FF72.7050006@citrix.com> <1440684278-16838-1-git-send-email-jonathan.creekmore@gmail.com> <20150827165643.GU22586@zion.uk.xensource.com> <21983.20700.957792.401545@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZV3sC-0001FK-2T for xen-devel@lists.xenproject.org; Thu, 27 Aug 2015 20:34:12 +0000 Received: by qkch123 with SMTP id h123so17734801qkc.0 for ; Thu, 27 Aug 2015 13:34:09 -0700 (PDT) In-Reply-To: <21983.20700.957792.401545@mariner.uk.xensource.com> (Ian Jackson's message of "Thu, 27 Aug 2015 19:03:08 +0100") 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 Jackson Cc: xen-devel@lists.xenproject.org, david.vrabel@citrix.com, Wei Liu , ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org Ian Jackson writes: > Wei Liu writes ("Re: [Xen-devel] [PATCH v2] libxenstore: prefer > using the character device"): >> On Thu, Aug 27, 2015 at 09:04:38AM -0500, Jonathan Creekmore >> wrote: >> > With the addition of FMODE_ATOMIC_POS in the Linux 3.14 >> > kernel, concurrent blocking file accesses to a single open >> > file descriptor can cause a deadlock trying to grab the file >> > position lock. If a watch has been set up, causing a >> > read_thread to blocking read on the file descriptor, then >> > future writes that would cause the background read to >> > complete will block waiting on the file position lock before >> > they can execute. This race condition only occurs when >> > libxenstore is accessing the xenstore daemon through the >> > /proc/xen/xenbus file and not through the unix domain socket, >> > which is the case when the xenstore daemon is running as a >> > stub domain or when oxenstored is passed >> > --disable-socket. Accessing the daemon from the true >> > character device also does not exhibit this problem. On >> > Linux, prefer using the character device file over the proc >> > file if the character device exists. > > I confess I still see this as working around a kernel bug. Only > this time we are switching from a buggy to non-buggy kernel > interface. > > Why don't we have the kernel provide only non-buggy interfaces ? I was just trying to implement what David suggested; CC'ing him on this as well. > >> > diff --git a/tools/xenstore/xs_lib.c >> > b/tools/xenstore/xs_lib.c index af4f75a..0c7744e 100644 --- >> > a/tools/xenstore/xs_lib.c +++ b/tools/xenstore/xs_lib.c @@ >> > -81,6 +81,8 @@ const char *xs_domain_dev(void) >> > #if defined(__RUMPUSER_XEN__) || defined(__RUMPRUN__) return >> > "/dev/xen/xenbus"; #elif defined(__linux__) >> > + if (access("/dev/xen/xenbus", F_OK) == 0) + >> > return "/dev/xen/xenbus"; > > Also, previously xs_domain_dev was a function which simply > returned a static value. I feel vaguely uneasy at putting this > kind of autodetection logic here. Not entirely; the existing code queried an environment variable first and, only if that was not set, did it return a static value. I added the autodetection logic to fall back in the case where, for some reason, /dev/xen/xenbus did not exist. Handling that kind of a fallback at a higher layer would be a larger refactor to probe for the existence of the character device first.