From mboxrd@z Thu Jan 1 00:00:00 1970 From: "J. Bruce Fields" Subject: Re: [PATCH] locks: try to catch potential deadlock between file-private and classic locks from same process Date: Tue, 4 Mar 2014 15:35:41 -0500 Message-ID: <20140304203541.GH12805@fieldses.org> References: <1393960249-18961-1-git-send-email-jlayton@redhat.com> <20140304193551.GG12805@fieldses.org> <20140304151451.07530a98@tlielax.poochiereds.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org, luto@amacapital.net To: Jeff Layton Return-path: Received: from fieldses.org ([174.143.236.118]:59208 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757250AbaCDUfm (ORCPT ); Tue, 4 Mar 2014 15:35:42 -0500 Content-Disposition: inline In-Reply-To: <20140304151451.07530a98@tlielax.poochiereds.net> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, Mar 04, 2014 at 03:14:51PM -0500, Jeff Layton wrote: > On Tue, 4 Mar 2014 14:35:51 -0500 > "J. Bruce Fields" wrote: > > > On Tue, Mar 04, 2014 at 02:10:49PM -0500, Jeff Layton wrote: > > > My expectation is that programs shouldn't mix classic and file-private > > > locks, but Glenn Skinner pointed out to me that that may occur at times > > > even if the programmer isn't aware. > > > > > > Suppose we have a program that uses file-private locks. That program > > > then links in a library that uses classic POSIX locks. If those locks > > > end up conflicting and one is using blocking locks, then the program > > > could end up deadlocked. > > > > > > Try to catch this situation in posix_locks_deadlock by looking for the > > > case where the blocking lock was set by the same process but has a > > > different type, and have the kernel return EDEADLK if that occurs. > > > > > > This check is not perfect. You could (in principle) have a threaded > > > process that is using classic locks in one thread and file-private locks > > > in another. That's not necessarily a deadlockable situation but this > > > check would cause an EDEADLK return in that case. > > > > > > By the same token, you could also have a file-private lock that was > > > inherited across a fork(). If the inheriting process ends up blocking on > > > that while trying to set a classic POSIX lock then this check would miss > > > it and the program would deadlock. > > > > If the caller's not prepared for the library to use classic posix locks, > > then it's not going to know how to recover from this EDEADLCK either, is > > it? > > > > Well, callers should be aware of that if we take this change. The > semantics aren't yet set in stone... > > > I guess I don't understand how this helps anyone. > > > > Has it ever made sense for a library function and its caller to both use > > classic posix locking on the same file without any coordination? > > > > Not really, but that doesn't mean that it isn't done... ;) > > > Besides the first-close problem there's the problem that locks merge, so > > for example you can't hold your own lock across a call to a function > > that grabs and drops a lock on the same file. > > > > It depends, but you're basically correct... > > It's likely that if the above situation occurred with a program using > classic locks, then those locks were silently lost at times. It's also > plausible that when it occurs that no one is aware of it due to the way > POSIX locks work. > > If the program switched to using file-private locks and the library > stays using classic locks (or vice versa), you then potentially trade > that silent loss of locks for a deadlock (since classic and > file-private locks always conflict). > > So, the idea would be to try to catch that situation explicitly and > return a hard error instead of deadlocking. Unfortunately, it's a > little tough to do that in all cases so all this does is try to catch a > subset of them. > > Will it be helpful in the long run? I'm not sure. It seems unlikely to > harm legit use cases though, and might catch some problematic > situations. I can drop this if that's the consensus however. As a way to tell you your program is using the interface in a fundamentally buggy way, maybe hanging isn't even any worse than returning an error. I'd rather stick with the simpler-to-document behavior ("file-private & classic locks always conflict") absent a stronger argument to the contrary. --b.