Re: [Embedded-pv-devel] [Xen-devel] Deadlock in /proc/xen/xenbus watch+read on 3.17+ (maybe earlier)

[Marek Marczykowski-GÃrecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Fri, Mar 20, 2015 at 12:08:48PM +0200, Vitaly Chernooky wrote:
> On Fri, Mar 20, 2015 at 6:04 AM, Marek Marczykowski-GÃrecki <
> marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
> 
> > On Thu, Mar 19, 2015 at 03:10:49PM +0200, Vitaly Chernooky wrote:
> > > David,
> > >
> > > On Thu, Mar 19, 2015 at 3:00 PM, David Vrabel <david.vrabel@xxxxxxxxxx>
> > > wrote:
> > >
> > > > On 19/03/15 12:10, Iurii Konovalenko wrote:
> > > > > Hi, guys!
> > > > >
> > > > > When I read, that I am not alone and that issue depends on kernel
> > > > > version, I decided to continue investigation.
> > > > > And I found why our threads locks on read/write operations.
> > > > > On Linux kernel 3.14+ syscalls of file read and write changed a bit:
> > > > > fdget() function was replaced by fdget_pos() - it is fdget() function
> > > > > plus additional position mutex lock for files with FMODE_ATOMIC_POS
> > > > > (files for inodes with S_IFREG flag set - regular nodes). As I
> > thought
> > > > > our xen files are not regular and nonseekable, I hoped this flag is
> > > > > not set. But it is set. It is because our file system is created by
> > > > > function simple_fill_super(), and inside it this flag is hardly set:
> > > > > inode->i_mode = S_IFREG | files->mode;
> > > > > So, as a fast hack I made a patch: just made copy of this function
> > for
> > > > > xen, which does not set this flag. It works for me. Could you please
> > > > > check if it works for you.
> > > >
> > > > I still can't get this to deadlock, but why not clear FMODE_ATOMIC_POS
> > > > in xenbus_file_open() ?
> > > >
> > >
> > > Because it is not the root of issue. FMODE_ATOMIC_POS is just one of
> > > results of bug. Iurii has fixed the root of issue but in suboptimal way.
> > So
> > > we just need to have found optimal way.
> >
> > I can just confirm that:
> > 1. (unsurprisingly) the bug is still present in 4.0-rc4
> > 2. both proposed fixes are effective
> >
> > I'm not sure if removing S_IFREG completely is a good idea, I guess
> > there will be much more side effects...
> > What about another idea: xenbus_file_open uses nonseekable_open - this
> > looks like a good place to clear FMODE_ATOMIC_POS if present? It
> > doesn't make sense to get a lock for position on nonseekable file,
> > right?
> >
> 
> The Open Group Base Specifications Issue 7 IEEE Std 1003.1, 2013 Edition
> requires from regular files to be seekable. But Linux kernel looks like
> Linus has own opinion on it :(((

Maybe the better idea would be to change filetype of xenbus (and others)
to S_IFIFO or something like this (but keep the file type present,
instead of removing it completely).

Regarding the implementation, maybe simple_fill_super can be modified to
not add S_IFREG if other file type is already present in files->mode?

-- 
Best Regards,
Marek Marczykowski-GÃrecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Attachment: pgpJtUY99_OYd.pgp
Description: PGP signature

_______________________________________________
Embedded-pv-devel mailing list
Embedded-pv-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/embedded-pv-devel