Hi Ben,
On Wed, Jan 20, 2021 at 12:04 PM Benjamin Coddington
<bcodding@xxxxxxxxxx> wrote:
>
> It is now possible that a reader will resume a directory listing after an
> invalidation and fill the rest of the pages with the offset left over from
> the last partially-filled page. These pages will then be recycled and
> refilled by the next reader since their alignment is incorrect.
>
> Add an index to the nfs_cache_array that will indicate where the next entry
> should be filled. This allows partially-filled pages to have the best
> alignment possible. They are more likely to be useful to readers that
> follow.
>
> This optimization targets the case when there are multiple processes
> listing the directory simultaneously. Often the processes will collect and
> block on the same page waiting for a READDIR call to fill the pagecache.
> If the pagecache is invalidated, a partially-filled page will usually
> result. This partially-filled page can immediately be used by all
> processes to emit entries rather than having to discard and refill it for
> every process.
>
> The addition of another integer to struct nfs_cache_array increases its
> size to 24 bytes. We do not lose the original capacity of 127 entries per
> page.
This patch causes cthon basic test #6 to start failing with unexpected
dir entries across all NFS versions:
./test6: readdir
basic tests failed
./test6: (/mnt/test/anna/Connectathon/cthon04) unexpected dir entry 'h'
Luckily, the next patch seems to resolve this issue. Could they maybe
be squashed together?
Thanks,
Anna
Anna
>
> Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
> ---
> fs/nfs/dir.c | 45 ++++++++++++++++++++++++++-------------------
> 1 file changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 4e21b21c75d0..d6101e45fd66 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -143,6 +143,7 @@ struct nfs_cache_array_entry {
>
> struct nfs_cache_array {
> u64 last_cookie;
> + unsigned int index;
> unsigned int size;
> unsigned char page_full : 1,
> page_is_eof : 1,
> @@ -179,13 +180,15 @@ 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)
> +static void
> +nfs_readdir_page_init_array(struct page *page, struct nfs_dir_page_cursor *pgc)
> {
> struct nfs_cache_array *array;
>
> array = kmap_atomic(page);
> nfs_readdir_array_init(array);
> - array->last_cookie = last_cookie;
> + array->last_cookie = pgc->index_cookie;
> + array->index = pgc->entry_index;
> array->cookies_are_ordered = 1;
> kunmap_atomic(array);
> if (page->mapping)
> @@ -224,7 +227,7 @@ void nfs_readdir_clear_array(struct page *page)
> int i;
>
> array = kmap_atomic(page);
> - for (i = 0; i < array->size; i++)
> + for (i = array->index - array->size; i < array->size; i++)
> kfree(array->array[i].name);
> nfs_readdir_array_init(array);
> kunmap_atomic(array);
> @@ -232,19 +235,20 @@ void nfs_readdir_clear_array(struct page *page)
> }
>
> static void
> -nfs_readdir_recycle_page(struct page *page, u64 last_cookie)
> +nfs_readdir_recycle_page(struct page *page, struct nfs_dir_page_cursor *pgc)
> {
> nfs_readdir_clear_array(page);
> nfs_readdir_invalidatepage(page, 0, 0);
> - nfs_readdir_page_init_array(page, last_cookie);
> + nfs_readdir_page_init_array(page, pgc);
> }
>
> static struct page *
> nfs_readdir_page_array_alloc(u64 last_cookie, gfp_t gfp_flags)
> {
> struct page *page = alloc_page(gfp_flags);
> + struct nfs_dir_page_cursor pgc = { .index_cookie = last_cookie };
> if (page)
> - nfs_readdir_page_init_array(page, last_cookie);
> + nfs_readdir_page_init_array(page, &pgc);
> return page;
> }
>
> @@ -309,7 +313,7 @@ static int nfs_readdir_array_can_expand(struct nfs_cache_array *array)
>
> if (array->page_full)
> return -ENOSPC;
> - cache_entry = &array->array[array->size + 1];
> + cache_entry = &array->array[array->index + 1];
> if ((char *)cache_entry - (char *)array > PAGE_SIZE) {
> array->page_full = 1;
> return -ENOSPC;
> @@ -336,7 +340,7 @@ int nfs_readdir_add_to_array(struct nfs_entry *entry, struct page *page)
> goto out;
> }
>
> - cache_entry = &array->array[array->size];
> + cache_entry = &array->array[array->index];
> cache_entry->cookie = entry->prev_cookie;
> cache_entry->ino = entry->ino;
> cache_entry->d_type = entry->d_type;
> @@ -345,6 +349,7 @@ int nfs_readdir_add_to_array(struct nfs_entry *entry, struct page *page)
> array->last_cookie = entry->cookie;
> if (array->last_cookie <= cache_entry->cookie)
> array->cookies_are_ordered = 0;
> + array->index++;
> array->size++;
> if (entry->eof != 0)
> nfs_readdir_array_set_eof(array);
> @@ -365,13 +370,15 @@ nfs_readdir_page_valid(struct page *page, unsigned int entry_index, u64 index_co
> ret = true;
> array = kmap_atomic(page);
>
> - if ((array->size == 0 || array->size == entry_index)
> - && array->last_cookie == index_cookie)
> - goto out_unmap;
> + if (entry_index >= array->index - array->size) {
> + if ((array->size == 0 || array->size == entry_index)
> + && array->last_cookie == index_cookie)
> + goto out_unmap;
>
> - if (array->size > entry_index &&
> - array->array[entry_index].cookie == index_cookie)
> - goto out_unmap;
> + if (array->size > entry_index &&
> + array->array[entry_index].cookie == index_cookie)
> + goto out_unmap;
> + }
>
> ret = false;
> out_unmap:
> @@ -391,10 +398,10 @@ static struct page *nfs_readdir_page_get_locked(struct address_space *mapping,
> return page;
>
> if (!PageUptodate(page))
> - nfs_readdir_page_init_array(page, pgc->index_cookie);
> + nfs_readdir_page_init_array(page, pgc);
>
> if (!nfs_readdir_page_valid(page, pgc->entry_index, pgc->index_cookie))
> - nfs_readdir_recycle_page(page, pgc->index_cookie);
> + nfs_readdir_recycle_page(page, pgc);
>
> return page;
> }
> @@ -513,7 +520,7 @@ static bool nfs_readdir_array_cookie_in_range(struct nfs_cache_array *array,
> /* Optimisation for monotonically increasing cookies */
> if (cookie >= array->last_cookie)
> return false;
> - if (array->size && cookie < array->array[0].cookie)
> + if (array->size && cookie < array->array[array->index - array->size].cookie)
> return false;
> return true;
> }
> @@ -528,7 +535,7 @@ static int nfs_readdir_search_for_cookie(struct nfs_cache_array *array,
> if (!nfs_readdir_array_cookie_in_range(array, desc->dir_cookie))
> goto check_eof;
>
> - for (i = 0; i < array->size; i++) {
> + for (i = array->index - array->size; i < array->index; i++) {
> if (array->array[i].cookie == desc->dir_cookie) {
> struct nfs_inode *nfsi = NFS_I(file_inode(desc->file));
>
> @@ -1072,7 +1079,7 @@ static void nfs_do_filldir(struct nfs_readdir_descriptor *desc)
> unsigned int i = 0;
>
> array = kmap(desc->page);
> - for (i = desc->pgc.entry_index; i < array->size; i++) {
> + for (i = desc->pgc.entry_index; i < array->index; i++) {
> struct nfs_cache_array_entry *ent;
>
> ent = &array->array[i];
> --
> 2.25.4
>
