From patchwork Thu Aug 3 13:45:01 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Trond Myklebust X-Patchwork-Id: 9879133 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id D484260360 for ; Thu, 3 Aug 2017 13:45:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CA097287B9 for ; Thu, 3 Aug 2017 13:45:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BEEEF2886B; Thu, 3 Aug 2017 13:45:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3CA242875C for ; Thu, 3 Aug 2017 13:45:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751870AbdHCNpq (ORCPT ); Thu, 3 Aug 2017 09:45:46 -0400 Received: from mail-it0-f68.google.com ([209.85.214.68]:35193 "EHLO mail-it0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751804AbdHCNpl (ORCPT ); Thu, 3 Aug 2017 09:45:41 -0400 Received: by mail-it0-f68.google.com with SMTP id v127so1319072itd.2 for ; Thu, 03 Aug 2017 06:45:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:subject:date:message-id:in-reply-to:references; bh=Cr9mk9+z6iZ3p4C6EZvFVIHsPUmXJsgZKdKwsxPrMGQ=; b=YHpURRxrhD39SHaY1E4F+eEdylKEvaYHOQt6Mutv85aYrWNIR04xK3jP808xdeCKHU vRd36H439PiTmP4c/kbvv2jAieCXON98c/jWrEiANUygicfWFFjvgGIQGoD23uStztoX kvoEE7u7trhdx1pDssN6hq7Cd/81XZ7JUCZI3MXxuiuDcowucp4PqX5Nkzl/lB8645DE n5DJLnSFjYVqwip7Krq6cCXnARnN16zahjn60YPe3M0xJkPG3RGHqruL5WeSBUJvFWIK 1PnJm1US26aD7xqlHWVXA66dUKyRaMApAaMH3t40yf3hEgxo+UgL+R2Qlz7f0TZLM0hA Bpdg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:subject:date:message-id :in-reply-to:references; bh=Cr9mk9+z6iZ3p4C6EZvFVIHsPUmXJsgZKdKwsxPrMGQ=; b=K1tpoAej0X4Wy4V3fiSo8kggmPkdUKQGoWTA4BYfh84H0pGbnL9J5vGNqzd6TWlL/+ oEfsf3lnCaIGpR5f9nUm2HcNbVwOxq28GXuBPkaSoKMZzV9VQT8IffCAsiWnI6Q2iYGj ZGtJVU/Im5Ie6pgjs+fXjGw5XF/OZNW+ExWqNd2c0QdSW8Eh37VZak+O5/rdp/mGmp5K 6sEwmoNbpDhrF5OP65GukxWstPDZAxm+An0rrE/FTbAtrVkEnfMX+9s+CAlUYVbzMOCd AdtV42pYmSOnamVFEeVbhkxjeb922RM+fGc8uzoF4ycNjGcwwuTC1Xq2vZyB7ZUZyhm9 H8wQ== X-Gm-Message-State: AIVw113lZHxId2u030f42t+AoV6+AnvsxncjSNxg0Rgo+dt42w/dWYrS AHvgkb4tHWdpvg== X-Received: by 10.36.123.23 with SMTP id q23mr1785782itc.69.1501767940548; Thu, 03 Aug 2017 06:45:40 -0700 (PDT) Received: from localhost.localdomain (50-108-95-251.adr01.mskg.mi.frontiernet.net. [50.108.95.251]) by smtp.gmail.com with ESMTPSA id q90sm5759547ioi.41.2017.08.03.06.45.39 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 03 Aug 2017 06:45:40 -0700 (PDT) From: Trond Myklebust To: Chuck Lever , linux-nfs@vger.kernel.org Subject: [PATCH v2 06/28] NFS: Fix an ABBA issue in nfs_lock_and_join_requests() Date: Thu, 3 Aug 2017 09:45:01 -0400 Message-Id: <20170803134523.4922-7-trond.myklebust@primarydata.com> X-Mailer: git-send-email 2.13.3 In-Reply-To: <20170803134523.4922-6-trond.myklebust@primarydata.com> References: <20170803134523.4922-1-trond.myklebust@primarydata.com> <20170803134523.4922-2-trond.myklebust@primarydata.com> <20170803134523.4922-3-trond.myklebust@primarydata.com> <20170803134523.4922-4-trond.myklebust@primarydata.com> <20170803134523.4922-5-trond.myklebust@primarydata.com> <20170803134523.4922-6-trond.myklebust@primarydata.com> Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP All other callers of nfs_page_group_lock() appear to already hold the page lock on the head page, so doing it in the opposite order here is inefficient, although not deadlock prone since we roll back all locks on contention. Signed-off-by: Trond Myklebust --- fs/nfs/write.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/fs/nfs/write.c b/fs/nfs/write.c index 1ca759719429..c940e615f5dc 100644 --- a/fs/nfs/write.c +++ b/fs/nfs/write.c @@ -383,7 +383,7 @@ nfs_unroll_locks_and_wait(struct inode *inode, struct nfs_page *head, int ret; /* relinquish all the locks successfully grabbed this run */ - for (tmp = head ; tmp != req; tmp = tmp->wb_this_page) + for (tmp = head->wb_this_page ; tmp != req; tmp = tmp->wb_this_page) nfs_unlock_request(tmp); WARN_ON_ONCE(test_bit(PG_TEARDOWN, &req->wb_flags)); @@ -395,7 +395,7 @@ nfs_unroll_locks_and_wait(struct inode *inode, struct nfs_page *head, spin_unlock(&inode->i_lock); /* release ref from nfs_page_find_head_request_locked */ - nfs_release_request(head); + nfs_unlock_and_release_request(head); ret = nfs_wait_on_request(req); nfs_release_request(req); @@ -484,10 +484,6 @@ nfs_lock_and_join_requests(struct page *page) int ret; try_again: - total_bytes = 0; - - WARN_ON_ONCE(destroy_list); - spin_lock(&inode->i_lock); /* @@ -502,6 +498,16 @@ nfs_lock_and_join_requests(struct page *page) return NULL; } + /* lock the page head first in order to avoid an ABBA inefficiency */ + if (!nfs_lock_request(head)) { + spin_unlock(&inode->i_lock); + ret = nfs_wait_on_request(head); + nfs_release_request(head); + if (ret < 0) + return ERR_PTR(ret); + goto try_again; + } + /* holding inode lock, so always make a non-blocking call to try the * page group lock */ ret = nfs_page_group_lock(head, true); @@ -509,13 +515,14 @@ nfs_lock_and_join_requests(struct page *page) spin_unlock(&inode->i_lock); nfs_page_group_lock_wait(head); - nfs_release_request(head); + nfs_unlock_and_release_request(head); goto try_again; } /* lock each request in the page group */ - subreq = head; - do { + total_bytes = head->wb_bytes; + for (subreq = head->wb_this_page; subreq != head; + subreq = subreq->wb_this_page) { /* * Subrequests are always contiguous, non overlapping * and in order - but may be repeated (mirrored writes). @@ -541,9 +548,7 @@ nfs_lock_and_join_requests(struct page *page) return ERR_PTR(ret); } - - subreq = subreq->wb_this_page; - } while (subreq != head); + } /* Now that all requests are locked, make sure they aren't on any list. * Commit list removal accounting is done after locks are dropped */