| Message ID | 20201103153329.531942-1-trondmy@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Readdir enhancements | expand |
Hi Trond, these look great!
I'm doing some comparison testing before/after this set, and I'm getting
into some memory pressure on a client with 4G ram listing 1.5M dentries
with
12 char filenames.
It looks like before this set, the readdir code was a bit more resilient
in
the face of memory pressure, and I'm wondering if we've dropped a call
to
mark_page_accessed().
* Ben adds:
@@ -460,7 +461,8 @@ static int nfs_readdir_search_array(struct
nfs_readdir_descriptor *desc)
desc->last_cookie = array->last_cookie;
desc->current_index += array->size;
desc->page_index++;
- }
+ } else
+ mark_page_accessed(desc->page);
kunmap_atomic(array);
return status;
}
.. no, that's not any better. I'm still getting evicted pages (or, at
least, low-indexed pages that don't have PageUptodate() set), which
makes
it nearly impossible to finish listing this directory because we just
keep
invalidating the mapping.
Any ideas? I'll keep looking.
Ben
Hi Ben Thanks for the review and the testing! On Wed, 2020-11-04 at 11:14 -0500, Benjamin Coddington wrote: > Hi Trond, these look great! > > I'm doing some comparison testing before/after this set, and I'm > getting > into some memory pressure on a client with 4G ram listing 1.5M > dentries > with > 12 char filenames. > > It looks like before this set, the readdir code was a bit more > resilient > in > the face of memory pressure, and I'm wondering if we've dropped a > call > to > mark_page_accessed(). > > * Ben adds: > > @@ -460,7 +461,8 @@ static int nfs_readdir_search_array(struct > nfs_readdir_descriptor *desc) > desc->last_cookie = array->last_cookie; > desc->current_index += array->size; > desc->page_index++; > - } > + } else > + mark_page_accessed(desc->page); > kunmap_atomic(array); > return status; > } > > .. no, that's not any better. I'm still getting evicted pages (or, > at > least, low-indexed pages that don't have PageUptodate() set), which > makes > it nearly impossible to finish listing this directory because we just > keep > invalidating the mapping. > You're right that I had screwed up the page access marking in the previous patchsets. I believe this should be fixed in v3 by the conversion to use grab_cache_page(), which calls find_or_create_page() and should therefore do the right thing with the FGP_ACCESSED flag. I believe the reason why your patch above fails to fully correct the issue is because we always want to mark the page as accessed if we've scanned it.
On 4 Nov 2020, at 12:04, Trond Myklebust wrote: > Hi Ben > > Thanks for the review and the testing! Thank /you/ for the work! > You're right that I had screwed up the page access marking in the > previous patchsets. I believe this should be fixed in v3 by the > conversion to use grab_cache_page(), which calls find_or_create_page() > and should therefore do the right thing with the FGP_ACCESSED flag. > > I believe the reason why your patch above fails to fully correct the > issue is because we always want to mark the page as accessed if we've > scanned it. Ah, that make sense. I'll take the v3 for a ride tomorrow morning. Ben
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). v2: Fix the handling of the NFSv3/v4 directory verifier Trond Myklebust (16): 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 NFS: Cleanup to remove nfs_readdir_descriptor_t typedef NFS: Allow the NFS generic code to pass in a verifier to readdir NFS: Handle NFS4ERR_NOT_SAME and NFSERR_BADCOOKIE from readdir calls NFS: Improve handling of directory verifiers fs/nfs/client.c | 4 +- fs/nfs/dir.c | 609 ++++++++++++++++++++++++---------------- fs/nfs/inode.c | 7 - fs/nfs/internal.h | 6 - fs/nfs/nfs3proc.c | 35 ++- fs/nfs/nfs4proc.c | 40 +-- fs/nfs/proc.c | 18 +- include/linux/nfs_fs.h | 9 +- include/linux/nfs_xdr.h | 17 +- 9 files changed, 439 insertions(+), 306 deletions(-)