Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752937AbbBQTln (ORCPT ); Tue, 17 Feb 2015 14:41:43 -0500 Received: from mail-ie0-f182.google.com ([209.85.223.182]:37383 "EHLO mail-ie0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751587AbbBQTll (ORCPT ); Tue, 17 Feb 2015 14:41:41 -0500 MIME-Version: 1.0 In-Reply-To: <20150217142714.36ed9ddb@tlielax.poochiereds.net> 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> Date: Tue, 17 Feb 2015 11:41:40 -0800 X-Google-Sender-Auth: MRwtTpYNjxCkWNFfqt07GDL6OTY Message-ID: Subject: Re: [GIT PULL] please pull file-locking related changes for v3.20 From: Linus Torvalds To: Jeff Layton Cc: "J. Bruce Fields" , "Kirill A. Shutemov" , linux-fsdevel , Linux Kernel Mailing List , Christoph Hellwig , Dave Chinner , Sasha Levin Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1937 Lines: 51 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. 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/