2022-03-22 21:43:34

by Trond Myklebust

Subject: [PATCH 1/2] NFS: Don't deadlock when cookie hashes collide

From: Trond Myklebust <[email protected]>

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 <[email protected]>
---
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

2022-03-23 01:10:06

by Trond Myklebust

Subject: [PATCH 2/2] NFS: Fix revalidation of empty readdir pages

From: Trond Myklebust <[email protected]>

If the page is empty, we need to check the array->last_cookie instead of
the first entry. Add a helper for the cases where we care.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/dir.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 17986c0019d4..bac4cf1a308e 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -252,6 +252,11 @@ static void nfs_readdir_page_array_free(struct page *page)
}
}

+static u64 nfs_readdir_array_index_cookie(struct nfs_cache_array *array)
+{
+ return array->size == 0 ? array->last_cookie : array->array[0].cookie;
+}
+
static void nfs_readdir_array_set_eof(struct nfs_cache_array *array)
{
array->page_is_eof = 1;
@@ -369,7 +374,7 @@ static bool nfs_readdir_page_validate(struct page *page, u64 last_cookie,

if (array->change_attr != change_attr)
ret = false;
- if (array->size > 0 && array->array[0].cookie != last_cookie)
+ if (nfs_readdir_array_index_cookie(array) != last_cookie)
ret = false;
kunmap_atomic(array);
return ret;
@@ -480,7 +485,7 @@ static void nfs_readdir_seek_next_array(struct nfs_cache_array *array,
desc->cache_entry_index = 0;
desc->page_index++;
} else
- desc->last_cookie = array->array[0].cookie;
+ desc->last_cookie = nfs_readdir_array_index_cookie(array);
}

static void nfs_readdir_rewind_search(struct nfs_readdir_descriptor *desc)
--
2.35.1