Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   trondmy@kernel.org
To:     linux-nfs@vger.kernel.org
Subject: [PATCH 1/2] NFS: Don't deadlock when cookie hashes collide
Date:   Tue, 22 Mar 2022 16:54:20 -0400
Message-Id: <20220322205421.726627-2-trondmy@kernel.org>
In-Reply-To: <20220322205421.726627-1-trondmy@kernel.org>
References: <20220322205421.726627-1-trondmy@kernel.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: Trond Myklebust <trond.myklebust@hammerspace.com>

In the very rare case where the readdir reply contains multiple cookies
that map to the same hash value, we can end up deadlocking waiting for a
page lock that we already hold. In this case we should fail the page
lock by using grab_cache_page_nowait().

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/dir.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 7e12102b29e7..17986c0019d4 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -381,23 +381,28 @@ static void nfs_readdir_page_unlock_and_put(struct page *page)
 	put_page(page);
 }
 
+static void nfs_readdir_page_init_and_validate(struct page *page, u64 cookie,
+					       u64 change_attr)
+{
+	if (PageUptodate(page)) {
+		if (nfs_readdir_page_validate(page, cookie, change_attr))
+			return;
+		nfs_readdir_clear_array(page);
+	}
+	nfs_readdir_page_init_array(page, cookie, change_attr);
+	SetPageUptodate(page);
+}
+
 static struct page *nfs_readdir_page_get_locked(struct address_space *mapping,
-						u64 last_cookie,
-						u64 change_attr)
+						u64 cookie, u64 change_attr)
 {
-	pgoff_t index = nfs_readdir_page_cookie_hash(last_cookie);
+	pgoff_t index = nfs_readdir_page_cookie_hash(cookie);
 	struct page *page;
 
 	page = grab_cache_page(mapping, index);
 	if (!page)
 		return NULL;
-	if (PageUptodate(page)) {
-		if (nfs_readdir_page_validate(page, last_cookie, change_attr))
-			return page;
-		nfs_readdir_clear_array(page);
-	}
-	nfs_readdir_page_init_array(page, last_cookie, change_attr);
-	SetPageUptodate(page);
+	nfs_readdir_page_init_and_validate(page, cookie, change_attr);
 	return page;
 }
 
@@ -435,11 +440,13 @@ static void nfs_readdir_page_set_eof(struct page *page)
 static struct page *nfs_readdir_page_get_next(struct address_space *mapping,
 					      u64 cookie, u64 change_attr)
 {
+	pgoff_t index = nfs_readdir_page_cookie_hash(cookie);
 	struct page *page;
 
-	page = nfs_readdir_page_get_locked(mapping, cookie, change_attr);
+	page = grab_cache_page_nowait(mapping, index);
 	if (!page)
 		return NULL;
+	nfs_readdir_page_init_and_validate(page, cookie, change_attr);
 	if (nfs_readdir_page_last_cookie(page) != cookie)
 		nfs_readdir_page_reinit_array(page, cookie, change_attr);
 	return page;
-- 
2.35.1