* [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 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-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 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
* 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
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).