Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Benjamin Coddington" <bcodding@redhat.com>
To:     trondmy@kernel.org
Cc:     linux-nfs@vger.kernel.org
Subject: Re: [PATCH v7 19/21] NFS: Convert readdir page cache to use a cookie
 based index
Date:   Thu, 24 Feb 2022 12:31:56 -0500
Message-ID: <EF67F180-F1D8-4291-92C8-86E5D10D1F25@redhat.com>
In-Reply-To: <20220223211305.296816-20-trondmy@kernel.org>
References: <20220223211305.296816-1-trondmy@kernel.org>
 <20220223211305.296816-2-trondmy@kernel.org>
 <20220223211305.296816-3-trondmy@kernel.org>
 <20220223211305.296816-4-trondmy@kernel.org>
 <20220223211305.296816-5-trondmy@kernel.org>
 <20220223211305.296816-6-trondmy@kernel.org>
 <20220223211305.296816-7-trondmy@kernel.org>
 <20220223211305.296816-8-trondmy@kernel.org>
 <20220223211305.296816-9-trondmy@kernel.org>
 <20220223211305.296816-10-trondmy@kernel.org>
 <20220223211305.296816-11-trondmy@kernel.org>
 <20220223211305.296816-12-trondmy@kernel.org>
 <20220223211305.296816-13-trondmy@kernel.org>
 <20220223211305.296816-14-trondmy@kernel.org>
 <20220223211305.296816-15-trondmy@kernel.org>
 <20220223211305.296816-16-trondmy@kernel.org>
 <20220223211305.296816-17-trondmy@kernel.org>
 <20220223211305.296816-18-trondmy@kernel.org>
 <20220223211305.296816-19-trondmy@kernel.org>
 <20220223211305.296816-20-trondmy@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

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?

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.

Hmm..

Ben