public inbox for v9fs@lists.linux.dev
 help / color / mirror / Atom feed
From: Remi Pommarel <repk@triplefau.lt>
To: Christian Schoenebeck <linux_oss@crudebyte.com>
Cc: v9fs@lists.linux.dev, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Eric Van Hensbergen <ericvh@kernel.org>,
	Latchesar Ionkov <lucho@ionkov.net>,
	Dominique Martinet <asmadeus@codewreck.org>
Subject: Re: [PATCH v2 1/3] 9p: Cache negative dentries for lookup performance
Date: Sat, 21 Feb 2026 21:35:52 +0100	[thread overview]
Message-ID: <aZoXKN8pPOqEBBBz@pilgrim> (raw)
In-Reply-To: <aY2aXG4ljulj1QRh@pilgrim>

On Thu, Feb 12, 2026 at 10:16:13AM +0100, Remi Pommarel wrote:
> On Wed, Feb 11, 2026 at 04:49:19PM +0100, Christian Schoenebeck wrote:
> > On Wednesday, 21 January 2026 20:56:08 CET Remi Pommarel wrote:
> > > Not caching negative dentries can result in poor performance for
> > > workloads that repeatedly look up non-existent paths. Each such
> > > lookup triggers a full 9P transaction with the server, adding
> > > unnecessary overhead.
> > > 
> > > A typical example is source compilation, where multiple cc1 processes
> > > are spawned and repeatedly search for the same missing header files
> > > over and over again.
> > > 
> > > This change enables caching of negative dentries, so that lookups for
> > > known non-existent paths do not require a full 9P transaction. The
> > > cached negative dentries are retained for a configurable duration
> > > (expressed in milliseconds), as specified by the ndentry_timeout
> > > field in struct v9fs_session_info. If set to -1, negative dentries
> > > are cached indefinitely.
> > > 
> > > This optimization reduces lookup overhead and improves performance for
> > > workloads involving frequent access to non-existent paths.
> > > 
> > > Signed-off-by: Remi Pommarel <repk@triplefau.lt>
> > > ---
> > >  fs/9p/fid.c             |  11 +++--
> > >  fs/9p/v9fs.c            |   1 +
> > >  fs/9p/v9fs.h            |   2 +
> > >  fs/9p/v9fs_vfs.h        |  15 ++++++
> > >  fs/9p/vfs_dentry.c      | 105 ++++++++++++++++++++++++++++++++++------
> > >  fs/9p/vfs_inode.c       |   7 +--
> > >  fs/9p/vfs_super.c       |   1 +
> > >  include/net/9p/client.h |   2 +
> > >  8 files changed, 122 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/fs/9p/fid.c b/fs/9p/fid.c
> > > index f84412290a30..76242d450aa7 100644
> > > --- a/fs/9p/fid.c
> > > +++ b/fs/9p/fid.c
> > > @@ -20,7 +20,9 @@
> > > 
> > >  static inline void __add_fid(struct dentry *dentry, struct p9_fid *fid)
> > >  {
> > > -	hlist_add_head(&fid->dlist, (struct hlist_head *)&dentry->d_fsdata);
> > > +	struct v9fs_dentry *v9fs_dentry = to_v9fs_dentry(dentry);
> > > +
> > > +	hlist_add_head(&fid->dlist, &v9fs_dentry->head);
> > >  }
> > > 
> > > 
> > > @@ -112,6 +114,7 @@ void v9fs_open_fid_add(struct inode *inode, struct
> > > p9_fid **pfid)
> > > 
> > >  static struct p9_fid *v9fs_fid_find(struct dentry *dentry, kuid_t uid, int
> > > any) {
> > > +	struct v9fs_dentry *v9fs_dentry = to_v9fs_dentry(dentry);
> > >  	struct p9_fid *fid, *ret;
> > > 
> > >  	p9_debug(P9_DEBUG_VFS, " dentry: %pd (%p) uid %d any %d\n",
> > > @@ -119,11 +122,9 @@ static struct p9_fid *v9fs_fid_find(struct dentry
> > > *dentry, kuid_t uid, int any) any);
> > >  	ret = NULL;
> > >  	/* we'll recheck under lock if there's anything to look in */
> > > -	if (dentry->d_fsdata) {
> > > -		struct hlist_head *h = (struct hlist_head *)&dentry->d_fsdata;
> > > -
> > > +	if (!hlist_empty(&v9fs_dentry->head)) {
> > >  		spin_lock(&dentry->d_lock);
> > > -		hlist_for_each_entry(fid, h, dlist) {
> > > +		hlist_for_each_entry(fid, &v9fs_dentry->head, dlist) {
> > >  			if (any || uid_eq(fid->uid, uid)) {
> > >  				ret = fid;
> > >  				p9_fid_get(ret);
> > > diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
> > > index 057487efaaeb..1da7ab186478 100644
> > > --- a/fs/9p/v9fs.c
> > > +++ b/fs/9p/v9fs.c
> > > @@ -422,6 +422,7 @@ static void v9fs_apply_options(struct v9fs_session_info
> > > *v9ses, v9ses->cache = ctx->session_opts.cache;
> > >  	v9ses->uid = ctx->session_opts.uid;
> > >  	v9ses->session_lock_timeout = ctx->session_opts.session_lock_timeout;
> > > +	v9ses->ndentry_timeout = ctx->session_opts.ndentry_timeout;
> > >  }
> > > 
> > >  /**
> > > diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
> > > index 6a12445d3858..99d1a0ff3368 100644
> > > --- a/fs/9p/v9fs.h
> > > +++ b/fs/9p/v9fs.h
> > > @@ -91,6 +91,7 @@ enum p9_cache_bits {
> > >   * @debug: debug level
> > >   * @afid: authentication handle
> > >   * @cache: cache mode of type &p9_cache_bits
> > > + * @ndentry_timeout: Negative dentry lookup cache retention time in ms
> > >   * @cachetag: the tag of the cache associated with this session
> > >   * @fscache: session cookie associated with FS-Cache
> > >   * @uname: string user name to mount hierarchy as
> > > @@ -116,6 +117,7 @@ struct v9fs_session_info {
> > >  	unsigned short debug;
> > >  	unsigned int afid;
> > >  	unsigned int cache;
> > > +	unsigned int ndentry_timeout;
> > 
> > Why not (signed) long?
> 
> I first though 40+ days of cache retention was enough but that is just
> an useless limitation, I will change it to signed long.

Well now that I think about it, this is supposed to be set from a mount
options. However, with the new mount API in use, there is currently no
support for fsparam_long or fsparam_s64.

While it could be implemented using a custom __fsparam, is the effort
truly justified here? Also in that case maybe a long long would be a bit
more portable across 32-bit and 64-bit platform?

Thanks,

-- 
Remi

  parent reply	other threads:[~2026-02-21 20:57 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-21 19:56 [PATCH v2 0/3] 9p: Performance improvements for build workloads Remi Pommarel
2026-01-21 19:56 ` [PATCH v2 1/3] 9p: Cache negative dentries for lookup performance Remi Pommarel
2026-02-11 15:49   ` Christian Schoenebeck
2026-02-12  9:16     ` Remi Pommarel
2026-02-18 12:46       ` Christian Schoenebeck
2026-02-21 20:35       ` Remi Pommarel [this message]
2026-02-23 14:45         ` Christian Schoenebeck
2026-01-21 19:56 ` [PATCH v2 2/3] 9p: Introduce option for negative dentry cache retention time Remi Pommarel
2026-02-11 15:58   ` Christian Schoenebeck
2026-02-12  9:24     ` Remi Pommarel
2026-02-18 12:56       ` Christian Schoenebeck
2026-01-21 19:56 ` [PATCH v2 3/3] 9p: Enable symlink caching in page cache Remi Pommarel
2026-02-12 15:35   ` Christian Schoenebeck
2026-02-12 21:42     ` Remi Pommarel
2026-02-15 12:36       ` Dominique Martinet
2026-02-19 10:18         ` Christian Schoenebeck
2026-01-21 23:23 ` [PATCH v2 0/3] 9p: Performance improvements for build workloads Dominique Martinet
2026-02-04 11:37 ` Christian Schoenebeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aZoXKN8pPOqEBBBz@pilgrim \
    --to=repk@triplefau.lt \
    --cc=asmadeus@codewreck.org \
    --cc=ericvh@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux_oss@crudebyte.com \
    --cc=lucho@ionkov.net \
    --cc=v9fs@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox