Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753983AbbBQTpP (ORCPT ); Tue, 17 Feb 2015 14:45:15 -0500 Received: from fieldses.org ([173.255.197.46]:47246 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752692AbbBQTpN (ORCPT ); Tue, 17 Feb 2015 14:45:13 -0500 Date: Tue, 17 Feb 2015 14:45:11 -0500 From: "J. Bruce Fields" To: Linus Torvalds Cc: Jeff Layton , "Kirill A. Shutemov" , linux-fsdevel , Linux Kernel Mailing List , Christoph Hellwig , Dave Chinner , Sasha Levin Subject: Re: [GIT PULL] please pull file-locking related changes for v3.20 Message-ID: <20150217194511.GE27900@fieldses.org> References: <20150209055540.2f2a3689@tlielax.poochiereds.net> <20150216133200.GB3270@node.dhcp.inet.fi> <20150216090054.62455465@tlielax.poochiereds.net> <20150217190844.GC27900@fieldses.org> <20150217142714.36ed9ddb@tlielax.poochiereds.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2189 Lines: 57 On Tue, Feb 17, 2015 at 11:41:40AM -0800, Linus Torvalds wrote: > On Tue, Feb 17, 2015 at 11:27 AM, Jeff Layton wrote: > > > > What about this instead then? > > No. Really. > > > - leave the "drop the spinlock" thing in place in flock_lock_file for > > v3.20 > > No. The whole concept of "drop the lock in the middle" is *BROKEN*. > It's seriously crap. It's not just a bug, it's a really fundamentally > wrong thing to do. > > > - change locks_remove_flock to just walk the list and delete any locks > > associated with the filp being closed > > No. That's still wrong. You can have two people holding a write-lock. > Seriously. That's *shit*. > > The "drop the spinlock in the middle" must go. There's not even any > reason for it. Just get rid of it. There can be no deadlock if you get > rid of it, because > > - we hold the flc_lock over the whole event, so we can never see any > half-way state > > - if we actually decide to sleep (due to conflicting locks) and > return FILE_LOCK_DEFERRED, we will drop the lock before actually > sleeping, so nobody else will be deadlocking on this file lock. So any > *other* person who tries to do an upgrade will not sleep, because the > pending upgrade will have moved to the blocking list (that whole > "locks_insert_block" part. Whoops, you're right, I was forgetting that wait happens up in flock_lock_file_wait(), OK. --b. > Ergo, either we'll upgrade the lock (atomically, within flc_lock), or > we will drop the lock (possibly moving it to the blocking list). I > don't see a deadlock. > > I think your (and mine - but mine had the more fundamental problem of > never setting "old_fl" correctly at all) patch had a deadlock because > you didn't actually remove the old lock when you returned > FILE_LOCK_DEFERRED. > > But I think the correct minimal patch is actually to just remove the > "if (found)" statement. > > Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/