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. -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@xxxxxxxxxxxxxxx
