Re: [PATCH v7 19/21] NFS: Convert readdir page cache to use a cookie based index

[Trond Myklebust <trondmy@hammerspace.com>]

On Thu, 2022-02-24 at 12:31 -0500, Benjamin Coddington wrote:
> On 23 Feb 2022, at 16:13, trondmy@kernel.org wrote:
> 
> > From: Trond Myklebust <trond.myklebust@hammerspace.com>
> > 
> > Instead of using a linear index to address the pages, use the
> > cookie of
> > the first entry, since that is what we use to match the page
> > anyway.
> > 
> > This allows us to avoid re-reading the entire cache on a seekdir()
> > type
> > of operation. The latter is very common when re-exporting NFS, and
> > is a
> > major performance drain.
> > 
> > The change does affect our duplicate cookie detection, since we can
> > no
> > longer rely on the page index as a linear offset for detecting
> > whether
> > we looped backwards. However since we no longer do a linear search
> > through all the pages on each call to nfs_readdir(), this is less
> > of a
> > concern than it was previously.
> > The other downside is that invalidate_mapping_pages() no longer can
> > use
> > the page index to avoid clearing pages that have been read. A
> > subsequent
> > patch will restore the functionality this provides to the 'ls -l'
> > heuristic.
> 
> This is cool, but one reason I did not explore this was that the page
> cache
> index uses XArray, which is optimized for densly clustered indexes. 
> This
> particular sentence in the documentation was enough to scare me away:
> 
> "The XArray implementation is efficient when the indices used are
> densely
> clustered; hashing the object and using the hash as the index will
> not
> perform well."
> 
> However, the "not perform well" may be orders of magnitude smaller
> than
> anthing like RPC.  Do you have concerns about this?

What is the difference between this workload and a random access
database workload?

If the XArray is incapable of dealing with random access, then we
should never have chosen it for the page cache. I'm therefore assuming
that either the above comment is referring to micro-optimisations that
don't matter much with these workloads, or else that the plan is to
replace the XArray with something more appropriate for a page cache
workload.


> 
> Another option might be to flag the context after a seekdir, which
> would
> trigger a shift in the page_index or "turn on" hashed indexes,
> however
> that's really only going to improve the re-export case with v4 or
> cached
> fds.
> 
> Or maybe the /first/ seekdir on a context sets its own offset into
> the
> pagecache - that could be a hash, and pages are filled from there.
> 

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com