Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756268AbbBQMrD (ORCPT ); Tue, 17 Feb 2015 07:47:03 -0500 Received: from cdptpa-outbound-snat.email.rr.com ([107.14.166.229]:40426 "EHLO cdptpa-oedge-vip.email.rr.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756214AbbBQMqe (ORCPT ); Tue, 17 Feb 2015 07:46:34 -0500 From: Jeff Layton To: linux-fsdevel@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Linus Torvalds , "Kirill A. Shutemov" , "J. Bruce Fields" , Christoph Hellwig , Dave Chinner , Sasha Levin Subject: [PATCH 2/4] locks: remove conditional lock release in middle of flock_lock_file Date: Tue, 17 Feb 2015 07:46:28 -0500 Message-Id: <1424177190-14252-3-git-send-email-jeff.layton@primarydata.com> X-Mailer: git-send-email 2.1.0 In-Reply-To: <1424177190-14252-1-git-send-email-jeff.layton@primarydata.com> References: <1424177190-14252-1-git-send-email-jeff.layton@primarydata.com> X-RR-Connecting-IP: 107.14.168.118:25 X-Cloudmark-Score: 0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2013 Lines: 62 As Linus pointed out: Say we have an existing flock, and now do a new one that conflicts. I see what looks like three separate bugs. - We go through the first loop, find a lock of another type, and delete it in preparation for replacing it - we *drop* the lock context spinlock. - BUG #1? So now there is no lock at all, and somebody can come in and see that unlocked state. Is that really valid? - another thread comes in while the first thread dropped the lock context lock, and wants to add its own lock. It doesn't see the deleted or pending locks, so it just adds it - the first thread gets the context spinlock again, and adds the lock that replaced the original - BUG #2? So now there are *two* locks on the thing, and the next time you do an unlock (or when you close the file), it will only remove/replace the first one. ...remove the "drop the spinlock" code in the middle of this function as it has always been suspicious. Reported-by: Linus Torvalds Signed-off-by: Jeff Layton --- fs/locks.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/fs/locks.c b/fs/locks.c index 7998f670812c..00c105f499a2 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -901,16 +901,6 @@ static int flock_lock_file(struct file *filp, struct file_lock *request) goto out; } - /* - * If a higher-priority process was blocked on the old file lock, - * give it the opportunity to lock the file. - */ - if (found) { - spin_unlock(&ctx->flc_lock); - cond_resched(); - spin_lock(&ctx->flc_lock); - } - find_conflict: list_for_each_entry(fl, &ctx->flc_flock, fl_list) { if (!flock_locks_conflict(request, fl)) -- 2.1.0 -- 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/