Ian Jackson <ian.jack...@eu.citrix.com> 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.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel