On 29 Sep 2021, at 12:23, Trond Myklebust wrote: > On Wed, 2021-09-29 at 10:56 -0400, Benjamin Coddington wrote: >> On 29 Sep 2021, at 9:49, trondmy@xxxxxxxxxx wrote: >> >>> From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> >>> >>> If a user is doing 'ls -l', we have a heuristic in GETATTR that >>> tells >>> the readdir code to try to use READDIRPLUS in order to refresh the >>> inode >>> attributes. In certain cirumstances, we also try to invalidate the >>> remaining directory entries in order to ensure this refresh. >>> >>> If there are multiple readers of the directory, we probably should >>> avoid >>> invalidating the page cache, since the heuristic breaks down in >>> that >>> situation anyway. >> >> Hi Trond, I'm curious about the motivation for this work because >> we're often >> managing expectations about performance between various workloads. >> What >> does the workload look like that prompted you to make this >> optimization? >> >> I'm also interested because I have some work that improves readdir >> performance for multiple readers on large directories, but is rotting >> because things have been "good enough" for folks over here. >> > > Are you talking about this patch specifically, or the series in > general? A bit of both - the first two patches didn't really make sense to me, but I get it now. Thanks. > The general motivation for the series is that in certain circumstances > involving a read-only scenario (no changes to the directory) and > multiple processes we're seeing a regression w.r.t. older kernels. > > The motivation for this particular patch is twofold: > 1. I want to get rid of the cached page_index in the inode and > replace it with something less persistent (inodes are forever, > unlike file descriptors). > 2. The heuristic in question is designed to only work in the single > process/single threaded case. Make sense to me now. I'm wondering if this might be creating a READDIR op amplification for N readers sharing the directory's fd because one process can be dropping the cache, which artificially deflates desc->page_index for a given dir_cookie. That lower page_index gets reused on the next pass dropping the cache, and it'll keep using the bottom pages and doing READDIRs. That wasn't possible before because we only invalidated past nfsi->page_index. Maybe an unlikely case (but I think some http servers could behave this way), and I'd have to test it to be sure. I can try to do that unless you think its not possible or concerning. Ben
