Re: [PATCH] libxenstore: Use poll() with a non-blocking read()

[Jonathan Creekmore <jonathan.creekmore@gmail.com>]

David Vrabel <david.vrabel@citrix.com> writes:

> On 16/08/15 09:59, Ian Campbell wrote:
>> On Thu, 2015-08-13 at 16:44 -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.
>
> I think you should make libxenstore open /dev/xen/xenbus instead.  Since
> this is a character device it should work correctly.
>

So, I did a quick test and verified that 1) /dev/xen/xenbus did exist on
my system and 2) that opening /dev/xen/xenbus instead of
/proc/xen/xenbus fixed the problem I was seeing as well. I did think
that it was a little weird that libxenstore was treating a proc file as
a character device. It makes more sense to me to open the actual
character device instead. 

> It may be necessary to try /dev/xen/xenbus first and fallback to
> /proc/xen/xenbus.
>

When did /dev/xen/xenbus get created? It is a fairly straightforward
change to just switch to /dev/xen/xenbus and is a bit larger refactor to
probe for its existence first before falling back to
/proc/xen/xenbus.

Either way, I could work up a patch to do this if people are ok with
that user-space change. 


That said, I still think that the original patch I submitted is
worthwhile to prevent a subtle, but probably benign, race condition with
changing the blocking flag on a file descriptor that is shared between
two threads.