* [PATCH 00/12] Readdir enhancements
@ 2020-11-02 18:06 trondmy
2020-11-02 18:06 ` [PATCH 01/12] NFS: Ensure contents of struct nfs_open_dir_context are consistent trondmy
2020-11-02 20:40 ` [PATCH 00/12] Readdir enhancements David Wysochanski
0 siblings, 2 replies; 20+ messages in thread
From: trondmy @ 2020-11-02 18:06 UTC (permalink / raw)
To: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
The following patch series performs a number of cleanups on the readdir
code.
It also adds support for 1MB readdir RPC calls on-the-wire, and modifies
the caching code to ensure that we cache the entire contents of that
1MB call (instead of discarding the data that doesn't fit into a single
page).
Trond Myklebust (12):
NFS: Ensure contents of struct nfs_open_dir_context are consistent
NFS: Clean up readdir struct nfs_cache_array
NFS: Clean up nfs_readdir_page_filler()
NFS: Clean up directory array handling
NFS: Don't discard readdir results
NFS: Remove unnecessary kmap in nfs_readdir_xdr_to_array()
NFS: Replace kmap() with kmap_atomic() in nfs_readdir_search_array()
NFS: Simplify struct nfs_cache_array_entry
NFS: Support larger readdir buffers
NFS: More readdir cleanups
NFS: nfs_do_filldir() does not return a value
NFS: Reduce readdir stack usage
fs/nfs/client.c | 4 +-
fs/nfs/dir.c | 555 ++++++++++++++++++++++++-----------------
fs/nfs/internal.h | 6 -
include/linux/nfs_fs.h | 1 -
4 files changed, 325 insertions(+), 241 deletions(-)
--
2.28.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 01/12] NFS: Ensure contents of struct nfs_open_dir_context are consistent
2020-11-02 18:06 [PATCH 00/12] Readdir enhancements trondmy
@ 2020-11-02 18:06 ` trondmy
2020-11-02 18:06 ` [PATCH 02/12] NFS: Clean up readdir struct nfs_cache_array trondmy
2020-11-02 20:40 ` [PATCH 00/12] Readdir enhancements David Wysochanski
1 sibling, 1 reply; 20+ messages in thread
From: trondmy @ 2020-11-02 18:06 UTC (permalink / raw)
To: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
Ensure that the contents of struct nfs_open_dir_context are consistent
by setting them under the file->f_lock from a private copy (that is
known to be consistent).
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/dir.c | 72 +++++++++++++++++++++++++++++++---------------------
1 file changed, 43 insertions(+), 29 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 4e011adaf967..67d8595cd6e5 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -144,20 +144,23 @@ struct nfs_cache_array {
struct nfs_cache_array_entry array[];
};
-typedef struct {
+typedef struct nfs_readdir_descriptor {
struct file *file;
struct page *page;
struct dir_context *ctx;
unsigned long page_index;
- u64 *dir_cookie;
+ u64 dir_cookie;
u64 last_cookie;
+ u64 dup_cookie;
loff_t current_index;
loff_t prev_index;
unsigned long dir_verifier;
unsigned long timestamp;
unsigned long gencount;
+ unsigned long attr_gencount;
unsigned int cache_entry_index;
+ signed char duped;
bool plus;
bool eof;
} nfs_readdir_descriptor_t;
@@ -273,7 +276,7 @@ int nfs_readdir_search_for_pos(struct nfs_cache_array *array, nfs_readdir_descri
}
index = (unsigned int)diff;
- *desc->dir_cookie = array->array[index].cookie;
+ desc->dir_cookie = array->array[index].cookie;
desc->cache_entry_index = index;
return 0;
out_eof:
@@ -298,33 +301,32 @@ int nfs_readdir_search_for_cookie(struct nfs_cache_array *array, nfs_readdir_des
int status = -EAGAIN;
for (i = 0; i < array->size; i++) {
- if (array->array[i].cookie == *desc->dir_cookie) {
+ if (array->array[i].cookie == desc->dir_cookie) {
struct nfs_inode *nfsi = NFS_I(file_inode(desc->file));
- struct nfs_open_dir_context *ctx = desc->file->private_data;
new_pos = desc->current_index + i;
- if (ctx->attr_gencount != nfsi->attr_gencount ||
+ if (desc->attr_gencount != nfsi->attr_gencount ||
!nfs_readdir_inode_mapping_valid(nfsi)) {
- ctx->duped = 0;
- ctx->attr_gencount = nfsi->attr_gencount;
+ desc->duped = 0;
+ desc->attr_gencount = nfsi->attr_gencount;
} else if (new_pos < desc->prev_index) {
- if (ctx->duped > 0
- && ctx->dup_cookie == *desc->dir_cookie) {
+ if (desc->duped > 0
+ && desc->dup_cookie == desc->dir_cookie) {
if (printk_ratelimit()) {
pr_notice("NFS: directory %pD2 contains a readdir loop."
"Please contact your server vendor. "
"The file: %.*s has duplicate cookie %llu\n",
desc->file, array->array[i].string.len,
- array->array[i].string.name, *desc->dir_cookie);
+ array->array[i].string.name, desc->dir_cookie);
}
status = -ELOOP;
goto out;
}
- ctx->dup_cookie = *desc->dir_cookie;
- ctx->duped = -1;
+ desc->dup_cookie = desc->dir_cookie;
+ desc->duped = -1;
}
if (nfs_readdir_use_cookie(desc->file))
- desc->ctx->pos = *desc->dir_cookie;
+ desc->ctx->pos = desc->dir_cookie;
else
desc->ctx->pos = new_pos;
desc->prev_index = new_pos;
@@ -334,7 +336,7 @@ int nfs_readdir_search_for_cookie(struct nfs_cache_array *array, nfs_readdir_des
}
if (array->eof_index >= 0) {
status = -EBADCOOKIE;
- if (*desc->dir_cookie == array->last_cookie)
+ if (desc->dir_cookie == array->last_cookie)
desc->eof = true;
}
out:
@@ -349,7 +351,7 @@ int nfs_readdir_search_array(nfs_readdir_descriptor_t *desc)
array = kmap(desc->page);
- if (*desc->dir_cookie == 0)
+ if (desc->dir_cookie == 0)
status = nfs_readdir_search_for_pos(array, desc);
else
status = nfs_readdir_search_for_cookie(array, desc);
@@ -801,7 +803,6 @@ int nfs_do_filldir(nfs_readdir_descriptor_t *desc)
int i = 0;
int res = 0;
struct nfs_cache_array *array = NULL;
- struct nfs_open_dir_context *ctx = file->private_data;
array = kmap(desc->page);
for (i = desc->cache_entry_index; i < array->size; i++) {
@@ -814,22 +815,22 @@ int nfs_do_filldir(nfs_readdir_descriptor_t *desc)
break;
}
if (i < (array->size-1))
- *desc->dir_cookie = array->array[i+1].cookie;
+ desc->dir_cookie = array->array[i+1].cookie;
else
- *desc->dir_cookie = array->last_cookie;
+ desc->dir_cookie = array->last_cookie;
if (nfs_readdir_use_cookie(file))
- desc->ctx->pos = *desc->dir_cookie;
+ desc->ctx->pos = desc->dir_cookie;
else
desc->ctx->pos++;
- if (ctx->duped != 0)
- ctx->duped = 1;
+ if (desc->duped != 0)
+ desc->duped = 1;
}
if (array->eof_index >= 0)
desc->eof = true;
kunmap(desc->page);
dfprintk(DIRCACHE, "NFS: nfs_do_filldir() filling ended @ cookie %Lu; returning = %d\n",
- (unsigned long long)*desc->dir_cookie, res);
+ (unsigned long long)desc->dir_cookie, res);
return res;
}
@@ -851,10 +852,9 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc)
struct page *page = NULL;
int status;
struct inode *inode = file_inode(desc->file);
- struct nfs_open_dir_context *ctx = desc->file->private_data;
dfprintk(DIRCACHE, "NFS: uncached_readdir() searching for cookie %Lu\n",
- (unsigned long long)*desc->dir_cookie);
+ (unsigned long long)desc->dir_cookie);
page = alloc_page(GFP_HIGHUSER);
if (!page) {
@@ -863,9 +863,9 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc)
}
desc->page_index = 0;
- desc->last_cookie = *desc->dir_cookie;
+ desc->last_cookie = desc->dir_cookie;
desc->page = page;
- ctx->duped = 0;
+ desc->duped = 0;
status = nfs_readdir_xdr_to_array(desc, page, inode);
if (status < 0)
@@ -894,7 +894,6 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
nfs_readdir_descriptor_t my_desc = {
.file = file,
.ctx = ctx,
- .dir_cookie = &dir_ctx->dir_cookie,
.plus = nfs_use_readdirplus(inode, ctx),
},
*desc = &my_desc;
@@ -915,13 +914,20 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
if (res < 0)
goto out;
+ spin_lock(&file->f_lock);
+ desc->dir_cookie = dir_ctx->dir_cookie;
+ desc->dup_cookie = dir_ctx->dup_cookie;
+ desc->duped = dir_ctx->duped;
+ desc->attr_gencount = dir_ctx->attr_gencount;
+ spin_unlock(&file->f_lock);
+
do {
res = readdir_search_pagecache(desc);
if (res == -EBADCOOKIE) {
res = 0;
/* This means either end of directory */
- if (*desc->dir_cookie && !desc->eof) {
+ if (desc->dir_cookie && !desc->eof) {
/* Or that the server has 'lost' a cookie */
res = uncached_readdir(desc);
if (res == 0)
@@ -946,6 +952,14 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
if (res < 0)
break;
} while (!desc->eof);
+
+ spin_lock(&file->f_lock);
+ dir_ctx->dir_cookie = desc->dir_cookie;
+ dir_ctx->dup_cookie = desc->dup_cookie;
+ dir_ctx->duped = desc->duped;
+ dir_ctx->attr_gencount = desc->attr_gencount;
+ spin_unlock(&file->f_lock);
+
out:
if (res > 0)
res = 0;
--
2.28.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 02/12] NFS: Clean up readdir struct nfs_cache_array
2020-11-02 18:06 ` [PATCH 01/12] NFS: Ensure contents of struct nfs_open_dir_context are consistent trondmy
@ 2020-11-02 18:06 ` trondmy
2020-11-02 18:06 ` [PATCH 03/12] NFS: Clean up nfs_readdir_page_filler() trondmy
2020-11-03 13:35 ` [PATCH 02/12] NFS: Clean up readdir struct nfs_cache_array Benjamin Coddington
0 siblings, 2 replies; 20+ messages in thread
From: trondmy @ 2020-11-02 18:06 UTC (permalink / raw)
To: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
Since the 'eof_index' is only ever used as a flag, make it so.
Also add a flag to detect if the page has been completely filled.
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/dir.c | 66 ++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 49 insertions(+), 17 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 67d8595cd6e5..604ebe015387 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -138,9 +138,10 @@ struct nfs_cache_array_entry {
};
struct nfs_cache_array {
- int size;
- int eof_index;
u64 last_cookie;
+ unsigned int size;
+ unsigned char page_full : 1,
+ page_is_eof : 1;
struct nfs_cache_array_entry array[];
};
@@ -172,7 +173,6 @@ void nfs_readdir_init_array(struct page *page)
array = kmap_atomic(page);
memset(array, 0, sizeof(struct nfs_cache_array));
- array->eof_index = -1;
kunmap_atomic(array);
}
@@ -192,6 +192,17 @@ void nfs_readdir_clear_array(struct page *page)
kunmap_atomic(array);
}
+static void nfs_readdir_array_set_eof(struct nfs_cache_array *array)
+{
+ array->page_is_eof = 1;
+ array->page_full = 1;
+}
+
+static bool nfs_readdir_array_is_full(struct nfs_cache_array *array)
+{
+ return array->page_full;
+}
+
/*
* the caller is responsible for freeing qstr.name
* when called by nfs_readdir_add_to_array, the strings will be freed in
@@ -213,6 +224,23 @@ int nfs_readdir_make_qstr(struct qstr *string, const char *name, unsigned int le
return 0;
}
+/*
+ * Check that the next array entry lies entirely within the page bounds
+ */
+static int nfs_readdir_array_can_expand(struct nfs_cache_array *array)
+{
+ struct nfs_cache_array_entry *cache_entry;
+
+ if (array->page_full)
+ return -ENOSPC;
+ cache_entry = &array->array[array->size + 1];
+ if ((char *)cache_entry - (char *)array > PAGE_SIZE) {
+ array->page_full = 1;
+ return -ENOSPC;
+ }
+ return 0;
+}
+
static
int nfs_readdir_add_to_array(struct nfs_entry *entry, struct page *page)
{
@@ -220,13 +248,11 @@ int nfs_readdir_add_to_array(struct nfs_entry *entry, struct page *page)
struct nfs_cache_array_entry *cache_entry;
int ret;
- cache_entry = &array->array[array->size];
-
- /* Check that this entry lies within the page bounds */
- ret = -ENOSPC;
- if ((char *)&cache_entry[1] - (char *)page_address(page) > PAGE_SIZE)
+ ret = nfs_readdir_array_can_expand(array);
+ if (ret)
goto out;
+ cache_entry = &array->array[array->size];
cache_entry->cookie = entry->prev_cookie;
cache_entry->ino = entry->ino;
cache_entry->d_type = entry->d_type;
@@ -236,12 +262,21 @@ int nfs_readdir_add_to_array(struct nfs_entry *entry, struct page *page)
array->last_cookie = entry->cookie;
array->size++;
if (entry->eof != 0)
- array->eof_index = array->size;
+ nfs_readdir_array_set_eof(array);
out:
kunmap(page);
return ret;
}
+static void nfs_readdir_page_set_eof(struct page *page)
+{
+ struct nfs_cache_array *array;
+
+ array = kmap_atomic(page);
+ nfs_readdir_array_set_eof(array);
+ kunmap_atomic(array);
+}
+
static inline
int is_32bit_api(void)
{
@@ -270,7 +305,7 @@ int nfs_readdir_search_for_pos(struct nfs_cache_array *array, nfs_readdir_descri
if (diff < 0)
goto out_eof;
if (diff >= array->size) {
- if (array->eof_index >= 0)
+ if (array->page_is_eof)
goto out_eof;
return -EAGAIN;
}
@@ -334,7 +369,7 @@ int nfs_readdir_search_for_cookie(struct nfs_cache_array *array, nfs_readdir_des
return 0;
}
}
- if (array->eof_index >= 0) {
+ if (array->page_is_eof) {
status = -EBADCOOKIE;
if (desc->dir_cookie == array->last_cookie)
desc->eof = true;
@@ -566,7 +601,6 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct nfs_entry *en
struct xdr_stream stream;
struct xdr_buf buf;
struct page *scratch;
- struct nfs_cache_array *array;
unsigned int count = 0;
int status;
@@ -604,10 +638,8 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct nfs_entry *en
out_nopages:
if (count == 0 || (status == -EBADCOOKIE && entry->eof != 0)) {
- array = kmap(page);
- array->eof_index = array->size;
+ nfs_readdir_page_set_eof(page);
status = 0;
- kunmap(page);
}
put_page(scratch);
@@ -689,7 +721,7 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
status = 0;
break;
}
- } while (array->eof_index < 0);
+ } while (!nfs_readdir_array_is_full(array));
nfs_readdir_free_pages(pages, array_size);
out_release_array:
@@ -825,7 +857,7 @@ int nfs_do_filldir(nfs_readdir_descriptor_t *desc)
if (desc->duped != 0)
desc->duped = 1;
}
- if (array->eof_index >= 0)
+ if (array->page_is_eof)
desc->eof = true;
kunmap(desc->page);
--
2.28.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 03/12] NFS: Clean up nfs_readdir_page_filler()
2020-11-02 18:06 ` [PATCH 02/12] NFS: Clean up readdir struct nfs_cache_array trondmy
@ 2020-11-02 18:06 ` trondmy
2020-11-02 18:06 ` [PATCH 04/12] NFS: Clean up directory array handling trondmy
2020-11-03 14:18 ` [PATCH 03/12] NFS: Clean up nfs_readdir_page_filler() Benjamin Coddington
2020-11-03 13:35 ` [PATCH 02/12] NFS: Clean up readdir struct nfs_cache_array Benjamin Coddington
1 sibling, 2 replies; 20+ messages in thread
From: trondmy @ 2020-11-02 18:06 UTC (permalink / raw)
To: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
Clean up handling of the case where there are no entries in the readdir
reply.
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/dir.c | 39 ++++++++++++++++++---------------------
1 file changed, 18 insertions(+), 21 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 604ebe015387..68acbde3f914 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -601,16 +601,12 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct nfs_entry *en
struct xdr_stream stream;
struct xdr_buf buf;
struct page *scratch;
- unsigned int count = 0;
int status;
scratch = alloc_page(GFP_KERNEL);
if (scratch == NULL)
return -ENOMEM;
- if (buflen == 0)
- goto out_nopages;
-
xdr_init_decode_pages(&stream, &buf, xdr_pages, buflen);
xdr_set_scratch_buffer(&stream, page_address(scratch), PAGE_SIZE);
@@ -619,27 +615,27 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct nfs_entry *en
entry->label->len = NFS4_MAXLABELLEN;
status = xdr_decode(desc, entry, &stream);
- if (status != 0) {
- if (status == -EAGAIN)
- status = 0;
+ if (status != 0)
break;
- }
-
- count++;
if (desc->plus)
nfs_prime_dcache(file_dentry(desc->file), entry,
desc->dir_verifier);
status = nfs_readdir_add_to_array(entry, page);
- if (status != 0)
- break;
- } while (!entry->eof);
+ } while (!status && !entry->eof);
-out_nopages:
- if (count == 0 || (status == -EBADCOOKIE && entry->eof != 0)) {
- nfs_readdir_page_set_eof(page);
+ switch (status) {
+ case -EBADCOOKIE:
+ if (entry->eof) {
+ nfs_readdir_page_set_eof(page);
+ status = 0;
+ }
+ break;
+ case -ENOSPC:
+ case -EAGAIN:
status = 0;
+ break;
}
put_page(scratch);
@@ -714,14 +710,15 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
if (status < 0)
break;
+
pglen = status;
- status = nfs_readdir_page_filler(desc, &entry, pages, page, pglen);
- if (status < 0) {
- if (status == -ENOSPC)
- status = 0;
+ if (pglen == 0) {
+ nfs_readdir_page_set_eof(page);
break;
}
- } while (!nfs_readdir_array_is_full(array));
+
+ status = nfs_readdir_page_filler(desc, &entry, pages, page, pglen);
+ } while (!status && !nfs_readdir_array_is_full(array));
nfs_readdir_free_pages(pages, array_size);
out_release_array:
--
2.28.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 04/12] NFS: Clean up directory array handling
2020-11-02 18:06 ` [PATCH 03/12] NFS: Clean up nfs_readdir_page_filler() trondmy
@ 2020-11-02 18:06 ` trondmy
2020-11-02 18:06 ` [PATCH 05/12] NFS: Don't discard readdir results trondmy
2020-11-03 14:18 ` [PATCH 03/12] NFS: Clean up nfs_readdir_page_filler() Benjamin Coddington
1 sibling, 1 reply; 20+ messages in thread
From: trondmy @ 2020-11-02 18:06 UTC (permalink / raw)
To: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
Refactor to use pagecache_get_page() so that we can fill the page
in multiple stages.
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/dir.c | 134 ++++++++++++++++++++++++++++-----------------------
1 file changed, 73 insertions(+), 61 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 68acbde3f914..89cd8d5d9d3e 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -149,7 +149,7 @@ typedef struct nfs_readdir_descriptor {
struct file *file;
struct page *page;
struct dir_context *ctx;
- unsigned long page_index;
+ pgoff_t page_index;
u64 dir_cookie;
u64 last_cookie;
u64 dup_cookie;
@@ -166,13 +166,18 @@ typedef struct nfs_readdir_descriptor {
bool eof;
} nfs_readdir_descriptor_t;
-static
-void nfs_readdir_init_array(struct page *page)
+static void nfs_readdir_array_init(struct nfs_cache_array *array)
+{
+ memset(array, 0, sizeof(struct nfs_cache_array));
+}
+
+static void nfs_readdir_page_init_array(struct page *page, u64 last_cookie)
{
struct nfs_cache_array *array;
array = kmap_atomic(page);
- memset(array, 0, sizeof(struct nfs_cache_array));
+ nfs_readdir_array_init(array);
+ array->last_cookie = last_cookie;
kunmap_atomic(array);
}
@@ -188,7 +193,7 @@ void nfs_readdir_clear_array(struct page *page)
array = kmap_atomic(page);
for (i = 0; i < array->size; i++)
kfree(array->array[i].string.name);
- array->size = 0;
+ nfs_readdir_array_init(array);
kunmap_atomic(array);
}
@@ -268,6 +273,45 @@ int nfs_readdir_add_to_array(struct nfs_entry *entry, struct page *page)
return ret;
}
+static struct page *nfs_readdir_page_get_locked(struct address_space *mapping,
+ pgoff_t index, u64 last_cookie)
+{
+ struct page *page;
+
+ page = pagecache_get_page(mapping, index, FGP_LOCK|FGP_WRITE|FGP_CREAT,
+ mapping_gfp_mask(mapping));
+ if (page && !PageUptodate(page)) {
+ nfs_readdir_page_init_array(page, last_cookie);
+ if (invalidate_inode_pages2_range(mapping, index + 1, -1) < 0)
+ nfs_zap_mapping(mapping->host, mapping);
+ SetPageUptodate(page);
+ }
+
+ return page;
+}
+
+static u64 nfs_readdir_page_last_cookie(struct page *page)
+{
+ struct nfs_cache_array *array;
+ u64 ret;
+
+ array = kmap_atomic(page);
+ ret = array->last_cookie;
+ kunmap_atomic(array);
+ return ret;
+}
+
+static bool nfs_readdir_page_needs_filling(struct page *page)
+{
+ struct nfs_cache_array *array;
+ bool ret;
+
+ array = kmap_atomic(page);
+ ret = !nfs_readdir_array_is_full(array);
+ kunmap_atomic(array);
+ return ret;
+}
+
static void nfs_readdir_page_set_eof(struct page *page)
{
struct nfs_cache_array *array;
@@ -682,10 +726,8 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
int status = -ENOMEM;
unsigned int array_size = ARRAY_SIZE(pages);
- nfs_readdir_init_array(page);
-
entry.prev_cookie = 0;
- entry.cookie = desc->last_cookie;
+ entry.cookie = nfs_readdir_page_last_cookie(page);
entry.eof = 0;
entry.fh = nfs_alloc_fhandle();
entry.fattr = nfs_alloc_fattr();
@@ -730,48 +772,18 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
return status;
}
-/*
- * Now we cache directories properly, by converting xdr information
- * to an array that can be used for lookups later. This results in
- * fewer cache pages, since we can store more information on each page.
- * We only need to convert from xdr once so future lookups are much simpler
- */
-static
-int nfs_readdir_filler(void *data, struct page* page)
-{
- nfs_readdir_descriptor_t *desc = data;
- struct inode *inode = file_inode(desc->file);
- int ret;
-
- ret = nfs_readdir_xdr_to_array(desc, page, inode);
- if (ret < 0)
- goto error;
- SetPageUptodate(page);
-
- if (invalidate_inode_pages2_range(inode->i_mapping, page->index + 1, -1) < 0) {
- /* Should never happen */
- nfs_zap_mapping(inode, inode->i_mapping);
- }
- unlock_page(page);
- return 0;
- error:
- nfs_readdir_clear_array(page);
- unlock_page(page);
- return ret;
-}
-
-static
-void cache_page_release(nfs_readdir_descriptor_t *desc)
+static void nfs_readdir_page_put(struct nfs_readdir_descriptor *desc)
{
put_page(desc->page);
desc->page = NULL;
}
-static
-struct page *get_cache_page(nfs_readdir_descriptor_t *desc)
+static struct page *
+nfs_readdir_page_get_cached(struct nfs_readdir_descriptor *desc)
{
- return read_cache_page(desc->file->f_mapping, desc->page_index,
- nfs_readdir_filler, desc);
+ return nfs_readdir_page_get_locked(desc->file->f_mapping,
+ desc->page_index,
+ desc->last_cookie);
}
/*
@@ -785,23 +797,22 @@ int find_and_lock_cache_page(nfs_readdir_descriptor_t *desc)
struct nfs_inode *nfsi = NFS_I(inode);
int res;
- desc->page = get_cache_page(desc);
- if (IS_ERR(desc->page))
- return PTR_ERR(desc->page);
- res = lock_page_killable(desc->page);
- if (res != 0)
- goto error;
- res = -EAGAIN;
- if (desc->page->mapping != NULL) {
- res = nfs_readdir_search_array(desc);
- if (res == 0) {
- nfsi->page_index = desc->page_index;
- return 0;
- }
+ desc->page = nfs_readdir_page_get_cached(desc);
+ if (!desc->page)
+ return -ENOMEM;
+ if (nfs_readdir_page_needs_filling(desc->page)) {
+ res = nfs_readdir_xdr_to_array(desc, desc->page, inode);
+ if (res < 0)
+ goto error;
+ }
+ res = nfs_readdir_search_array(desc);
+ if (res == 0) {
+ nfsi->page_index = desc->page_index;
+ return 0;
}
- unlock_page(desc->page);
error:
- cache_page_release(desc);
+ unlock_page(desc->page);
+ nfs_readdir_page_put(desc);
return res;
}
@@ -896,6 +907,7 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc)
desc->page = page;
desc->duped = 0;
+ nfs_readdir_page_init_array(page, desc->dir_cookie);
status = nfs_readdir_xdr_to_array(desc, page, inode);
if (status < 0)
goto out_release;
@@ -904,7 +916,7 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc)
out_release:
nfs_readdir_clear_array(desc->page);
- cache_page_release(desc);
+ nfs_readdir_page_put(desc);
out:
dfprintk(DIRCACHE, "NFS: %s: returns %d\n",
__func__, status);
@@ -977,7 +989,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
res = nfs_do_filldir(desc);
unlock_page(desc->page);
- cache_page_release(desc);
+ nfs_readdir_page_put(desc);
if (res < 0)
break;
} while (!desc->eof);
--
2.28.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 05/12] NFS: Don't discard readdir results
2020-11-02 18:06 ` [PATCH 04/12] NFS: Clean up directory array handling trondmy
@ 2020-11-02 18:06 ` trondmy
2020-11-02 18:06 ` [PATCH 06/12] NFS: Remove unnecessary kmap in nfs_readdir_xdr_to_array() trondmy
0 siblings, 1 reply; 20+ messages in thread
From: trondmy @ 2020-11-02 18:06 UTC (permalink / raw)
To: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
If a readdir call returns more data than we can fit into one page
cache page, then allocate a new one for that data rather than
discarding the data.
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/dir.c | 45 +++++++++++++++++++++++++++++++++++++++++----
1 file changed, 41 insertions(+), 4 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 89cd8d5d9d3e..6f84038735a1 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -321,6 +321,21 @@ static void nfs_readdir_page_set_eof(struct page *page)
kunmap_atomic(array);
}
+static struct page *nfs_readdir_page_get_next(struct address_space *mapping,
+ pgoff_t index, u64 cookie)
+{
+ struct page *page;
+
+ page = nfs_readdir_page_get_locked(mapping, index, cookie);
+ if (page) {
+ if (nfs_readdir_page_last_cookie(page) == cookie)
+ return page;
+ unlock_page(page);
+ put_page(page);
+ }
+ return NULL;
+}
+
static inline
int is_32bit_api(void)
{
@@ -638,13 +653,15 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry,
}
/* Perform conversion from xdr to cache array */
-static
-int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct nfs_entry *entry,
- struct page **xdr_pages, struct page *page, unsigned int buflen)
+static int nfs_readdir_page_filler(struct nfs_readdir_descriptor *desc,
+ struct nfs_entry *entry,
+ struct page **xdr_pages,
+ struct page *fillme, unsigned int buflen)
{
+ struct address_space *mapping = desc->file->f_mapping;
struct xdr_stream stream;
struct xdr_buf buf;
- struct page *scratch;
+ struct page *scratch, *new, *page = fillme;
int status;
scratch = alloc_page(GFP_KERNEL);
@@ -667,6 +684,21 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct nfs_entry *en
desc->dir_verifier);
status = nfs_readdir_add_to_array(entry, page);
+ if (status != -ENOSPC)
+ continue;
+
+ if (page->mapping != mapping)
+ break;
+ new = nfs_readdir_page_get_next(mapping, page->index + 1,
+ entry->prev_cookie);
+ if (!new)
+ break;
+ if (page != fillme) {
+ unlock_page(page);
+ put_page(page);
+ }
+ page = new;
+ status = nfs_readdir_add_to_array(entry, page);
} while (!status && !entry->eof);
switch (status) {
@@ -682,6 +714,11 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct nfs_entry *en
break;
}
+ if (page != fillme) {
+ unlock_page(page);
+ put_page(page);
+ }
+
put_page(scratch);
return status;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 06/12] NFS: Remove unnecessary kmap in nfs_readdir_xdr_to_array()
2020-11-02 18:06 ` [PATCH 05/12] NFS: Don't discard readdir results trondmy
@ 2020-11-02 18:06 ` trondmy
2020-11-02 18:06 ` [PATCH 07/12] NFS: Replace kmap() with kmap_atomic() in nfs_readdir_search_array() trondmy
0 siblings, 1 reply; 20+ messages in thread
From: trondmy @ 2020-11-02 18:06 UTC (permalink / raw)
To: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
The kmapped pointer is only used once per loop to check if we need to
exit.
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/dir.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 6f84038735a1..560a9c1553e5 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -759,7 +759,6 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
struct page *pages[NFS_MAX_READDIR_PAGES];
struct nfs_entry entry;
struct file *file = desc->file;
- struct nfs_cache_array *array;
int status = -ENOMEM;
unsigned int array_size = ARRAY_SIZE(pages);
@@ -778,11 +777,9 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
goto out;
}
- array = kmap(page);
-
status = nfs_readdir_alloc_pages(pages, array_size);
if (status < 0)
- goto out_release_array;
+ goto out_release_label;
do {
unsigned int pglen;
status = nfs_readdir_xdr_filler(pages, desc, &entry, file, inode);
@@ -797,11 +794,10 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
}
status = nfs_readdir_page_filler(desc, &entry, pages, page, pglen);
- } while (!status && !nfs_readdir_array_is_full(array));
+ } while (!status && nfs_readdir_page_needs_filling(page));
nfs_readdir_free_pages(pages, array_size);
-out_release_array:
- kunmap(page);
+out_release_label:
nfs4_label_free(entry.label);
out:
nfs_free_fattr(entry.fattr);
--
2.28.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 07/12] NFS: Replace kmap() with kmap_atomic() in nfs_readdir_search_array()
2020-11-02 18:06 ` [PATCH 06/12] NFS: Remove unnecessary kmap in nfs_readdir_xdr_to_array() trondmy
@ 2020-11-02 18:06 ` trondmy
2020-11-02 18:06 ` [PATCH 08/12] NFS: Simplify struct nfs_cache_array_entry trondmy
0 siblings, 1 reply; 20+ messages in thread
From: trondmy @ 2020-11-02 18:06 UTC (permalink / raw)
To: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/dir.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 560a9c1553e5..a91f9ea87611 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -443,7 +443,7 @@ int nfs_readdir_search_array(nfs_readdir_descriptor_t *desc)
struct nfs_cache_array *array;
int status;
- array = kmap(desc->page);
+ array = kmap_atomic(desc->page);
if (desc->dir_cookie == 0)
status = nfs_readdir_search_for_pos(array, desc);
@@ -455,7 +455,7 @@ int nfs_readdir_search_array(nfs_readdir_descriptor_t *desc)
desc->current_index += array->size;
desc->page_index++;
}
- kunmap(desc->page);
+ kunmap_atomic(array);
return status;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 08/12] NFS: Simplify struct nfs_cache_array_entry
2020-11-02 18:06 ` [PATCH 07/12] NFS: Replace kmap() with kmap_atomic() in nfs_readdir_search_array() trondmy
@ 2020-11-02 18:06 ` trondmy
2020-11-02 18:06 ` [PATCH 09/12] NFS: Support larger readdir buffers trondmy
0 siblings, 1 reply; 20+ messages in thread
From: trondmy @ 2020-11-02 18:06 UTC (permalink / raw)
To: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
We don't need to store a hash, so replace struct qstr with a simple
const char pointer and length.
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/dir.c | 46 +++++++++++++++++++++++++---------------------
1 file changed, 25 insertions(+), 21 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index a91f9ea87611..c82f1bde4f13 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -133,7 +133,8 @@ nfs_closedir(struct inode *inode, struct file *filp)
struct nfs_cache_array_entry {
u64 cookie;
u64 ino;
- struct qstr string;
+ const char *name;
+ unsigned int name_len;
unsigned char d_type;
};
@@ -192,7 +193,7 @@ void nfs_readdir_clear_array(struct page *page)
array = kmap_atomic(page);
for (i = 0; i < array->size; i++)
- kfree(array->array[i].string.name);
+ kfree(array->array[i].name);
nfs_readdir_array_init(array);
kunmap_atomic(array);
}
@@ -213,20 +214,17 @@ static bool nfs_readdir_array_is_full(struct nfs_cache_array *array)
* when called by nfs_readdir_add_to_array, the strings will be freed in
* nfs_clear_readdir_array()
*/
-static
-int nfs_readdir_make_qstr(struct qstr *string, const char *name, unsigned int len)
+static const char *nfs_readdir_copy_name(const char *name, unsigned int len)
{
- string->len = len;
- string->name = kmemdup_nul(name, len, GFP_KERNEL);
- if (string->name == NULL)
- return -ENOMEM;
+ const char *ret = kmemdup_nul(name, len, GFP_KERNEL);
+
/*
* Avoid a kmemleak false positive. The pointer to the name is stored
* in a page cache page which kmemleak does not scan.
*/
- kmemleak_not_leak(string->name);
- string->hash = full_name_hash(NULL, name, len);
- return 0;
+ if (ret != NULL)
+ kmemleak_not_leak(ret);
+ return ret;
}
/*
@@ -249,27 +247,34 @@ static int nfs_readdir_array_can_expand(struct nfs_cache_array *array)
static
int nfs_readdir_add_to_array(struct nfs_entry *entry, struct page *page)
{
- struct nfs_cache_array *array = kmap(page);
+ struct nfs_cache_array *array;
struct nfs_cache_array_entry *cache_entry;
+ const char *name;
int ret;
+ name = nfs_readdir_copy_name(entry->name, entry->len);
+ if (!name)
+ return -ENOMEM;
+
+ array = kmap_atomic(page);
ret = nfs_readdir_array_can_expand(array);
- if (ret)
+ if (ret) {
+ kfree(name);
goto out;
+ }
cache_entry = &array->array[array->size];
cache_entry->cookie = entry->prev_cookie;
cache_entry->ino = entry->ino;
cache_entry->d_type = entry->d_type;
- ret = nfs_readdir_make_qstr(&cache_entry->string, entry->name, entry->len);
- if (ret)
- goto out;
+ cache_entry->name_len = entry->len;
+ cache_entry->name = name;
array->last_cookie = entry->cookie;
array->size++;
if (entry->eof != 0)
nfs_readdir_array_set_eof(array);
out:
- kunmap(page);
+ kunmap_atomic(array);
return ret;
}
@@ -409,9 +414,8 @@ int nfs_readdir_search_for_cookie(struct nfs_cache_array *array, nfs_readdir_des
if (printk_ratelimit()) {
pr_notice("NFS: directory %pD2 contains a readdir loop."
"Please contact your server vendor. "
- "The file: %.*s has duplicate cookie %llu\n",
- desc->file, array->array[i].string.len,
- array->array[i].string.name, desc->dir_cookie);
+ "The file: %s has duplicate cookie %llu\n",
+ desc->file, array->array[i].name, desc->dir_cookie);
}
status = -ELOOP;
goto out;
@@ -882,7 +886,7 @@ int nfs_do_filldir(nfs_readdir_descriptor_t *desc)
struct nfs_cache_array_entry *ent;
ent = &array->array[i];
- if (!dir_emit(desc->ctx, ent->string.name, ent->string.len,
+ if (!dir_emit(desc->ctx, ent->name, ent->name_len,
nfs_compat_user_ino64(ent->ino), ent->d_type)) {
desc->eof = true;
break;
--
2.28.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 09/12] NFS: Support larger readdir buffers
2020-11-02 18:06 ` [PATCH 08/12] NFS: Simplify struct nfs_cache_array_entry trondmy
@ 2020-11-02 18:06 ` trondmy
2020-11-02 18:06 ` [PATCH 10/12] NFS: More readdir cleanups trondmy
0 siblings, 1 reply; 20+ messages in thread
From: trondmy @ 2020-11-02 18:06 UTC (permalink / raw)
To: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
Support readdir buffers of up to 1MB in size so that we can read
large directories using few RPC calls.
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/client.c | 4 ++--
fs/nfs/dir.c | 33 +++++++++++++++++++--------------
fs/nfs/internal.h | 6 ------
3 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 4b8cc93913f7..f6454ba53d05 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -781,8 +781,8 @@ static void nfs_server_set_fsinfo(struct nfs_server *server,
server->wtmult = nfs_block_bits(fsinfo->wtmult, NULL);
server->dtsize = nfs_block_size(fsinfo->dtpref, NULL);
- if (server->dtsize > PAGE_SIZE * NFS_MAX_READDIR_PAGES)
- server->dtsize = PAGE_SIZE * NFS_MAX_READDIR_PAGES;
+ if (server->dtsize > NFS_MAX_FILE_IO_SIZE)
+ server->dtsize = NFS_MAX_FILE_IO_SIZE;
if (server->dtsize > server->rsize)
server->dtsize = server->rsize;
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index c82f1bde4f13..272b856147af 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -727,44 +727,47 @@ static int nfs_readdir_page_filler(struct nfs_readdir_descriptor *desc,
return status;
}
-static
-void nfs_readdir_free_pages(struct page **pages, unsigned int npages)
+static void nfs_readdir_free_pages(struct page **pages, size_t npages)
{
- unsigned int i;
- for (i = 0; i < npages; i++)
- put_page(pages[i]);
+ while (npages--)
+ put_page(pages[npages]);
+ kfree(pages);
}
/*
* nfs_readdir_alloc_pages() will allocate pages that must be freed with a call
* to nfs_readdir_free_pages()
*/
-static
-int nfs_readdir_alloc_pages(struct page **pages, unsigned int npages)
+static struct page **nfs_readdir_alloc_pages(size_t npages)
{
- unsigned int i;
+ struct page **pages;
+ size_t i;
+ pages = kmalloc_array(npages, sizeof(*pages), GFP_KERNEL);
+ if (!pages)
+ return NULL;
for (i = 0; i < npages; i++) {
struct page *page = alloc_page(GFP_KERNEL);
if (page == NULL)
goto out_freepages;
pages[i] = page;
}
- return 0;
+ return pages;
out_freepages:
nfs_readdir_free_pages(pages, i);
- return -ENOMEM;
+ return NULL;
}
static
int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page, struct inode *inode)
{
- struct page *pages[NFS_MAX_READDIR_PAGES];
+ struct page **pages;
struct nfs_entry entry;
struct file *file = desc->file;
+ size_t array_size;
+ size_t dtsize = NFS_SERVER(inode)->dtsize;
int status = -ENOMEM;
- unsigned int array_size = ARRAY_SIZE(pages);
entry.prev_cookie = 0;
entry.cookie = nfs_readdir_page_last_cookie(page);
@@ -781,9 +784,11 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
goto out;
}
- status = nfs_readdir_alloc_pages(pages, array_size);
- if (status < 0)
+ array_size = (dtsize + PAGE_SIZE - 1) >> PAGE_SHIFT;
+ pages = nfs_readdir_alloc_pages(array_size);
+ if (!pages)
goto out_release_label;
+
do {
unsigned int pglen;
status = nfs_readdir_xdr_filler(pages, desc, &entry, file, inode);
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 6673a77884d9..b840d0a91c9d 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -56,12 +56,6 @@ static inline bool nfs_lookup_is_soft_revalidate(const struct dentry *dentry)
#define NFS_UNSPEC_RETRANS (UINT_MAX)
#define NFS_UNSPEC_TIMEO (UINT_MAX)
-/*
- * Maximum number of pages that readdir can use for creating
- * a vmapped array of pages.
- */
-#define NFS_MAX_READDIR_PAGES 8
-
struct nfs_client_initdata {
unsigned long init_flags;
const char *hostname; /* Hostname of the server */
--
2.28.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 10/12] NFS: More readdir cleanups
2020-11-02 18:06 ` [PATCH 09/12] NFS: Support larger readdir buffers trondmy
@ 2020-11-02 18:06 ` trondmy
2020-11-02 18:06 ` [PATCH 11/12] NFS: nfs_do_filldir() does not return a value trondmy
0 siblings, 1 reply; 20+ messages in thread
From: trondmy @ 2020-11-02 18:06 UTC (permalink / raw)
To: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
Remove the redundant caching of the credential in struct
nfs_open_dir_context.
Pass the buffer size as an argument to nfs_readdir_xdr_filler().
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/dir.c | 25 +++++++++++--------------
include/linux/nfs_fs.h | 1 -
2 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 272b856147af..564e737ef93e 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -68,7 +68,7 @@ const struct address_space_operations nfs_dir_aops = {
.freepage = nfs_readdir_clear_array,
};
-static struct nfs_open_dir_context *alloc_nfs_open_dir_context(struct inode *dir, const struct cred *cred)
+static struct nfs_open_dir_context *alloc_nfs_open_dir_context(struct inode *dir)
{
struct nfs_inode *nfsi = NFS_I(dir);
struct nfs_open_dir_context *ctx;
@@ -78,7 +78,6 @@ static struct nfs_open_dir_context *alloc_nfs_open_dir_context(struct inode *dir
ctx->attr_gencount = nfsi->attr_gencount;
ctx->dir_cookie = 0;
ctx->dup_cookie = 0;
- ctx->cred = get_cred(cred);
spin_lock(&dir->i_lock);
if (list_empty(&nfsi->open_files) &&
(nfsi->cache_validity & NFS_INO_DATA_INVAL_DEFER))
@@ -96,7 +95,6 @@ static void put_nfs_open_dir_context(struct inode *dir, struct nfs_open_dir_cont
spin_lock(&dir->i_lock);
list_del(&ctx->list);
spin_unlock(&dir->i_lock);
- put_cred(ctx->cred);
kfree(ctx);
}
@@ -113,7 +111,7 @@ nfs_opendir(struct inode *inode, struct file *filp)
nfs_inc_stats(inode, NFSIOS_VFSOPEN);
- ctx = alloc_nfs_open_dir_context(inode, current_cred());
+ ctx = alloc_nfs_open_dir_context(inode);
if (IS_ERR(ctx)) {
res = PTR_ERR(ctx);
goto out;
@@ -464,12 +462,12 @@ int nfs_readdir_search_array(nfs_readdir_descriptor_t *desc)
}
/* Fill a page with xdr information before transferring to the cache page */
-static
-int nfs_readdir_xdr_filler(struct page **pages, nfs_readdir_descriptor_t *desc,
- struct nfs_entry *entry, struct file *file, struct inode *inode)
+static int nfs_readdir_xdr_filler(struct nfs_readdir_descriptor *desc,
+ u64 cookie, struct page **pages,
+ size_t bufsize)
{
- struct nfs_open_dir_context *ctx = file->private_data;
- const struct cred *cred = ctx->cred;
+ struct file *file = desc->file;
+ struct inode *inode = file_inode(file);
unsigned long timestamp, gencount;
int error;
@@ -477,8 +475,8 @@ int nfs_readdir_xdr_filler(struct page **pages, nfs_readdir_descriptor_t *desc,
timestamp = jiffies;
gencount = nfs_inc_attr_generation_counter();
desc->dir_verifier = nfs_save_change_attribute(inode);
- error = NFS_PROTO(inode)->readdir(file_dentry(file), cred, entry->cookie, pages,
- NFS_SERVER(inode)->dtsize, desc->plus);
+ error = NFS_PROTO(inode)->readdir(file_dentry(file), file->f_cred,
+ cookie, pages, bufsize, desc->plus);
if (error < 0) {
/* We requested READDIRPLUS, but the server doesn't grok it */
if (error == -ENOTSUPP && desc->plus) {
@@ -764,7 +762,6 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
{
struct page **pages;
struct nfs_entry entry;
- struct file *file = desc->file;
size_t array_size;
size_t dtsize = NFS_SERVER(inode)->dtsize;
int status = -ENOMEM;
@@ -791,8 +788,8 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
do {
unsigned int pglen;
- status = nfs_readdir_xdr_filler(pages, desc, &entry, file, inode);
-
+ status = nfs_readdir_xdr_filler(desc, entry.cookie,
+ pages, dtsize);
if (status < 0)
break;
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index a2c6455ea3fa..dd6b463dda80 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -88,7 +88,6 @@ struct nfs_open_context {
struct nfs_open_dir_context {
struct list_head list;
- const struct cred *cred;
unsigned long attr_gencount;
__u64 dir_cookie;
__u64 dup_cookie;
--
2.28.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 11/12] NFS: nfs_do_filldir() does not return a value
2020-11-02 18:06 ` [PATCH 10/12] NFS: More readdir cleanups trondmy
@ 2020-11-02 18:06 ` trondmy
2020-11-02 18:06 ` [PATCH 12/12] NFS: Reduce readdir stack usage trondmy
0 siblings, 1 reply; 20+ messages in thread
From: trondmy @ 2020-11-02 18:06 UTC (permalink / raw)
To: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
Clean up nfs_do_filldir().
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/dir.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 564e737ef93e..a2cebd365948 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -875,13 +875,11 @@ int readdir_search_pagecache(nfs_readdir_descriptor_t *desc)
/*
* Once we've found the start of the dirent within a page: fill 'er up...
*/
-static
-int nfs_do_filldir(nfs_readdir_descriptor_t *desc)
+static void nfs_do_filldir(struct nfs_readdir_descriptor *desc)
{
struct file *file = desc->file;
- int i = 0;
- int res = 0;
- struct nfs_cache_array *array = NULL;
+ struct nfs_cache_array *array;
+ unsigned int i = 0;
array = kmap(desc->page);
for (i = desc->cache_entry_index; i < array->size; i++) {
@@ -908,9 +906,8 @@ int nfs_do_filldir(nfs_readdir_descriptor_t *desc)
desc->eof = true;
kunmap(desc->page);
- dfprintk(DIRCACHE, "NFS: nfs_do_filldir() filling ended @ cookie %Lu; returning = %d\n",
- (unsigned long long)desc->dir_cookie, res);
- return res;
+ dfprintk(DIRCACHE, "NFS: nfs_do_filldir() filling ended @ cookie %llu\n",
+ (unsigned long long)desc->dir_cookie);
}
/*
@@ -951,7 +948,7 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc)
if (status < 0)
goto out_release;
- status = nfs_do_filldir(desc);
+ nfs_do_filldir(desc);
out_release:
nfs_readdir_clear_array(desc->page);
@@ -1026,11 +1023,9 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
if (res < 0)
break;
- res = nfs_do_filldir(desc);
+ nfs_do_filldir(desc);
unlock_page(desc->page);
nfs_readdir_page_put(desc);
- if (res < 0)
- break;
} while (!desc->eof);
spin_lock(&file->f_lock);
@@ -1041,8 +1036,6 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
spin_unlock(&file->f_lock);
out:
- if (res > 0)
- res = 0;
dfprintk(FILE, "NFS: readdir(%pD2) returns %d\n", file, res);
return res;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 12/12] NFS: Reduce readdir stack usage
2020-11-02 18:06 ` [PATCH 11/12] NFS: nfs_do_filldir() does not return a value trondmy
@ 2020-11-02 18:06 ` trondmy
0 siblings, 0 replies; 20+ messages in thread
From: trondmy @ 2020-11-02 18:06 UTC (permalink / raw)
To: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
The descriptor and the struct nfs_entry are both large structures,
so don't allocate them from the stack.
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/dir.c | 88 +++++++++++++++++++++++++++-------------------------
1 file changed, 46 insertions(+), 42 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index a2cebd365948..966e8fc6c9b7 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -144,7 +144,7 @@ struct nfs_cache_array {
struct nfs_cache_array_entry array[];
};
-typedef struct nfs_readdir_descriptor {
+struct nfs_readdir_descriptor {
struct file *file;
struct page *page;
struct dir_context *ctx;
@@ -163,7 +163,7 @@ typedef struct nfs_readdir_descriptor {
signed char duped;
bool plus;
bool eof;
-} nfs_readdir_descriptor_t;
+};
static void nfs_readdir_array_init(struct nfs_cache_array *array)
{
@@ -358,8 +358,8 @@ bool nfs_readdir_use_cookie(const struct file *filp)
return true;
}
-static
-int nfs_readdir_search_for_pos(struct nfs_cache_array *array, nfs_readdir_descriptor_t *desc)
+static int nfs_readdir_search_for_pos(struct nfs_cache_array *array,
+ struct nfs_readdir_descriptor *desc)
{
loff_t diff = desc->ctx->pos - desc->current_index;
unsigned int index;
@@ -390,8 +390,8 @@ nfs_readdir_inode_mapping_valid(struct nfs_inode *nfsi)
return !test_bit(NFS_INO_INVALIDATING, &nfsi->flags);
}
-static
-int nfs_readdir_search_for_cookie(struct nfs_cache_array *array, nfs_readdir_descriptor_t *desc)
+static int nfs_readdir_search_for_cookie(struct nfs_cache_array *array,
+ struct nfs_readdir_descriptor *desc)
{
int i;
loff_t new_pos;
@@ -439,8 +439,7 @@ int nfs_readdir_search_for_cookie(struct nfs_cache_array *array, nfs_readdir_des
return status;
}
-static
-int nfs_readdir_search_array(nfs_readdir_descriptor_t *desc)
+static int nfs_readdir_search_array(struct nfs_readdir_descriptor *desc)
{
struct nfs_cache_array *array;
int status;
@@ -493,7 +492,7 @@ static int nfs_readdir_xdr_filler(struct nfs_readdir_descriptor *desc,
return error;
}
-static int xdr_decode(nfs_readdir_descriptor_t *desc,
+static int xdr_decode(struct nfs_readdir_descriptor *desc,
struct nfs_entry *entry, struct xdr_stream *xdr)
{
struct inode *inode = file_inode(desc->file);
@@ -757,27 +756,28 @@ static struct page **nfs_readdir_alloc_pages(size_t npages)
return NULL;
}
-static
-int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page, struct inode *inode)
+static int nfs_readdir_xdr_to_array(struct nfs_readdir_descriptor *desc,
+ struct page *page, struct inode *inode)
{
struct page **pages;
- struct nfs_entry entry;
+ struct nfs_entry *entry;
size_t array_size;
size_t dtsize = NFS_SERVER(inode)->dtsize;
int status = -ENOMEM;
- entry.prev_cookie = 0;
- entry.cookie = nfs_readdir_page_last_cookie(page);
- entry.eof = 0;
- entry.fh = nfs_alloc_fhandle();
- entry.fattr = nfs_alloc_fattr();
- entry.server = NFS_SERVER(inode);
- if (entry.fh == NULL || entry.fattr == NULL)
+ entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+ if (!entry)
+ return -ENOMEM;
+ entry->cookie = nfs_readdir_page_last_cookie(page);
+ entry->fh = nfs_alloc_fhandle();
+ entry->fattr = nfs_alloc_fattr();
+ entry->server = NFS_SERVER(inode);
+ if (entry->fh == NULL || entry->fattr == NULL)
goto out;
- entry.label = nfs4_label_alloc(NFS_SERVER(inode), GFP_NOWAIT);
- if (IS_ERR(entry.label)) {
- status = PTR_ERR(entry.label);
+ entry->label = nfs4_label_alloc(NFS_SERVER(inode), GFP_NOWAIT);
+ if (IS_ERR(entry->label)) {
+ status = PTR_ERR(entry->label);
goto out;
}
@@ -788,7 +788,7 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
do {
unsigned int pglen;
- status = nfs_readdir_xdr_filler(desc, entry.cookie,
+ status = nfs_readdir_xdr_filler(desc, entry->cookie,
pages, dtsize);
if (status < 0)
break;
@@ -799,15 +799,16 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
break;
}
- status = nfs_readdir_page_filler(desc, &entry, pages, page, pglen);
+ status = nfs_readdir_page_filler(desc, entry, pages, page, pglen);
} while (!status && nfs_readdir_page_needs_filling(page));
nfs_readdir_free_pages(pages, array_size);
out_release_label:
- nfs4_label_free(entry.label);
+ nfs4_label_free(entry->label);
out:
- nfs_free_fattr(entry.fattr);
- nfs_free_fhandle(entry.fh);
+ nfs_free_fattr(entry->fattr);
+ nfs_free_fhandle(entry->fh);
+ kfree(entry);
return status;
}
@@ -829,8 +830,7 @@ nfs_readdir_page_get_cached(struct nfs_readdir_descriptor *desc)
* Returns 0 if desc->dir_cookie was found on page desc->page_index
* and locks the page to prevent removal from the page cache.
*/
-static
-int find_and_lock_cache_page(nfs_readdir_descriptor_t *desc)
+static int find_and_lock_cache_page(struct nfs_readdir_descriptor *desc)
{
struct inode *inode = file_inode(desc->file);
struct nfs_inode *nfsi = NFS_I(inode);
@@ -856,8 +856,7 @@ int find_and_lock_cache_page(nfs_readdir_descriptor_t *desc)
}
/* Search for desc->dir_cookie from the beginning of the page cache */
-static inline
-int readdir_search_pagecache(nfs_readdir_descriptor_t *desc)
+static int readdir_search_pagecache(struct nfs_readdir_descriptor *desc)
{
int res;
@@ -922,8 +921,7 @@ static void nfs_do_filldir(struct nfs_readdir_descriptor *desc)
* we should already have a complete representation of the
* directory in the page cache by the time we get here.
*/
-static inline
-int uncached_readdir(nfs_readdir_descriptor_t *desc)
+static int uncached_readdir(struct nfs_readdir_descriptor *desc)
{
struct page *page = NULL;
int status;
@@ -968,13 +966,8 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
struct dentry *dentry = file_dentry(file);
struct inode *inode = d_inode(dentry);
struct nfs_open_dir_context *dir_ctx = file->private_data;
- nfs_readdir_descriptor_t my_desc = {
- .file = file,
- .ctx = ctx,
- .plus = nfs_use_readdirplus(inode, ctx),
- },
- *desc = &my_desc;
- int res = 0;
+ struct nfs_readdir_descriptor *desc;
+ int res;
dfprintk(FILE, "NFS: readdir(%pD2) starting at cookie %llu\n",
file, (long long)ctx->pos);
@@ -986,10 +979,19 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
* to either find the entry with the appropriate number or
* revalidate the cookie.
*/
- if (ctx->pos == 0 || nfs_attribute_cache_expired(inode))
+ if (ctx->pos == 0 || nfs_attribute_cache_expired(inode)) {
res = nfs_revalidate_mapping(inode, file->f_mapping);
- if (res < 0)
+ if (res < 0)
+ goto out;
+ }
+
+ res = -ENOMEM;
+ desc = kzalloc(sizeof(*desc), GFP_KERNEL);
+ if (!desc)
goto out;
+ desc->file = file;
+ desc->ctx = ctx;
+ desc->plus = nfs_use_readdirplus(inode, ctx);
spin_lock(&file->f_lock);
desc->dir_cookie = dir_ctx->dir_cookie;
@@ -1035,6 +1037,8 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
dir_ctx->attr_gencount = desc->attr_gencount;
spin_unlock(&file->f_lock);
+ kfree(desc);
+
out:
dfprintk(FILE, "NFS: readdir(%pD2) returns %d\n", file, res);
return res;
--
2.28.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 00/12] Readdir enhancements
2020-11-02 18:06 [PATCH 00/12] Readdir enhancements trondmy
2020-11-02 18:06 ` [PATCH 01/12] NFS: Ensure contents of struct nfs_open_dir_context are consistent trondmy
@ 2020-11-02 20:40 ` David Wysochanski
2020-11-02 21:32 ` Trond Myklebust
1 sibling, 1 reply; 20+ messages in thread
From: David Wysochanski @ 2020-11-02 20:40 UTC (permalink / raw)
To: trondmy; +Cc: linux-nfs
On Mon, Nov 2, 2020 at 1:17 PM <trondmy@kernel.org> wrote:
>
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>
> The following patch series performs a number of cleanups on the readdir
> code.
> It also adds support for 1MB readdir RPC calls on-the-wire, and modifies
> the caching code to ensure that we cache the entire contents of that
> 1MB call (instead of discarding the data that doesn't fit into a single
> page).
>
> Trond Myklebust (12):
> NFS: Ensure contents of struct nfs_open_dir_context are consistent
> NFS: Clean up readdir struct nfs_cache_array
> NFS: Clean up nfs_readdir_page_filler()
> NFS: Clean up directory array handling
> NFS: Don't discard readdir results
> NFS: Remove unnecessary kmap in nfs_readdir_xdr_to_array()
> NFS: Replace kmap() with kmap_atomic() in nfs_readdir_search_array()
> NFS: Simplify struct nfs_cache_array_entry
> NFS: Support larger readdir buffers
> NFS: More readdir cleanups
> NFS: nfs_do_filldir() does not return a value
> NFS: Reduce readdir stack usage
>
> fs/nfs/client.c | 4 +-
> fs/nfs/dir.c | 555 ++++++++++++++++++++++++-----------------
> fs/nfs/internal.h | 6 -
> include/linux/nfs_fs.h | 1 -
> 4 files changed, 325 insertions(+), 241 deletions(-)
>
> --
> 2.28.0
>
Nice to see these, especially
[PATCH 05/12] NFS: Don't discard readdir results
Are you testing these on top of 5.10-rc2 or something else?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 00/12] Readdir enhancements
2020-11-02 20:40 ` [PATCH 00/12] Readdir enhancements David Wysochanski
@ 2020-11-02 21:32 ` Trond Myklebust
0 siblings, 0 replies; 20+ messages in thread
From: Trond Myklebust @ 2020-11-02 21:32 UTC (permalink / raw)
To: dwysocha; +Cc: linux-nfs
On Mon, 2020-11-02 at 15:40 -0500, David Wysochanski wrote:
> On Mon, Nov 2, 2020 at 1:17 PM <trondmy@kernel.org> wrote:
> >
> > From: Trond Myklebust <trond.myklebust@hammerspace.com>
> >
> > The following patch series performs a number of cleanups on the
> > readdir
> > code.
> > It also adds support for 1MB readdir RPC calls on-the-wire, and
> > modifies
> > the caching code to ensure that we cache the entire contents of
> > that
> > 1MB call (instead of discarding the data that doesn't fit into a
> > single
> > page).
> >
> > Trond Myklebust (12):
> > NFS: Ensure contents of struct nfs_open_dir_context are
> > consistent
> > NFS: Clean up readdir struct nfs_cache_array
> > NFS: Clean up nfs_readdir_page_filler()
> > NFS: Clean up directory array handling
> > NFS: Don't discard readdir results
> > NFS: Remove unnecessary kmap in nfs_readdir_xdr_to_array()
> > NFS: Replace kmap() with kmap_atomic() in
> > nfs_readdir_search_array()
> > NFS: Simplify struct nfs_cache_array_entry
> > NFS: Support larger readdir buffers
> > NFS: More readdir cleanups
> > NFS: nfs_do_filldir() does not return a value
> > NFS: Reduce readdir stack usage
> >
> > fs/nfs/client.c | 4 +-
> > fs/nfs/dir.c | 555 ++++++++++++++++++++++++-------------
> > ----
> > fs/nfs/internal.h | 6 -
> > include/linux/nfs_fs.h | 1 -
> > 4 files changed, 325 insertions(+), 241 deletions(-)
> >
> > --
> > 2.28.0
> >
>
> Nice to see these, especially
> [PATCH 05/12] NFS: Don't discard readdir results
>
> Are you testing these on top of 5.10-rc2 or something else?
>
They are being tested on 5.10-rc2.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 02/12] NFS: Clean up readdir struct nfs_cache_array
2020-11-02 18:06 ` [PATCH 02/12] NFS: Clean up readdir struct nfs_cache_array trondmy
2020-11-02 18:06 ` [PATCH 03/12] NFS: Clean up nfs_readdir_page_filler() trondmy
@ 2020-11-03 13:35 ` Benjamin Coddington
2020-11-03 14:09 ` Benjamin Coddington
1 sibling, 1 reply; 20+ messages in thread
From: Benjamin Coddington @ 2020-11-03 13:35 UTC (permalink / raw)
To: trondmy; +Cc: linux-nfs
On 2 Nov 2020, at 13:06, trondmy@kernel.org wrote:
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>
> Since the 'eof_index' is only ever used as a flag, make it so.
> Also add a flag to detect if the page has been completely filled.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
> fs/nfs/dir.c | 66
> ++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 49 insertions(+), 17 deletions(-)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 67d8595cd6e5..604ebe015387 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -138,9 +138,10 @@ struct nfs_cache_array_entry {
> };
>
> struct nfs_cache_array {
> - int size;
> - int eof_index;
> u64 last_cookie;
> + unsigned int size;
> + unsigned char page_full : 1,
> + page_is_eof : 1;
> struct nfs_cache_array_entry array[];
> };
>
> @@ -172,7 +173,6 @@ void nfs_readdir_init_array(struct page *page)
>
> array = kmap_atomic(page);
> memset(array, 0, sizeof(struct nfs_cache_array));
> - array->eof_index = -1;
> kunmap_atomic(array);
> }
>
> @@ -192,6 +192,17 @@ void nfs_readdir_clear_array(struct page *page)
> kunmap_atomic(array);
> }
>
> +static void nfs_readdir_array_set_eof(struct nfs_cache_array *array)
> +{
> + array->page_is_eof = 1;
> + array->page_full = 1;
> +}
> +
> +static bool nfs_readdir_array_is_full(struct nfs_cache_array *array)
> +{
> + return array->page_full;
> +}
> +
> /*
> * the caller is responsible for freeing qstr.name
> * when called by nfs_readdir_add_to_array, the strings will be freed
> in
> @@ -213,6 +224,23 @@ int nfs_readdir_make_qstr(struct qstr *string,
> const char *name, unsigned int le
> return 0;
> }
>
> +/*
> + * Check that the next array entry lies entirely within the page
> bounds
> + */
> +static int nfs_readdir_array_can_expand(struct nfs_cache_array
> *array)
> +{
> + struct nfs_cache_array_entry *cache_entry;
> +
> + if (array->page_full)
> + return -ENOSPC;
> + cache_entry = &array->array[array->size + 1];
> + if ((char *)cache_entry - (char *)array > PAGE_SIZE) {
> + array->page_full = 1;
> + return -ENOSPC;
> + }
> + return 0;
> +}
> +
I think we could do this calculation at compile-time instead of doing it
for
each entry, for dubious nominal gains..
Ben
> static
> int nfs_readdir_add_to_array(struct nfs_entry *entry, struct page
> *page)
> {
> @@ -220,13 +248,11 @@ int nfs_readdir_add_to_array(struct nfs_entry
> *entry, struct page *page)
> struct nfs_cache_array_entry *cache_entry;
> int ret;
>
> - cache_entry = &array->array[array->size];
> -
> - /* Check that this entry lies within the page bounds */
> - ret = -ENOSPC;
> - if ((char *)&cache_entry[1] - (char *)page_address(page) >
> PAGE_SIZE)
> + ret = nfs_readdir_array_can_expand(array);
> + if (ret)
> goto out;
>
> + cache_entry = &array->array[array->size];
> cache_entry->cookie = entry->prev_cookie;
> cache_entry->ino = entry->ino;
> cache_entry->d_type = entry->d_type;
> @@ -236,12 +262,21 @@ int nfs_readdir_add_to_array(struct nfs_entry
> *entry, struct page *page)
> array->last_cookie = entry->cookie;
> array->size++;
> if (entry->eof != 0)
> - array->eof_index = array->size;
> + nfs_readdir_array_set_eof(array);
> out:
> kunmap(page);
> return ret;
> }
>
> +static void nfs_readdir_page_set_eof(struct page *page)
> +{
> + struct nfs_cache_array *array;
> +
> + array = kmap_atomic(page);
> + nfs_readdir_array_set_eof(array);
> + kunmap_atomic(array);
> +}
> +
> static inline
> int is_32bit_api(void)
> {
> @@ -270,7 +305,7 @@ int nfs_readdir_search_for_pos(struct
> nfs_cache_array *array, nfs_readdir_descri
> if (diff < 0)
> goto out_eof;
> if (diff >= array->size) {
> - if (array->eof_index >= 0)
> + if (array->page_is_eof)
> goto out_eof;
> return -EAGAIN;
> }
> @@ -334,7 +369,7 @@ int nfs_readdir_search_for_cookie(struct
> nfs_cache_array *array, nfs_readdir_des
> return 0;
> }
> }
> - if (array->eof_index >= 0) {
> + if (array->page_is_eof) {
> status = -EBADCOOKIE;
> if (desc->dir_cookie == array->last_cookie)
> desc->eof = true;
> @@ -566,7 +601,6 @@ int
> nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct
> nfs_entry *en
> struct xdr_stream stream;
> struct xdr_buf buf;
> struct page *scratch;
> - struct nfs_cache_array *array;
> unsigned int count = 0;
> int status;
>
> @@ -604,10 +638,8 @@ int
> nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct
> nfs_entry *en
>
> out_nopages:
> if (count == 0 || (status == -EBADCOOKIE && entry->eof != 0)) {
> - array = kmap(page);
> - array->eof_index = array->size;
> + nfs_readdir_page_set_eof(page);
> status = 0;
> - kunmap(page);
> }
>
> put_page(scratch);
> @@ -689,7 +721,7 @@ int
> nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page
> *page,
> status = 0;
> break;
> }
> - } while (array->eof_index < 0);
> + } while (!nfs_readdir_array_is_full(array));
>
> nfs_readdir_free_pages(pages, array_size);
> out_release_array:
> @@ -825,7 +857,7 @@ int nfs_do_filldir(nfs_readdir_descriptor_t *desc)
> if (desc->duped != 0)
> desc->duped = 1;
> }
> - if (array->eof_index >= 0)
> + if (array->page_is_eof)
> desc->eof = true;
>
> kunmap(desc->page);
> --
> 2.28.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 02/12] NFS: Clean up readdir struct nfs_cache_array
2020-11-03 13:35 ` [PATCH 02/12] NFS: Clean up readdir struct nfs_cache_array Benjamin Coddington
@ 2020-11-03 14:09 ` Benjamin Coddington
2020-11-03 14:34 ` Benjamin Coddington
0 siblings, 1 reply; 20+ messages in thread
From: Benjamin Coddington @ 2020-11-03 14:09 UTC (permalink / raw)
To: trondmy; +Cc: linux-nfs
On 3 Nov 2020, at 8:35, Benjamin Coddington wrote:
> On 2 Nov 2020, at 13:06, trondmy@kernel.org wrote:
>
>> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>>
>> Since the 'eof_index' is only ever used as a flag, make it so.
>> Also add a flag to detect if the page has been completely filled.
>>
>> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
>> ---
>> fs/nfs/dir.c | 66
>> ++++++++++++++++++++++++++++++++++++++--------------
>> 1 file changed, 49 insertions(+), 17 deletions(-)
>>
>> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
>> index 67d8595cd6e5..604ebe015387 100644
>> --- a/fs/nfs/dir.c
>> +++ b/fs/nfs/dir.c
>> @@ -138,9 +138,10 @@ struct nfs_cache_array_entry {
>> };
>>
>> struct nfs_cache_array {
>> - int size;
>> - int eof_index;
>> u64 last_cookie;
>> + unsigned int size;
>> + unsigned char page_full : 1,
>> + page_is_eof : 1;
>> struct nfs_cache_array_entry array[];
>> };
>>
>> @@ -172,7 +173,6 @@ void nfs_readdir_init_array(struct page *page)
>>
>> array = kmap_atomic(page);
>> memset(array, 0, sizeof(struct nfs_cache_array));
>> - array->eof_index = -1;
>> kunmap_atomic(array);
>> }
>>
>> @@ -192,6 +192,17 @@ void nfs_readdir_clear_array(struct page *page)
>> kunmap_atomic(array);
>> }
>>
>> +static void nfs_readdir_array_set_eof(struct nfs_cache_array *array)
>> +{
>> + array->page_is_eof = 1;
>> + array->page_full = 1;
>> +}
>> +
>> +static bool nfs_readdir_array_is_full(struct nfs_cache_array *array)
>> +{
>> + return array->page_full;
>> +}
>> +
>> /*
>> * the caller is responsible for freeing qstr.name
>> * when called by nfs_readdir_add_to_array, the strings will be
>> freed in
>> @@ -213,6 +224,23 @@ int nfs_readdir_make_qstr(struct qstr *string,
>> const char *name, unsigned int le
>> return 0;
>> }
>>
>> +/*
>> + * Check that the next array entry lies entirely within the page
>> bounds
>> + */
>> +static int nfs_readdir_array_can_expand(struct nfs_cache_array
>> *array)
>> +{
>> + struct nfs_cache_array_entry *cache_entry;
>> +
>> + if (array->page_full)
>> + return -ENOSPC;
>> + cache_entry = &array->array[array->size + 1];
>> + if ((char *)cache_entry - (char *)array > PAGE_SIZE) {
>> + array->page_full = 1;
>> + return -ENOSPC;
>> + }
>> + return 0;
>> +}
>> +
>
> I think we could do this calculation at compile-time instead of doing
> it for
> each entry, for dubious nominal gains..
and doing so might allow us to detect the case where the array is full
before doing another RPC that we'll discard.
Ben
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 03/12] NFS: Clean up nfs_readdir_page_filler()
2020-11-02 18:06 ` [PATCH 03/12] NFS: Clean up nfs_readdir_page_filler() trondmy
2020-11-02 18:06 ` [PATCH 04/12] NFS: Clean up directory array handling trondmy
@ 2020-11-03 14:18 ` Benjamin Coddington
2020-11-03 14:24 ` Benjamin Coddington
1 sibling, 1 reply; 20+ messages in thread
From: Benjamin Coddington @ 2020-11-03 14:18 UTC (permalink / raw)
To: trondmy; +Cc: linux-nfs
On 2 Nov 2020, at 13:06, trondmy@kernel.org wrote:
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>
> Clean up handling of the case where there are no entries in the
> readdir
> reply.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
> fs/nfs/dir.c | 39 ++++++++++++++++++---------------------
> 1 file changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 604ebe015387..68acbde3f914 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -601,16 +601,12 @@ int
> nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct
> nfs_entry *en
> struct xdr_stream stream;
> struct xdr_buf buf;
> struct page *scratch;
> - unsigned int count = 0;
> int status;
>
> scratch = alloc_page(GFP_KERNEL);
> if (scratch == NULL)
> return -ENOMEM;
>
> - if (buflen == 0)
> - goto out_nopages;
> -
> xdr_init_decode_pages(&stream, &buf, xdr_pages, buflen);
> xdr_set_scratch_buffer(&stream, page_address(scratch), PAGE_SIZE);
>
> @@ -619,27 +615,27 @@ int
> nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct
> nfs_entry *en
> entry->label->len = NFS4_MAXLABELLEN;
>
> status = xdr_decode(desc, entry, &stream);
> - if (status != 0) {
> - if (status == -EAGAIN)
> - status = 0;
> + if (status != 0)
> break;
> - }
> -
> - count++;
>
> if (desc->plus)
> nfs_prime_dcache(file_dentry(desc->file), entry,
> desc->dir_verifier);
>
> status = nfs_readdir_add_to_array(entry, page);
> - if (status != 0)
> - break;
> - } while (!entry->eof);
> + } while (!status && !entry->eof);
>
> -out_nopages:
> - if (count == 0 || (status == -EBADCOOKIE && entry->eof != 0)) {
> - nfs_readdir_page_set_eof(page);
> + switch (status) {
> + case -EBADCOOKIE:
> + if (entry->eof) {
> + nfs_readdir_page_set_eof(page);
> + status = 0;
> + }
> + break;
> + case -ENOSPC:
If you return ENOSPC, then you don't need to use
nfs_readdir_array_is_full(array) below..
> + case -EAGAIN:
> status = 0;
> + break;
> }
>
> put_page(scratch);
> @@ -714,14 +710,15 @@ int
> nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page
> *page,
>
> if (status < 0)
> break;
> +
> pglen = status;
> - status = nfs_readdir_page_filler(desc, &entry, pages, page, pglen);
> - if (status < 0) {
> - if (status == -ENOSPC)
> - status = 0;
> + if (pglen == 0) {
> + nfs_readdir_page_set_eof(page);
> break;
> }
> - } while (!nfs_readdir_array_is_full(array));
> +
> + status = nfs_readdir_page_filler(desc, &entry, pages, page, pglen);
> + } while (!status && !nfs_readdir_array_is_full(array));
^^ here.
I suppose nfs_readdir_array_is_full() is nice and clear.. I wonder if
the
compiler would optimize it away.
Ben
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 03/12] NFS: Clean up nfs_readdir_page_filler()
2020-11-03 14:18 ` [PATCH 03/12] NFS: Clean up nfs_readdir_page_filler() Benjamin Coddington
@ 2020-11-03 14:24 ` Benjamin Coddington
0 siblings, 0 replies; 20+ messages in thread
From: Benjamin Coddington @ 2020-11-03 14:24 UTC (permalink / raw)
To: trondmy; +Cc: linux-nfs
On 3 Nov 2020, at 9:18, Benjamin Coddington wrote:
> On 2 Nov 2020, at 13:06, trondmy@kernel.org wrote:
>
>> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>>
>> Clean up handling of the case where there are no entries in the
>> readdir
>> reply.
>>
>> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
>> ---
>> fs/nfs/dir.c | 39 ++++++++++++++++++---------------------
>> 1 file changed, 18 insertions(+), 21 deletions(-)
>>
>> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
>> index 604ebe015387..68acbde3f914 100644
>> --- a/fs/nfs/dir.c
>> +++ b/fs/nfs/dir.c
>> @@ -601,16 +601,12 @@ int
>> nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct
>> nfs_entry *en
>> struct xdr_stream stream;
>> struct xdr_buf buf;
>> struct page *scratch;
>> - unsigned int count = 0;
>> int status;
>>
>> scratch = alloc_page(GFP_KERNEL);
>> if (scratch == NULL)
>> return -ENOMEM;
>>
>> - if (buflen == 0)
>> - goto out_nopages;
>> -
>> xdr_init_decode_pages(&stream, &buf, xdr_pages, buflen);
>> xdr_set_scratch_buffer(&stream, page_address(scratch), PAGE_SIZE);
>>
>> @@ -619,27 +615,27 @@ int
>> nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct
>> nfs_entry *en
>> entry->label->len = NFS4_MAXLABELLEN;
>>
>> status = xdr_decode(desc, entry, &stream);
>> - if (status != 0) {
>> - if (status == -EAGAIN)
>> - status = 0;
>> + if (status != 0)
>> break;
>> - }
>> -
>> - count++;
>>
>> if (desc->plus)
>> nfs_prime_dcache(file_dentry(desc->file), entry,
>> desc->dir_verifier);
>>
>> status = nfs_readdir_add_to_array(entry, page);
>> - if (status != 0)
>> - break;
>> - } while (!entry->eof);
>> + } while (!status && !entry->eof);
>>
>> -out_nopages:
>> - if (count == 0 || (status == -EBADCOOKIE && entry->eof != 0)) {
>> - nfs_readdir_page_set_eof(page);
>> + switch (status) {
>> + case -EBADCOOKIE:
>> + if (entry->eof) {
>> + nfs_readdir_page_set_eof(page);
>> + status = 0;
>> + }
>> + break;
>> + case -ENOSPC:
>
> If you return ENOSPC, then you don't need to use
> nfs_readdir_array_is_full(array) below..
>
>> + case -EAGAIN:
>> status = 0;
>> + break;
>> }
>>
>> put_page(scratch);
>> @@ -714,14 +710,15 @@ int
>> nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page
>> *page,
>>
>> if (status < 0)
>> break;
>> +
>> pglen = status;
>> - status = nfs_readdir_page_filler(desc, &entry, pages, page,
>> pglen);
>> - if (status < 0) {
>> - if (status == -ENOSPC)
>> - status = 0;
>> + if (pglen == 0) {
>> + nfs_readdir_page_set_eof(page);
>> break;
>> }
>> - } while (!nfs_readdir_array_is_full(array));
>> +
>> + status = nfs_readdir_page_filler(desc, &entry, pages, page,
>> pglen);
>> + } while (!status && !nfs_readdir_array_is_full(array));
>
> ^^ here.
>
> I suppose nfs_readdir_array_is_full() is nice and clear.. I wonder if
> the
> compiler would optimize it away.
Ah, I see now -- the check handles the case of eof as well..
I suppose I shouldn't send a stream of consciousness to the list. Sorry
about that.
Ben
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 02/12] NFS: Clean up readdir struct nfs_cache_array
2020-11-03 14:09 ` Benjamin Coddington
@ 2020-11-03 14:34 ` Benjamin Coddington
0 siblings, 0 replies; 20+ messages in thread
From: Benjamin Coddington @ 2020-11-03 14:34 UTC (permalink / raw)
To: trondmy; +Cc: linux-nfs
On 3 Nov 2020, at 9:09, Benjamin Coddington wrote:
> On 3 Nov 2020, at 8:35, Benjamin Coddington wrote:
>
>> On 2 Nov 2020, at 13:06, trondmy@kernel.org wrote:
>>
>>> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>>>
>>> Since the 'eof_index' is only ever used as a flag, make it so.
>>> Also add a flag to detect if the page has been completely filled.
>>>
>>> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
>>> ---
>>> fs/nfs/dir.c | 66
>>> ++++++++++++++++++++++++++++++++++++++--------------
>>> 1 file changed, 49 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
>>> index 67d8595cd6e5..604ebe015387 100644
>>> --- a/fs/nfs/dir.c
>>> +++ b/fs/nfs/dir.c
>>> @@ -138,9 +138,10 @@ struct nfs_cache_array_entry {
>>> };
>>>
>>> struct nfs_cache_array {
>>> - int size;
>>> - int eof_index;
>>> u64 last_cookie;
>>> + unsigned int size;
>>> + unsigned char page_full : 1,
>>> + page_is_eof : 1;
>>> struct nfs_cache_array_entry array[];
>>> };
>>>
>>> @@ -172,7 +173,6 @@ void nfs_readdir_init_array(struct page *page)
>>>
>>> array = kmap_atomic(page);
>>> memset(array, 0, sizeof(struct nfs_cache_array));
>>> - array->eof_index = -1;
>>> kunmap_atomic(array);
>>> }
>>>
>>> @@ -192,6 +192,17 @@ void nfs_readdir_clear_array(struct page *page)
>>> kunmap_atomic(array);
>>> }
>>>
>>> +static void nfs_readdir_array_set_eof(struct nfs_cache_array
>>> *array)
>>> +{
>>> + array->page_is_eof = 1;
>>> + array->page_full = 1;
>>> +}
>>> +
>>> +static bool nfs_readdir_array_is_full(struct nfs_cache_array
>>> *array)
>>> +{
>>> + return array->page_full;
>>> +}
>>> +
>>> /*
>>> * the caller is responsible for freeing qstr.name
>>> * when called by nfs_readdir_add_to_array, the strings will be
>>> freed in
>>> @@ -213,6 +224,23 @@ int nfs_readdir_make_qstr(struct qstr *string,
>>> const char *name, unsigned int le
>>> return 0;
>>> }
>>>
>>> +/*
>>> + * Check that the next array entry lies entirely within the page
>>> bounds
>>> + */
>>> +static int nfs_readdir_array_can_expand(struct nfs_cache_array
>>> *array)
>>> +{
>>> + struct nfs_cache_array_entry *cache_entry;
>>> +
>>> + if (array->page_full)
>>> + return -ENOSPC;
>>> + cache_entry = &array->array[array->size + 1];
>>> + if ((char *)cache_entry - (char *)array > PAGE_SIZE) {
>>> + array->page_full = 1;
>>> + return -ENOSPC;
>>> + }
>>> + return 0;
>>> +}
>>> +
>>
>> I think we could do this calculation at compile-time instead of doing
>> it for
>> each entry, for dubious nominal gains..
>
> and doing so might allow us to detect the case where the array is full
> before doing another RPC that we'll discard.
And you handle this case in patch 4/12...
Ben
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2020-11-03 14:36 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-02 18:06 [PATCH 00/12] Readdir enhancements trondmy
2020-11-02 18:06 ` [PATCH 01/12] NFS: Ensure contents of struct nfs_open_dir_context are consistent trondmy
2020-11-02 18:06 ` [PATCH 02/12] NFS: Clean up readdir struct nfs_cache_array trondmy
2020-11-02 18:06 ` [PATCH 03/12] NFS: Clean up nfs_readdir_page_filler() trondmy
2020-11-02 18:06 ` [PATCH 04/12] NFS: Clean up directory array handling trondmy
2020-11-02 18:06 ` [PATCH 05/12] NFS: Don't discard readdir results trondmy
2020-11-02 18:06 ` [PATCH 06/12] NFS: Remove unnecessary kmap in nfs_readdir_xdr_to_array() trondmy
2020-11-02 18:06 ` [PATCH 07/12] NFS: Replace kmap() with kmap_atomic() in nfs_readdir_search_array() trondmy
2020-11-02 18:06 ` [PATCH 08/12] NFS: Simplify struct nfs_cache_array_entry trondmy
2020-11-02 18:06 ` [PATCH 09/12] NFS: Support larger readdir buffers trondmy
2020-11-02 18:06 ` [PATCH 10/12] NFS: More readdir cleanups trondmy
2020-11-02 18:06 ` [PATCH 11/12] NFS: nfs_do_filldir() does not return a value trondmy
2020-11-02 18:06 ` [PATCH 12/12] NFS: Reduce readdir stack usage trondmy
2020-11-03 14:18 ` [PATCH 03/12] NFS: Clean up nfs_readdir_page_filler() Benjamin Coddington
2020-11-03 14:24 ` Benjamin Coddington
2020-11-03 13:35 ` [PATCH 02/12] NFS: Clean up readdir struct nfs_cache_array Benjamin Coddington
2020-11-03 14:09 ` Benjamin Coddington
2020-11-03 14:34 ` Benjamin Coddington
2020-11-02 20:40 ` [PATCH 00/12] Readdir enhancements David Wysochanski
2020-11-02 21:32 ` Trond Myklebust
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).