From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jonathan Creekmore Subject: Re: [PATCH] libxenstore: Use poll() with a non-blocking read() Date: Tue, 18 Aug 2015 07:49:16 -0700 Message-ID: References: <1439502287-5520-1-git-send-email-jonathan.creekmore@gmail.com> <1439715572.3480.19.camel@citrix.com> <55D2FF72.7050006@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZRiCX-0000Cm-Gz for xen-devel@lists.xenproject.org; Tue, 18 Aug 2015 14:49:21 +0000 Received: by pdob1 with SMTP id b1so13507072pdo.2 for ; Tue, 18 Aug 2015 07:49:18 -0700 (PDT) In-Reply-To: <55D2FF72.7050006@citrix.com> (David Vrabel's message of "Tue, 18 Aug 2015 10:48:34 +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: David Vrabel Cc: Ian Campbell , stefano.stabellini@eu.citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xenproject.org, BorisOstrovsky , wei.liu@citrix.com List-Id: xen-devel@lists.xenproject.org David Vrabel 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.