All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Schoenebeck <linux_oss@crudebyte.com>
To: Remi Pommarel <repk@triplefau.lt>,
	Dominique Martinet <asmadeus@codewreck.org>
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>
Subject: Re: [PATCH v2 1/3] 9p: Cache negative dentries for lookup performance
Date: Wed, 18 Feb 2026 13:46:26 +0100	[thread overview]
Message-ID: <4714422.LvFx2qVVIh@weasel> (raw)
In-Reply-To: <aY2aXG4ljulj1QRh@pilgrim>

On Thursday, 12 February 2026 10:16:12 CET 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:
[...]
> > > diff --git a/fs/9p/vfs_dentry.c b/fs/9p/vfs_dentry.c
> > > index c5bf74d547e8..90291cf0a34b 100644
> > > --- a/fs/9p/vfs_dentry.c
> > > +++ b/fs/9p/vfs_dentry.c
> > > @@ -23,6 +23,46 @@
> > > 
> > >  #include "v9fs_vfs.h"
> > >  #include "fid.h"
> > > 
> > > +/**
> > > + * v9fs_dentry_is_expired - Check if dentry lookup has expired
> > > + *
> > > + * This should be called to know if a negative dentry should be removed
> > > from + * cache.
> > > + *
> > > + * @dentry: dentry in question
> > > + *
> > > + */
> > > +static bool v9fs_dentry_is_expired(struct dentry const *dentry)
> > > +{
> > > +	struct v9fs_session_info *v9ses = v9fs_dentry2v9ses(dentry);
> > > +	struct v9fs_dentry *v9fs_dentry = to_v9fs_dentry(dentry);
> > > +
> > > +	if (v9ses->ndentry_timeout == -1)
> > > +		return false;
> > > +
> > > +	return time_before_eq64(v9fs_dentry->expire_time, get_jiffies_64());
> > > +}
> > 
> > v9fs_negative_dentry_is_expired() ?
> > 
> > Or is there a plan to use this for regular dentries, say with cache=loose
> > in future?
> 
> Yes I wanted to let the possibility for dentry cache expiration open,
> maybe this could be a nice thing to have ?

Fine either way, I leave it up to you.

> > > +
> > > +/**
> > > + * v9fs_dentry_refresh - Refresh dentry lookup cache timeout
> > > + *
> > > + * This should be called when a look up yields a negative entry.
> > > + *
> > > + * @dentry: dentry in question
> > > + *
> > > + */
> > > +void v9fs_dentry_refresh(struct dentry *dentry)
> > > +{
> > > +	struct v9fs_session_info *v9ses = v9fs_dentry2v9ses(dentry);
> > > +	struct v9fs_dentry *v9fs_dentry = to_v9fs_dentry(dentry);
> > > +
> > > +	if (v9ses->ndentry_timeout == -1)
> > > +		return;
> > > +
> > > +	v9fs_dentry->expire_time = get_jiffies_64() +
> > > +				   msecs_to_jiffies(v9ses->ndentry_timeout);
> > > +}
> > 
> > v9fs_negative_dentry_refresh_timeout() ?

Nevertheless I would rename this function to something that at least contains
"timeout" in its name, as v9fs_dentry_refresh() is somewhat too generic IMO.

> > 
> > > +
> > > 
> > >  /**
> > >  
> > >   * v9fs_cached_dentry_delete - called when dentry refcount equals 0
> > >   * @dentry:  dentry in question
> > > 
> > > @@ -33,20 +73,15 @@ static int v9fs_cached_dentry_delete(const struct
> > > dentry *dentry) p9_debug(P9_DEBUG_VFS, " dentry: %pd (%p)\n",
> > > 
> > >  		 dentry, dentry);
> > > 
> > > -	/* Don't cache negative dentries */
> > > -	if (d_really_is_negative(dentry))
> > > -		return 1;
> > > -	return 0;
> > > -}
> > > +	if (!d_really_is_negative(dentry))
> > > +		return 0;
> > 
> > Is it worth a check for v9ses->ndentry_timeout != 0 here?
> 
> The check will be done in v9fs_dentry_is_expired() not sure this is
> worth the optimization here ?

Right, that's OK.

Overall I think it makes sense to bring this series forward. The improvement
is really impressive.

Thanks!

/Christian



  reply	other threads:[~2026-02-18 12:46 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 [this message]
2026-02-21 20:35       ` Remi Pommarel
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=4714422.LvFx2qVVIh@weasel \
    --to=linux_oss@crudebyte.com \
    --cc=asmadeus@codewreck.org \
    --cc=ericvh@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lucho@ionkov.net \
    --cc=repk@triplefau.lt \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.