From patchwork Tue Feb 17 00:21:30 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Torvalds X-Patchwork-Id: 5835331 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 98E66BF440 for ; Tue, 17 Feb 2015 00:21:49 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BF3D8201FB for ; Tue, 17 Feb 2015 00:21:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0287C201FA for ; Tue, 17 Feb 2015 00:21:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751511AbbBQAVc (ORCPT ); Mon, 16 Feb 2015 19:21:32 -0500 Received: from mail-ie0-f182.google.com ([209.85.223.182]:37793 "EHLO mail-ie0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751244AbbBQAVb (ORCPT ); Mon, 16 Feb 2015 19:21:31 -0500 Received: by iecrl12 with SMTP id rl12so35044659iec.4; Mon, 16 Feb 2015 16:21:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=stN5GlV9PuG54Or3iBIh38J/ow2B07nLygO5Mo4aJ/8=; b=IQLfX6v6m3EJoJf+iEgUIiTrjqdYhy3v/iDUbHzYnHdAQ0zKmqDcEcivz1NvtgJUeA GpCJhd4ITlyHL8hs4NlrgrtknbTsN9bQc2uLUuNcEEwLhqoBvoP78XcxLPX9GWs/mZr5 fpLFr8o081gWvuk9iimZBUoh9UDX92/iAzFwOsgi53fkBaeckKZ+nNe1VxmK2YKqbTke aiA31kOM5CP4I8y63MZm3QCAfk/ir9nQ+okRqIgmpSL8DyHdxaZtTZN1an1BPJVRuBWK 703nQ9ogWgznoRI1XEkFB1mDSN/b404sKgRnsauMReuYy0ldxp0tXZNL/MjFLwZezwOA 8jtw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=stN5GlV9PuG54Or3iBIh38J/ow2B07nLygO5Mo4aJ/8=; b=g88IkGoFjUNQ0cJ+ypI0kUdPTwvh/vukDoyzzTtdUasJznCnGnATtFL38nUYMbNPzb TXiuPW6n275jUOZYxJchGw9hGVtN4eoEL5NIlU/dzJTZenS+19Q4U+5eMhIIYbEVMxkw rWNQDpzjyEqp/byIv42S5FuY/AeXBt1O3pA54= MIME-Version: 1.0 X-Received: by 10.107.7.154 with SMTP id g26mr23106875ioi.64.1424132490353; Mon, 16 Feb 2015 16:21:30 -0800 (PST) Received: by 10.36.58.135 with HTTP; Mon, 16 Feb 2015 16:21:30 -0800 (PST) In-Reply-To: <20150216190254.3b66a9ba@tlielax.poochiereds.net> References: <20150209055540.2f2a3689@tlielax.poochiereds.net> <20150216133200.GB3270@node.dhcp.inet.fi> <20150216090054.62455465@tlielax.poochiereds.net> <20150216190254.3b66a9ba@tlielax.poochiereds.net> Date: Mon, 16 Feb 2015 16:21:30 -0800 X-Google-Sender-Auth: luPSRrbOXw7hY4jHRO30kvOefl8 Message-ID: Subject: Re: [GIT PULL] please pull file-locking related changes for v3.20 From: Linus Torvalds To: Jeff Layton Cc: "Kirill A. Shutemov" , linux-fsdevel , Linux Kernel Mailing List , "J. Bruce Fields" , Christoph Hellwig , Dave Chinner , Sasha Levin Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID,T_RP_MATCHES_RCVD,T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Feb 16, 2015 at 4:02 PM, Jeff Layton wrote: > > Now that I look, it may be best to just revert this whole set for now. > Linus, are you amenable to doing that? Sure. But I'd prefer seeing how hard it would be to fix things first. If this was at the end of the release cycle, I'd revert it immediately. As it is, try to see how had it is. The bugs I found might be as easy as just the attached (TOTALLY UNTESTED) patch. The comment about a higher-priority process and sched_resced() is just complete and utter crap. If somebody holds a read lock and upgrades it to a write lock, there is absolutely *zero* reason to let some higher-priority process get the write-lock first - that's just simply semantically wrong bullshit. "Higher priority" does not mean "can punch through locks". Removing the silly incorrect counts should be trivial too. There really are not very many users, and they can just walk the list instead. Now, if you've found other more fundamental bugs that look unfixable, then that might mean that reverting it all is unavoidable, but.. Linus fs/locks.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/fs/locks.c b/fs/locks.c index 4753218f308e..8fbf81429608 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -867,12 +867,11 @@ static int posix_locks_deadlock(struct file_lock *caller_fl, */ static int flock_lock_file(struct file *filp, struct file_lock *request) { - struct file_lock *new_fl = NULL; + struct file_lock *new_fl = NULL, *old_fl = NULL; struct file_lock *fl; struct file_lock_context *ctx; struct inode *inode = file_inode(filp); int error = 0; - bool found = false; LIST_HEAD(dispose); ctx = locks_get_lock_context(inode); @@ -894,27 +893,18 @@ static int flock_lock_file(struct file *filp, struct file_lock *request) continue; if (request->fl_type == fl->fl_type) goto out; - found = true; - locks_delete_lock_ctx(fl, &ctx->flc_flock_cnt, &dispose); + old_fl = NULL; break; } if (request->fl_type == F_UNLCK) { - if ((request->fl_flags & FL_EXISTS) && !found) + if (old_fl) + locks_delete_lock_ctx(old_fl, &ctx->flc_flock_cnt, &dispose); + else if (request->fl_flags & FL_EXISTS) error = -ENOENT; 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)) @@ -928,6 +918,8 @@ find_conflict: } if (request->fl_flags & FL_ACCESS) goto out; + if (old_fl) + locks_delete_lock_ctx(old_fl, &ctx->flc_flock_cnt, &dispose); locks_copy_lock(new_fl, request); locks_insert_lock_ctx(new_fl, &ctx->flc_flock_cnt, &ctx->flc_flock); new_fl = NULL;