From: Anna Schumaker <schumaker.anna@gmail.com>
To: Benjamin Coddington <bcodding@redhat.com>
Cc: Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v1 07/10] NFS: Support headless readdir pagecache pages
Date: Thu, 21 Jan 2021 12:24:58 -0500 [thread overview]
Message-ID: <CAFX2Jf=Kg+fbxSfgJ_Kzxe6LerQ8RcZu_8AYp2JFF4THDfy8fQ@mail.gmail.com> (raw)
In-Reply-To: <7394b4d348d0c92b64cd0fb4fbf74bfa6e676d24.1611160121.git.bcodding@redhat.com>
Hi Ben,
On Wed, Jan 20, 2021 at 12:04 PM Benjamin Coddington
<bcodding@redhat.com> 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@redhat.com>
> ---
> 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
>
next prev parent reply other threads:[~2021-01-21 17:26 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-20 16:59 [PATCH v1 00/10] NFS client readdir per-page validation Benjamin Coddington
2021-01-20 16:59 ` [PATCH v1 01/10] NFS: save the directory's change attribute on pagecache pages Benjamin Coddington
2021-01-20 16:59 ` [PATCH v1 02/10] NFSv4: Send GETATTR with READDIR Benjamin Coddington
2021-01-20 16:59 ` [PATCH v1 03/10] NFS: Add a struct to track readdir pagecache location Benjamin Coddington
2021-01-20 16:59 ` [PATCH v1 04/10] NFS: Keep the readdir pagecache cursor updated Benjamin Coddington
2021-01-21 20:00 ` Trond Myklebust
2021-01-21 20:11 ` Trond Myklebust
2021-01-22 0:21 ` Benjamin Coddington
2021-01-20 16:59 ` [PATCH v1 05/10] NFS: readdir per-page cache validation Benjamin Coddington
2021-01-20 20:08 ` kernel test robot
2021-01-20 16:59 ` [PATCH v1 06/10] NFS: stash the readdir pagecache cursor on the open directory context Benjamin Coddington
2021-01-20 16:59 ` [PATCH v1 07/10] NFS: Support headless readdir pagecache pages Benjamin Coddington
2021-01-21 17:24 ` Anna Schumaker [this message]
2021-01-21 17:57 ` Benjamin Coddington
2021-01-20 16:59 ` [PATCH v1 08/10] NFS: Reset pagecache cursor on llseek Benjamin Coddington
2021-01-20 16:59 ` [PATCH v1 09/10] NFS: Remove nfs_readdir_dont_search_cache() Benjamin Coddington
2021-01-20 16:59 ` [PATCH v1 10/10] NFS: Revalidate the directory pagecache on every nfs_readdir() Benjamin Coddington
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFX2Jf=Kg+fbxSfgJ_Kzxe6LerQ8RcZu_8AYp2JFF4THDfy8fQ@mail.gmail.com' \
--to=schumaker.anna@gmail.com \
--cc=bcodding@redhat.com \
--cc=linux-nfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).