From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Subject: Re: [PATCH] [RFC] Fix deadlock on regular nonseekable files Date: Fri, 20 Mar 2015 19:35:06 +0000 Message-ID: <20150320193504.GD29656@ZenIV.linux.org.uk> References: <20150320134232.GX29656@ZenIV.linux.org.uk> <20150320144634.GY29656@ZenIV.linux.org.uk> <20150320175516.GC29656@ZenIV.linux.org.uk> <20150320190052.GF2321@mail-itl> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Vitaly Chernooky , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Linus Torvalds , David Vrabel , Iurii Konovalenko , Ian Campbell , Boris Ostrovsky , Andrii Anisov , Artem Mygaiev To: Marek =?iso-8859-1?Q?Marczykowski-G=F3recki?= Return-path: Received: from zeniv.linux.org.uk ([195.92.253.2]:59948 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750992AbbCTTfO (ORCPT ); Fri, 20 Mar 2015 15:35:14 -0400 Content-Disposition: inline In-Reply-To: <20150320190052.GF2321@mail-itl> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, Mar 20, 2015 at 08:00:52PM +0100, Marek Marczykowski-G=F3recki = wrote: > > What the devil does that have to do with seeks, anyway? Exact > > same problem will happen for blocking read() vs. another read() att= empts > > on the same descriptor. With perfectly accepted lseek() (which wil= l also > > have to block, as per 2.9.7). >=20 > Yes, the problem here is because this particular file (/proc/xen/xenb= us) > blocks the read() operation waiting for new events. Because of said > commit, now it also blocks write() operation used to send some reques= t > (which would result in some response, so unblocking read() call). It > shouldn't be a normal file in the first place... Aha. OK, so you have something that looks a whole lot like a FIFO in that respect, and this semantics simply isn't compatible with read() being atomic wrt write(). So just have that flag explicitly knocked out in your ->open(), prefera= bly with a comment explaining why is that done. Having lseek() is a red he= rring in that respect - the same problem would exist if that file *did* have something done on lseek(). That's actually what I'm objecting against - "uses nonseekable_open()" = is used a weird proxy for "can't have read(), write(), etc. atomic wrt eac= h other". It's not true in either direction - there's a lot of e.g. proc= fs files that are just fine with current exclusion and there can very well be files _not_ using nonseekable_open() that would break the same way and for the same reasons as /proc/xen/xenbus does. It's trivial to fix - either by explicit filp->f_mode &=3D ~FMODE_ATOMI= C_POS; in xenbus_file_open(), or by adding static inline void no_atomic_pos(struct file *f) { f->f_mode &=3D ~FMODE_ATOMIC_POS; } somewhere in include/linux/fs.h and having it called in the same xenbus_file_open(). Either way, it ought to come with something along the lines of /* * we can't live with read() vs. write() atomicity, since we use * write() as source of events returned by read() and write() * called after another thread has blocked in read() waiting for * events cannot be required to wait for that read() to finish. */ next to this removal of FMODE_ATOMIC_POS, whichever way we express it..= =2E Objections? -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html