* [PATCH v2 1/3] 9p: Cache negative dentries for lookup performance
2026-01-21 19:56 [PATCH v2 0/3] 9p: Performance improvements for build workloads Remi Pommarel
@ 2026-01-21 19:56 ` Remi Pommarel
2026-02-11 15:49 ` Christian Schoenebeck
2026-01-21 19:56 ` [PATCH v2 2/3] 9p: Introduce option for negative dentry cache retention time Remi Pommarel
` (3 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Remi Pommarel @ 2026-01-21 19:56 UTC (permalink / raw)
To: v9fs
Cc: linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet, Christian Schoenebeck,
Remi Pommarel
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;
#ifdef CONFIG_9P_FSCACHE
char *cachetag;
struct fscache_volume *fscache;
diff --git a/fs/9p/v9fs_vfs.h b/fs/9p/v9fs_vfs.h
index d3aefbec4de6..7e6e8881081c 100644
--- a/fs/9p/v9fs_vfs.h
+++ b/fs/9p/v9fs_vfs.h
@@ -28,6 +28,19 @@
/* flags for v9fs_stat2inode() & v9fs_stat2inode_dotl() */
#define V9FS_STAT2INODE_KEEP_ISIZE 1
+/**
+ * struct v9fs_dentry - v9fs specific dentry data
+ * @head: List of fid associated with this dentry
+ * @expire_time: Lookup cache expiration time for negative dentries
+ * @rcu: used by kfree_rcu to schedule clean up job
+ */
+struct v9fs_dentry {
+ struct hlist_head head;
+ u64 expire_time;
+ struct rcu_head rcu;
+};
+#define to_v9fs_dentry(d) ((struct v9fs_dentry *)((d)->d_fsdata))
+
extern struct file_system_type v9fs_fs_type;
extern const struct address_space_operations v9fs_addr_operations;
extern const struct file_operations v9fs_file_operations;
@@ -35,6 +48,8 @@ extern const struct file_operations v9fs_file_operations_dotl;
extern const struct file_operations v9fs_dir_operations;
extern const struct file_operations v9fs_dir_operations_dotl;
extern const struct dentry_operations v9fs_dentry_operations;
+extern void v9fs_dentry_refresh(struct dentry *dentry);
+extern void v9fs_dentry_fid_remove(struct dentry *dentry);
extern const struct dentry_operations v9fs_cached_dentry_operations;
extern struct kmem_cache *v9fs_inode_cache;
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_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_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;
-/**
- * v9fs_dentry_release - called when dentry is going to be freed
- * @dentry: dentry that is being release
- *
- */
+ return v9fs_dentry_is_expired(dentry);
+}
-static void v9fs_dentry_release(struct dentry *dentry)
+static void __v9fs_dentry_fid_remove(struct dentry *dentry)
{
+ struct v9fs_dentry *v9fs_dentry = to_v9fs_dentry(dentry);
struct hlist_node *p, *n;
struct hlist_head head;
@@ -54,13 +89,54 @@ static void v9fs_dentry_release(struct dentry *dentry)
dentry, dentry);
spin_lock(&dentry->d_lock);
- hlist_move_list((struct hlist_head *)&dentry->d_fsdata, &head);
+ hlist_move_list(&v9fs_dentry->head, &head);
spin_unlock(&dentry->d_lock);
hlist_for_each_safe(p, n, &head)
p9_fid_put(hlist_entry(p, struct p9_fid, dlist));
}
+/**
+ * v9fs_dentry_fid_remove - Release all dentry's fid
+ * @dentry: dentry in question
+ *
+ */
+void v9fs_dentry_fid_remove(struct dentry *dentry)
+{
+ __v9fs_dentry_fid_remove(dentry);
+}
+
+/**
+ * v9fs_dentry_init - Initialize v9fs dentry data
+ * @dentry: dentry in question
+ *
+ */
+static int v9fs_dentry_init(struct dentry *dentry)
+{
+ struct v9fs_dentry *v9fs_dentry = kzalloc(sizeof(*v9fs_dentry),
+ GFP_KERNEL);
+
+ if (!v9fs_dentry)
+ return -ENOMEM;
+
+ INIT_HLIST_HEAD(&v9fs_dentry->head);
+ dentry->d_fsdata = (void *)v9fs_dentry;
+ return 0;
+}
+
+/**
+ * v9fs_dentry_release - called when dentry is going to be freed
+ * @dentry: dentry that is being release
+ *
+ */
+static void v9fs_dentry_release(struct dentry *dentry)
+{
+ struct v9fs_dentry *v9fs_dentry = to_v9fs_dentry(dentry);
+
+ __v9fs_dentry_fid_remove(dentry);
+ kfree_rcu(v9fs_dentry, rcu);
+}
+
static int __v9fs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
{
struct p9_fid *fid;
@@ -72,7 +148,7 @@ static int __v9fs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
inode = d_inode(dentry);
if (!inode)
- goto out_valid;
+ return !v9fs_dentry_is_expired(dentry);
v9inode = V9FS_I(inode);
if (v9inode->cache_validity & V9FS_INO_INVALID_ATTR) {
@@ -112,7 +188,6 @@ static int __v9fs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
return retval;
}
}
-out_valid:
p9_debug(P9_DEBUG_VFS, "dentry: %pd (%p) is valid\n", dentry, dentry);
return 1;
}
@@ -139,12 +214,14 @@ const struct dentry_operations v9fs_cached_dentry_operations = {
.d_revalidate = v9fs_lookup_revalidate,
.d_weak_revalidate = __v9fs_lookup_revalidate,
.d_delete = v9fs_cached_dentry_delete,
+ .d_init = v9fs_dentry_init,
.d_release = v9fs_dentry_release,
.d_unalias_trylock = v9fs_dentry_unalias_trylock,
.d_unalias_unlock = v9fs_dentry_unalias_unlock,
};
const struct dentry_operations v9fs_dentry_operations = {
+ .d_init = v9fs_dentry_init,
.d_release = v9fs_dentry_release,
.d_unalias_trylock = v9fs_dentry_unalias_trylock,
.d_unalias_unlock = v9fs_dentry_unalias_unlock,
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 0f3189a0a516..a82a71be309b 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -549,7 +549,7 @@ static int v9fs_remove(struct inode *dir, struct dentry *dentry, int flags)
/* invalidate all fids associated with dentry */
/* NOTE: This will not include open fids */
- dentry->d_op->d_release(dentry);
+ v9fs_dentry_fid_remove(dentry);
}
return retval;
}
@@ -732,9 +732,10 @@ struct dentry *v9fs_vfs_lookup(struct inode *dir, struct dentry *dentry,
name = dentry->d_name.name;
fid = p9_client_walk(dfid, 1, &name, 1);
p9_fid_put(dfid);
- if (fid == ERR_PTR(-ENOENT))
+ if (fid == ERR_PTR(-ENOENT)) {
inode = NULL;
- else if (IS_ERR(fid))
+ v9fs_dentry_refresh(dentry);
+ } else if (IS_ERR(fid))
inode = ERR_CAST(fid);
else if (v9ses->cache & (CACHE_META|CACHE_LOOSE))
inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb);
diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 315336de6f02..9d9360f9e502 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -327,6 +327,7 @@ static int v9fs_init_fs_context(struct fs_context *fc)
ctx->session_opts.uid = INVALID_UID;
ctx->session_opts.dfltuid = V9FS_DEFUID;
ctx->session_opts.dfltgid = V9FS_DEFGID;
+ ctx->session_opts.ndentry_timeout = 0;
/* initialize client options */
ctx->client_opts.proto_version = p9_proto_2000L;
diff --git a/include/net/9p/client.h b/include/net/9p/client.h
index 838a94218b59..3d2483db9259 100644
--- a/include/net/9p/client.h
+++ b/include/net/9p/client.h
@@ -192,6 +192,7 @@ struct p9_rdma_opts {
* @dfltgid: default numeric groupid to mount hierarchy as
* @uid: if %V9FS_ACCESS_SINGLE, the numeric uid which mounted the hierarchy
* @session_lock_timeout: retry interval for blocking locks
+ * @ndentry_timeout: Negative dentry lookup cache retention time in ms
*
* This strucure holds options which are parsed and will be transferred
* to the v9fs_session_info structure when mounted, and therefore largely
@@ -212,6 +213,7 @@ struct p9_session_opts {
kgid_t dfltgid;
kuid_t uid;
long session_lock_timeout;
+ unsigned int ndentry_timeout;
};
/* Used by mount API to store parsed mount options */
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 1/3] 9p: Cache negative dentries for lookup performance
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
0 siblings, 1 reply; 18+ messages in thread
From: Christian Schoenebeck @ 2026-02-11 15:49 UTC (permalink / raw)
To: v9fs, Remi Pommarel
Cc: linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet, Remi Pommarel
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?
> #ifdef CONFIG_9P_FSCACHE
> char *cachetag;
> struct fscache_volume *fscache;
> diff --git a/fs/9p/v9fs_vfs.h b/fs/9p/v9fs_vfs.h
> index d3aefbec4de6..7e6e8881081c 100644
> --- a/fs/9p/v9fs_vfs.h
> +++ b/fs/9p/v9fs_vfs.h
> @@ -28,6 +28,19 @@
> /* flags for v9fs_stat2inode() & v9fs_stat2inode_dotl() */
> #define V9FS_STAT2INODE_KEEP_ISIZE 1
>
> +/**
> + * struct v9fs_dentry - v9fs specific dentry data
> + * @head: List of fid associated with this dentry
> + * @expire_time: Lookup cache expiration time for negative dentries
> + * @rcu: used by kfree_rcu to schedule clean up job
> + */
> +struct v9fs_dentry {
> + struct hlist_head head;
> + u64 expire_time;
> + struct rcu_head rcu;
> +};
> +#define to_v9fs_dentry(d) ((struct v9fs_dentry *)((d)->d_fsdata))
> +
> extern struct file_system_type v9fs_fs_type;
> extern const struct address_space_operations v9fs_addr_operations;
> extern const struct file_operations v9fs_file_operations;
> @@ -35,6 +48,8 @@ extern const struct file_operations
> v9fs_file_operations_dotl; extern const struct file_operations
> v9fs_dir_operations;
> extern const struct file_operations v9fs_dir_operations_dotl;
> extern const struct dentry_operations v9fs_dentry_operations;
> +extern void v9fs_dentry_refresh(struct dentry *dentry);
> +extern void v9fs_dentry_fid_remove(struct dentry *dentry);
> extern const struct dentry_operations v9fs_cached_dentry_operations;
> extern struct kmem_cache *v9fs_inode_cache;
>
> 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?
> +
> +/**
> + * 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() ?
> +
> /**
> * 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?
>
> -/**
> - * v9fs_dentry_release - called when dentry is going to be freed
> - * @dentry: dentry that is being release
> - *
> - */
> + return v9fs_dentry_is_expired(dentry);
> +}
"... is being released"
>
> -static void v9fs_dentry_release(struct dentry *dentry)
> +static void __v9fs_dentry_fid_remove(struct dentry *dentry)
> {
> + struct v9fs_dentry *v9fs_dentry = to_v9fs_dentry(dentry);
> struct hlist_node *p, *n;
> struct hlist_head head;
>
> @@ -54,13 +89,54 @@ static void v9fs_dentry_release(struct dentry *dentry)
> dentry, dentry);
>
> spin_lock(&dentry->d_lock);
> - hlist_move_list((struct hlist_head *)&dentry->d_fsdata, &head);
> + hlist_move_list(&v9fs_dentry->head, &head);
> spin_unlock(&dentry->d_lock);
>
> hlist_for_each_safe(p, n, &head)
> p9_fid_put(hlist_entry(p, struct p9_fid, dlist));
> }
>
> +/**
> + * v9fs_dentry_fid_remove - Release all dentry's fid
> + * @dentry: dentry in question
> + *
> + */
> +void v9fs_dentry_fid_remove(struct dentry *dentry)
> +{
> + __v9fs_dentry_fid_remove(dentry);
> +}
" ... all dentry's fids" ?
> +
> +/**
> + * v9fs_dentry_init - Initialize v9fs dentry data
> + * @dentry: dentry in question
> + *
> + */
> +static int v9fs_dentry_init(struct dentry *dentry)
> +{
> + struct v9fs_dentry *v9fs_dentry = kzalloc(sizeof(*v9fs_dentry),
> + GFP_KERNEL);
> +
> + if (!v9fs_dentry)
> + return -ENOMEM;
> +
> + INIT_HLIST_HEAD(&v9fs_dentry->head);
> + dentry->d_fsdata = (void *)v9fs_dentry;
> + return 0;
> +}
> +
> +/**
> + * v9fs_dentry_release - called when dentry is going to be freed
> + * @dentry: dentry that is being release
> + *
> + */
> +static void v9fs_dentry_release(struct dentry *dentry)
> +{
> + struct v9fs_dentry *v9fs_dentry = to_v9fs_dentry(dentry);
> +
> + __v9fs_dentry_fid_remove(dentry);
> + kfree_rcu(v9fs_dentry, rcu);
> +}
> +
> static int __v9fs_lookup_revalidate(struct dentry *dentry, unsigned int
> flags) {
> struct p9_fid *fid;
> @@ -72,7 +148,7 @@ static int __v9fs_lookup_revalidate(struct dentry
> *dentry, unsigned int flags)
>
> inode = d_inode(dentry);
> if (!inode)
> - goto out_valid;
> + return !v9fs_dentry_is_expired(dentry);
>
> v9inode = V9FS_I(inode);
> if (v9inode->cache_validity & V9FS_INO_INVALID_ATTR) {
> @@ -112,7 +188,6 @@ static int __v9fs_lookup_revalidate(struct dentry
> *dentry, unsigned int flags) return retval;
> }
> }
> -out_valid:
> p9_debug(P9_DEBUG_VFS, "dentry: %pd (%p) is valid\n", dentry, dentry);
> return 1;
> }
> @@ -139,12 +214,14 @@ const struct dentry_operations
> v9fs_cached_dentry_operations = { .d_revalidate = v9fs_lookup_revalidate,
> .d_weak_revalidate = __v9fs_lookup_revalidate,
> .d_delete = v9fs_cached_dentry_delete,
> + .d_init = v9fs_dentry_init,
> .d_release = v9fs_dentry_release,
> .d_unalias_trylock = v9fs_dentry_unalias_trylock,
> .d_unalias_unlock = v9fs_dentry_unalias_unlock,
> };
>
> const struct dentry_operations v9fs_dentry_operations = {
> + .d_init = v9fs_dentry_init,
> .d_release = v9fs_dentry_release,
> .d_unalias_trylock = v9fs_dentry_unalias_trylock,
> .d_unalias_unlock = v9fs_dentry_unalias_unlock,
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index 0f3189a0a516..a82a71be309b 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -549,7 +549,7 @@ static int v9fs_remove(struct inode *dir, struct dentry
> *dentry, int flags)
>
> /* invalidate all fids associated with dentry */
> /* NOTE: This will not include open fids */
> - dentry->d_op->d_release(dentry);
> + v9fs_dentry_fid_remove(dentry);
> }
> return retval;
> }
> @@ -732,9 +732,10 @@ struct dentry *v9fs_vfs_lookup(struct inode *dir,
> struct dentry *dentry, name = dentry->d_name.name;
> fid = p9_client_walk(dfid, 1, &name, 1);
> p9_fid_put(dfid);
> - if (fid == ERR_PTR(-ENOENT))
> + if (fid == ERR_PTR(-ENOENT)) {
> inode = NULL;
> - else if (IS_ERR(fid))
> + v9fs_dentry_refresh(dentry);
> + } else if (IS_ERR(fid))
> inode = ERR_CAST(fid);
> else if (v9ses->cache & (CACHE_META|CACHE_LOOSE))
> inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb);
> diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
> index 315336de6f02..9d9360f9e502 100644
> --- a/fs/9p/vfs_super.c
> +++ b/fs/9p/vfs_super.c
> @@ -327,6 +327,7 @@ static int v9fs_init_fs_context(struct fs_context *fc)
> ctx->session_opts.uid = INVALID_UID;
> ctx->session_opts.dfltuid = V9FS_DEFUID;
> ctx->session_opts.dfltgid = V9FS_DEFGID;
> + ctx->session_opts.ndentry_timeout = 0;
>
> /* initialize client options */
> ctx->client_opts.proto_version = p9_proto_2000L;
> diff --git a/include/net/9p/client.h b/include/net/9p/client.h
> index 838a94218b59..3d2483db9259 100644
> --- a/include/net/9p/client.h
> +++ b/include/net/9p/client.h
> @@ -192,6 +192,7 @@ struct p9_rdma_opts {
> * @dfltgid: default numeric groupid to mount hierarchy as
> * @uid: if %V9FS_ACCESS_SINGLE, the numeric uid which mounted the
> hierarchy * @session_lock_timeout: retry interval for blocking locks
> + * @ndentry_timeout: Negative dentry lookup cache retention time in ms
> *
> * This strucure holds options which are parsed and will be transferred
> * to the v9fs_session_info structure when mounted, and therefore largely
> @@ -212,6 +213,7 @@ struct p9_session_opts {
> kgid_t dfltgid;
> kuid_t uid;
> long session_lock_timeout;
> + unsigned int ndentry_timeout;
> };
>
> /* Used by mount API to store parsed mount options */
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 1/3] 9p: Cache negative dentries for lookup performance
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
0 siblings, 2 replies; 18+ messages in thread
From: Remi Pommarel @ 2026-02-12 9:16 UTC (permalink / raw)
To: Christian Schoenebeck
Cc: v9fs, linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet
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.
>
> > #ifdef CONFIG_9P_FSCACHE
> > char *cachetag;
> > struct fscache_volume *fscache;
> > diff --git a/fs/9p/v9fs_vfs.h b/fs/9p/v9fs_vfs.h
> > index d3aefbec4de6..7e6e8881081c 100644
> > --- a/fs/9p/v9fs_vfs.h
> > +++ b/fs/9p/v9fs_vfs.h
> > @@ -28,6 +28,19 @@
> > /* flags for v9fs_stat2inode() & v9fs_stat2inode_dotl() */
> > #define V9FS_STAT2INODE_KEEP_ISIZE 1
> >
> > +/**
> > + * struct v9fs_dentry - v9fs specific dentry data
> > + * @head: List of fid associated with this dentry
> > + * @expire_time: Lookup cache expiration time for negative dentries
> > + * @rcu: used by kfree_rcu to schedule clean up job
> > + */
> > +struct v9fs_dentry {
> > + struct hlist_head head;
> > + u64 expire_time;
> > + struct rcu_head rcu;
> > +};
> > +#define to_v9fs_dentry(d) ((struct v9fs_dentry *)((d)->d_fsdata))
> > +
> > extern struct file_system_type v9fs_fs_type;
> > extern const struct address_space_operations v9fs_addr_operations;
> > extern const struct file_operations v9fs_file_operations;
> > @@ -35,6 +48,8 @@ extern const struct file_operations
> > v9fs_file_operations_dotl; extern const struct file_operations
> > v9fs_dir_operations;
> > extern const struct file_operations v9fs_dir_operations_dotl;
> > extern const struct dentry_operations v9fs_dentry_operations;
> > +extern void v9fs_dentry_refresh(struct dentry *dentry);
> > +extern void v9fs_dentry_fid_remove(struct dentry *dentry);
> > extern const struct dentry_operations v9fs_cached_dentry_operations;
> > extern struct kmem_cache *v9fs_inode_cache;
> >
> > 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 ?
>
> > +
> > +/**
> > + * 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() ?
>
> > +
> > /**
> > * 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 ?
>
> >
> > -/**
> > - * v9fs_dentry_release - called when dentry is going to be freed
> > - * @dentry: dentry that is being release
> > - *
> > - */
> > + return v9fs_dentry_is_expired(dentry);
> > +}
>
> "... is being released"
>
> >
> > -static void v9fs_dentry_release(struct dentry *dentry)
> > +static void __v9fs_dentry_fid_remove(struct dentry *dentry)
> > {
> > + struct v9fs_dentry *v9fs_dentry = to_v9fs_dentry(dentry);
> > struct hlist_node *p, *n;
> > struct hlist_head head;
> >
> > @@ -54,13 +89,54 @@ static void v9fs_dentry_release(struct dentry *dentry)
> > dentry, dentry);
> >
> > spin_lock(&dentry->d_lock);
> > - hlist_move_list((struct hlist_head *)&dentry->d_fsdata, &head);
> > + hlist_move_list(&v9fs_dentry->head, &head);
> > spin_unlock(&dentry->d_lock);
> >
> > hlist_for_each_safe(p, n, &head)
> > p9_fid_put(hlist_entry(p, struct p9_fid, dlist));
> > }
> >
> > +/**
> > + * v9fs_dentry_fid_remove - Release all dentry's fid
> > + * @dentry: dentry in question
> > + *
> > + */
> > +void v9fs_dentry_fid_remove(struct dentry *dentry)
> > +{
> > + __v9fs_dentry_fid_remove(dentry);
> > +}
>
> " ... all dentry's fids" ?
>
> > +
> > +/**
> > + * v9fs_dentry_init - Initialize v9fs dentry data
> > + * @dentry: dentry in question
> > + *
> > + */
> > +static int v9fs_dentry_init(struct dentry *dentry)
> > +{
> > + struct v9fs_dentry *v9fs_dentry = kzalloc(sizeof(*v9fs_dentry),
> > + GFP_KERNEL);
> > +
> > + if (!v9fs_dentry)
> > + return -ENOMEM;
> > +
> > + INIT_HLIST_HEAD(&v9fs_dentry->head);
> > + dentry->d_fsdata = (void *)v9fs_dentry;
> > + return 0;
> > +}
> > +
> > +/**
> > + * v9fs_dentry_release - called when dentry is going to be freed
> > + * @dentry: dentry that is being release
> > + *
> > + */
> > +static void v9fs_dentry_release(struct dentry *dentry)
> > +{
> > + struct v9fs_dentry *v9fs_dentry = to_v9fs_dentry(dentry);
> > +
> > + __v9fs_dentry_fid_remove(dentry);
> > + kfree_rcu(v9fs_dentry, rcu);
> > +}
> > +
> > static int __v9fs_lookup_revalidate(struct dentry *dentry, unsigned int
> > flags) {
> > struct p9_fid *fid;
> > @@ -72,7 +148,7 @@ static int __v9fs_lookup_revalidate(struct dentry
> > *dentry, unsigned int flags)
> >
> > inode = d_inode(dentry);
> > if (!inode)
> > - goto out_valid;
> > + return !v9fs_dentry_is_expired(dentry);
> >
> > v9inode = V9FS_I(inode);
> > if (v9inode->cache_validity & V9FS_INO_INVALID_ATTR) {
> > @@ -112,7 +188,6 @@ static int __v9fs_lookup_revalidate(struct dentry
> > *dentry, unsigned int flags) return retval;
> > }
> > }
> > -out_valid:
> > p9_debug(P9_DEBUG_VFS, "dentry: %pd (%p) is valid\n", dentry, dentry);
> > return 1;
> > }
> > @@ -139,12 +214,14 @@ const struct dentry_operations
> > v9fs_cached_dentry_operations = { .d_revalidate = v9fs_lookup_revalidate,
> > .d_weak_revalidate = __v9fs_lookup_revalidate,
> > .d_delete = v9fs_cached_dentry_delete,
> > + .d_init = v9fs_dentry_init,
> > .d_release = v9fs_dentry_release,
> > .d_unalias_trylock = v9fs_dentry_unalias_trylock,
> > .d_unalias_unlock = v9fs_dentry_unalias_unlock,
> > };
> >
> > const struct dentry_operations v9fs_dentry_operations = {
> > + .d_init = v9fs_dentry_init,
> > .d_release = v9fs_dentry_release,
> > .d_unalias_trylock = v9fs_dentry_unalias_trylock,
> > .d_unalias_unlock = v9fs_dentry_unalias_unlock,
> > diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> > index 0f3189a0a516..a82a71be309b 100644
> > --- a/fs/9p/vfs_inode.c
> > +++ b/fs/9p/vfs_inode.c
> > @@ -549,7 +549,7 @@ static int v9fs_remove(struct inode *dir, struct dentry
> > *dentry, int flags)
> >
> > /* invalidate all fids associated with dentry */
> > /* NOTE: This will not include open fids */
> > - dentry->d_op->d_release(dentry);
> > + v9fs_dentry_fid_remove(dentry);
> > }
> > return retval;
> > }
> > @@ -732,9 +732,10 @@ struct dentry *v9fs_vfs_lookup(struct inode *dir,
> > struct dentry *dentry, name = dentry->d_name.name;
> > fid = p9_client_walk(dfid, 1, &name, 1);
> > p9_fid_put(dfid);
> > - if (fid == ERR_PTR(-ENOENT))
> > + if (fid == ERR_PTR(-ENOENT)) {
> > inode = NULL;
> > - else if (IS_ERR(fid))
> > + v9fs_dentry_refresh(dentry);
> > + } else if (IS_ERR(fid))
> > inode = ERR_CAST(fid);
> > else if (v9ses->cache & (CACHE_META|CACHE_LOOSE))
> > inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb);
> > diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
> > index 315336de6f02..9d9360f9e502 100644
> > --- a/fs/9p/vfs_super.c
> > +++ b/fs/9p/vfs_super.c
> > @@ -327,6 +327,7 @@ static int v9fs_init_fs_context(struct fs_context *fc)
> > ctx->session_opts.uid = INVALID_UID;
> > ctx->session_opts.dfltuid = V9FS_DEFUID;
> > ctx->session_opts.dfltgid = V9FS_DEFGID;
> > + ctx->session_opts.ndentry_timeout = 0;
> >
> > /* initialize client options */
> > ctx->client_opts.proto_version = p9_proto_2000L;
> > diff --git a/include/net/9p/client.h b/include/net/9p/client.h
> > index 838a94218b59..3d2483db9259 100644
> > --- a/include/net/9p/client.h
> > +++ b/include/net/9p/client.h
> > @@ -192,6 +192,7 @@ struct p9_rdma_opts {
> > * @dfltgid: default numeric groupid to mount hierarchy as
> > * @uid: if %V9FS_ACCESS_SINGLE, the numeric uid which mounted the
> > hierarchy * @session_lock_timeout: retry interval for blocking locks
> > + * @ndentry_timeout: Negative dentry lookup cache retention time in ms
> > *
> > * This strucure holds options which are parsed and will be transferred
> > * to the v9fs_session_info structure when mounted, and therefore largely
> > @@ -212,6 +213,7 @@ struct p9_session_opts {
> > kgid_t dfltgid;
> > kuid_t uid;
> > long session_lock_timeout;
> > + unsigned int ndentry_timeout;
> > };
> >
> > /* Used by mount API to store parsed mount options */
Thanks a lot for the review.
--
Remi
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 1/3] 9p: Cache negative dentries for lookup performance
2026-02-12 9:16 ` Remi Pommarel
@ 2026-02-18 12:46 ` Christian Schoenebeck
2026-02-21 20:35 ` Remi Pommarel
1 sibling, 0 replies; 18+ messages in thread
From: Christian Schoenebeck @ 2026-02-18 12:46 UTC (permalink / raw)
To: Remi Pommarel, Dominique Martinet
Cc: v9fs, linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov
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
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 1/3] 9p: Cache negative dentries for lookup performance
2026-02-12 9:16 ` Remi Pommarel
2026-02-18 12:46 ` Christian Schoenebeck
@ 2026-02-21 20:35 ` Remi Pommarel
2026-02-23 14:45 ` Christian Schoenebeck
1 sibling, 1 reply; 18+ messages in thread
From: Remi Pommarel @ 2026-02-21 20:35 UTC (permalink / raw)
To: Christian Schoenebeck
Cc: v9fs, linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet
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
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 1/3] 9p: Cache negative dentries for lookup performance
2026-02-21 20:35 ` Remi Pommarel
@ 2026-02-23 14:45 ` Christian Schoenebeck
0 siblings, 0 replies; 18+ messages in thread
From: Christian Schoenebeck @ 2026-02-23 14:45 UTC (permalink / raw)
To: Remi Pommarel, Dominique Martinet
Cc: v9fs, linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov
On Saturday, 21 February 2026 21:35:52 CET Remi Pommarel wrote:
> 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:
[...]
> > > > 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?
Ah, there is fsparam_u64 but not fsparam_s64 and you are using -1 as option
for "infinite".
There is also `long session_lock_timeout´, but now that I look at it, its mount
option was apparently always limited to 32-bit, even before the new mount API
transition.
I agree, then just leave it as `int´ type now.
Thanks!
/Christian
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 2/3] 9p: Introduce option for negative dentry cache retention time
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-01-21 19:56 ` Remi Pommarel
2026-02-11 15:58 ` Christian Schoenebeck
2026-01-21 19:56 ` [PATCH v2 3/3] 9p: Enable symlink caching in page cache Remi Pommarel
` (2 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Remi Pommarel @ 2026-01-21 19:56 UTC (permalink / raw)
To: v9fs
Cc: linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet, Christian Schoenebeck,
Remi Pommarel
Add support for a new mount option in v9fs that allows users to specify
the duration for which negative dentries are retained in the cache. The
retention time can be set in milliseconds using the ndentrytmo option.
For the same consistency reasons, this option should only be used in
exclusive or read-only mount scenarios, aligning with the cache=loose
usage.
Signed-off-by: Remi Pommarel <repk@triplefau.lt>
---
fs/9p/v9fs.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index 1da7ab186478..f58a2718e412 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -39,7 +39,7 @@ enum {
* source if we rejected it as EINVAL */
Opt_source,
/* Options that take integer arguments */
- Opt_debug, Opt_dfltuid, Opt_dfltgid, Opt_afid,
+ Opt_debug, Opt_dfltuid, Opt_dfltgid, Opt_afid, Opt_ndentrytmo,
/* String options */
Opt_uname, Opt_remotename, Opt_cache, Opt_cachetag,
/* Options that take no arguments */
@@ -93,6 +93,7 @@ const struct fs_parameter_spec v9fs_param_spec[] = {
fsparam_string ("access", Opt_access),
fsparam_flag ("posixacl", Opt_posixacl),
fsparam_u32 ("locktimeout", Opt_locktimeout),
+ fsparam_s32 ("ndentrytmo", Opt_ndentrytmo),
/* client options */
fsparam_u32 ("msize", Opt_msize),
@@ -159,6 +160,8 @@ int v9fs_show_options(struct seq_file *m, struct dentry *root)
from_kgid_munged(&init_user_ns, v9ses->dfltgid));
if (v9ses->afid != ~0)
seq_printf(m, ",afid=%u", v9ses->afid);
+ if (v9ses->ndentry_timeout != 0)
+ seq_printf(m, ",ndentrytmo=%d", v9ses->ndentry_timeout);
if (strcmp(v9ses->uname, V9FS_DEFUSER) != 0)
seq_printf(m, ",uname=%s", v9ses->uname);
if (strcmp(v9ses->aname, V9FS_DEFANAME) != 0)
@@ -337,6 +340,10 @@ int v9fs_parse_param(struct fs_context *fc, struct fs_parameter *param)
session_opts->session_lock_timeout = (long)result.uint_32 * HZ;
break;
+ case Opt_ndentrytmo:
+ session_opts->ndentry_timeout = result.int_32;
+ break;
+
/* Options for client */
case Opt_msize:
if (result.uint_32 < 4096) {
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 2/3] 9p: Introduce option for negative dentry cache retention time
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
0 siblings, 1 reply; 18+ messages in thread
From: Christian Schoenebeck @ 2026-02-11 15:58 UTC (permalink / raw)
To: v9fs, Remi Pommarel
Cc: linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet, Remi Pommarel
On Wednesday, 21 January 2026 20:56:09 CET Remi Pommarel wrote:
> Add support for a new mount option in v9fs that allows users to specify
> the duration for which negative dentries are retained in the cache. The
> retention time can be set in milliseconds using the ndentrytmo option.
>
> For the same consistency reasons, this option should only be used in
> exclusive or read-only mount scenarios, aligning with the cache=loose
> usage.
>
> Signed-off-by: Remi Pommarel <repk@triplefau.lt>
> ---
> fs/9p/v9fs.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
> index 1da7ab186478..f58a2718e412 100644
> --- a/fs/9p/v9fs.c
> +++ b/fs/9p/v9fs.c
> @@ -39,7 +39,7 @@ enum {
> * source if we rejected it as EINVAL */
> Opt_source,
> /* Options that take integer arguments */
> - Opt_debug, Opt_dfltuid, Opt_dfltgid, Opt_afid,
> + Opt_debug, Opt_dfltuid, Opt_dfltgid, Opt_afid, Opt_ndentrytmo,
> /* String options */
> Opt_uname, Opt_remotename, Opt_cache, Opt_cachetag,
> /* Options that take no arguments */
> @@ -93,6 +93,7 @@ const struct fs_parameter_spec v9fs_param_spec[] = {
> fsparam_string ("access", Opt_access),
> fsparam_flag ("posixacl", Opt_posixacl),
> fsparam_u32 ("locktimeout", Opt_locktimeout),
> + fsparam_s32 ("ndentrytmo", Opt_ndentrytmo),
Not better "ndentrytimeout" ?
My first thought was whether it was really worth introducing a dedicated
timeout option exactly for negative dentries (instead of a general cache
timeout option). But on a 2nd thought it actually needs separate handling, as
negative dentries have the potential to pollute with a ridiculous amount of
bogus entries.
Wouldn't it make sense to enable this option with some meaningful value for
say cache=loose by default? 24 hours maybe?
>
> /* client options */
> fsparam_u32 ("msize", Opt_msize),
> @@ -159,6 +160,8 @@ int v9fs_show_options(struct seq_file *m, struct dentry
> *root) from_kgid_munged(&init_user_ns, v9ses->dfltgid));
> if (v9ses->afid != ~0)
> seq_printf(m, ",afid=%u", v9ses->afid);
> + if (v9ses->ndentry_timeout != 0)
> + seq_printf(m, ",ndentrytmo=%d", v9ses->ndentry_timeout);
> if (strcmp(v9ses->uname, V9FS_DEFUSER) != 0)
> seq_printf(m, ",uname=%s", v9ses->uname);
> if (strcmp(v9ses->aname, V9FS_DEFANAME) != 0)
> @@ -337,6 +340,10 @@ int v9fs_parse_param(struct fs_context *fc, struct
> fs_parameter *param) session_opts->session_lock_timeout =
> (long)result.uint_32 * HZ; break;
>
> + case Opt_ndentrytmo:
> + session_opts->ndentry_timeout = result.int_32;
> + break;
> +
> /* Options for client */
> case Opt_msize:
> if (result.uint_32 < 4096) {
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 2/3] 9p: Introduce option for negative dentry cache retention time
2026-02-11 15:58 ` Christian Schoenebeck
@ 2026-02-12 9:24 ` Remi Pommarel
2026-02-18 12:56 ` Christian Schoenebeck
0 siblings, 1 reply; 18+ messages in thread
From: Remi Pommarel @ 2026-02-12 9:24 UTC (permalink / raw)
To: Christian Schoenebeck
Cc: v9fs, linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet
On Wed, Feb 11, 2026 at 04:58:02PM +0100, Christian Schoenebeck wrote:
> On Wednesday, 21 January 2026 20:56:09 CET Remi Pommarel wrote:
> > Add support for a new mount option in v9fs that allows users to specify
> > the duration for which negative dentries are retained in the cache. The
> > retention time can be set in milliseconds using the ndentrytmo option.
> >
> > For the same consistency reasons, this option should only be used in
> > exclusive or read-only mount scenarios, aligning with the cache=loose
> > usage.
> >
> > Signed-off-by: Remi Pommarel <repk@triplefau.lt>
> > ---
> > fs/9p/v9fs.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
> > index 1da7ab186478..f58a2718e412 100644
> > --- a/fs/9p/v9fs.c
> > +++ b/fs/9p/v9fs.c
> > @@ -39,7 +39,7 @@ enum {
> > * source if we rejected it as EINVAL */
> > Opt_source,
> > /* Options that take integer arguments */
> > - Opt_debug, Opt_dfltuid, Opt_dfltgid, Opt_afid,
> > + Opt_debug, Opt_dfltuid, Opt_dfltgid, Opt_afid, Opt_ndentrytmo,
> > /* String options */
> > Opt_uname, Opt_remotename, Opt_cache, Opt_cachetag,
> > /* Options that take no arguments */
> > @@ -93,6 +93,7 @@ const struct fs_parameter_spec v9fs_param_spec[] = {
> > fsparam_string ("access", Opt_access),
> > fsparam_flag ("posixacl", Opt_posixacl),
> > fsparam_u32 ("locktimeout", Opt_locktimeout),
> > + fsparam_s32 ("ndentrytmo", Opt_ndentrytmo),
>
> Not better "ndentrytimeout" ?
I just wanted to avoid to re-align all the above, but lazyness should
not prevail over readability :). I will change that thanks.
>
> My first thought was whether it was really worth introducing a dedicated
> timeout option exactly for negative dentries (instead of a general cache
> timeout option). But on a 2nd thought it actually needs separate handling, as
> negative dentries have the potential to pollute with a ridiculous amount of
> bogus entries.
Agreed.
>
> Wouldn't it make sense to enable this option with some meaningful value for
> say cache=loose by default? 24 hours maybe?
That is an interesting question, I have seen pretty satisfying (at least
for me) perf results on the different builds I ran, even with a 1 to 2
seconds cache timeout, maybe this would be a good tradeoff for
cache=loose being almost transparent in the eye of the user ? But maybe
this is too specific to the build workflow (that hit the same negative
dentries fast enough) ?
>
> >
> > /* client options */
> > fsparam_u32 ("msize", Opt_msize),
> > @@ -159,6 +160,8 @@ int v9fs_show_options(struct seq_file *m, struct dentry
> > *root) from_kgid_munged(&init_user_ns, v9ses->dfltgid));
> > if (v9ses->afid != ~0)
> > seq_printf(m, ",afid=%u", v9ses->afid);
> > + if (v9ses->ndentry_timeout != 0)
> > + seq_printf(m, ",ndentrytmo=%d", v9ses->ndentry_timeout);
> > if (strcmp(v9ses->uname, V9FS_DEFUSER) != 0)
> > seq_printf(m, ",uname=%s", v9ses->uname);
> > if (strcmp(v9ses->aname, V9FS_DEFANAME) != 0)
> > @@ -337,6 +340,10 @@ int v9fs_parse_param(struct fs_context *fc, struct
> > fs_parameter *param) session_opts->session_lock_timeout =
> > (long)result.uint_32 * HZ; break;
> >
> > + case Opt_ndentrytmo:
> > + session_opts->ndentry_timeout = result.int_32;
> > + break;
> > +
> > /* Options for client */
> > case Opt_msize:
> > if (result.uint_32 < 4096) {
>
Thanks,
--
Remi
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 2/3] 9p: Introduce option for negative dentry cache retention time
2026-02-12 9:24 ` Remi Pommarel
@ 2026-02-18 12:56 ` Christian Schoenebeck
0 siblings, 0 replies; 18+ messages in thread
From: Christian Schoenebeck @ 2026-02-18 12:56 UTC (permalink / raw)
To: Remi Pommarel, Dominique Martinet
Cc: v9fs, linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov
On Thursday, 12 February 2026 10:24:27 CET Remi Pommarel wrote:
> On Wed, Feb 11, 2026 at 04:58:02PM +0100, Christian Schoenebeck wrote:
> > On Wednesday, 21 January 2026 20:56:09 CET Remi Pommarel wrote:
[...]
> > Wouldn't it make sense to enable this option with some meaningful value
> > for
> > say cache=loose by default? 24 hours maybe?
>
> That is an interesting question, I have seen pretty satisfying (at least
> for me) perf results on the different builds I ran, even with a 1 to 2
> seconds cache timeout, maybe this would be a good tradeoff for
> cache=loose being almost transparent in the eye of the user ? But maybe
> this is too specific to the build workflow (that hit the same negative
> dentries fast enough) ?
Always hard to pick magic numbers. But I would also say that 1s...2s is
probably a use-case specific pick specifically for compiling sources.
When running 9p as rootfs you will also frequently run into libs querying the
same non-existing configuration files and DLLs over and over again. So I would
pick a higher value. Personally I would be fine with anything between few
minutes ... 24h for cache=loose. For other cache modes this could be lower.
/Christian
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 3/3] 9p: Enable symlink caching in page cache
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-01-21 19:56 ` [PATCH v2 2/3] 9p: Introduce option for negative dentry cache retention time Remi Pommarel
@ 2026-01-21 19:56 ` Remi Pommarel
2026-02-12 15:35 ` 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
4 siblings, 1 reply; 18+ messages in thread
From: Remi Pommarel @ 2026-01-21 19:56 UTC (permalink / raw)
To: v9fs
Cc: linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet, Christian Schoenebeck,
Remi Pommarel
Currently, when cache=loose is enabled, file reads are cached in the
page cache, but symlink reads are not. This patch allows the results
of p9_client_readlink() to be stored in the page cache, eliminating
the need for repeated 9P transactions on subsequent symlink accesses.
This change improves performance for workloads that involve frequent
symlink resolution.
Signed-off-by: Remi Pommarel <repk@triplefau.lt>
---
fs/9p/vfs_addr.c | 24 ++++++++++++--
fs/9p/vfs_inode.c | 6 ++--
fs/9p/vfs_inode_dotl.c | 73 +++++++++++++++++++++++++++++++++++++-----
3 files changed, 90 insertions(+), 13 deletions(-)
diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
index 862164181bac..ee672abbb02c 100644
--- a/fs/9p/vfs_addr.c
+++ b/fs/9p/vfs_addr.c
@@ -70,10 +70,19 @@ static void v9fs_issue_read(struct netfs_io_subrequest *subreq)
{
struct netfs_io_request *rreq = subreq->rreq;
struct p9_fid *fid = rreq->netfs_priv;
+ char *target;
unsigned long long pos = subreq->start + subreq->transferred;
- int total, err;
-
- total = p9_client_read(fid, pos, &subreq->io_iter, &err);
+ int total, err, len, n;
+
+ if (S_ISLNK(rreq->inode->i_mode)) {
+ err = p9_client_readlink(fid, &target);
+ len = strnlen(target, PAGE_SIZE - 1);
+ n = copy_to_iter(target, len, &subreq->io_iter);
+ if (n != len)
+ err = -EFAULT;
+ total = i_size_read(rreq->inode);
+ } else
+ total = p9_client_read(fid, pos, &subreq->io_iter, &err);
/* if we just extended the file size, any portion not in
* cache won't be on server and is zeroes */
@@ -99,6 +108,7 @@ static void v9fs_issue_read(struct netfs_io_subrequest *subreq)
static int v9fs_init_request(struct netfs_io_request *rreq, struct file *file)
{
struct p9_fid *fid;
+ struct dentry *dentry;
bool writing = (rreq->origin == NETFS_READ_FOR_WRITE ||
rreq->origin == NETFS_WRITETHROUGH ||
rreq->origin == NETFS_UNBUFFERED_WRITE ||
@@ -115,6 +125,14 @@ static int v9fs_init_request(struct netfs_io_request *rreq, struct file *file)
if (!fid)
goto no_fid;
p9_fid_get(fid);
+ } else if (S_ISLNK(rreq->inode->i_mode)) {
+ dentry = d_find_alias(rreq->inode);
+ if (!dentry)
+ goto no_fid;
+ fid = v9fs_fid_lookup(dentry);
+ dput(dentry);
+ if (IS_ERR(fid))
+ goto no_fid;
} else {
fid = v9fs_fid_find_inode(rreq->inode, writing, INVALID_UID, true);
if (!fid)
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index a82a71be309b..e1b762f3e081 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -302,10 +302,12 @@ int v9fs_init_inode(struct v9fs_session_info *v9ses,
goto error;
}
- if (v9fs_proto_dotl(v9ses))
+ if (v9fs_proto_dotl(v9ses)) {
inode->i_op = &v9fs_symlink_inode_operations_dotl;
- else
+ inode_nohighmem(inode);
+ } else {
inode->i_op = &v9fs_symlink_inode_operations;
+ }
break;
case S_IFDIR:
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 6312b3590f74..486b11dbada3 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -686,9 +686,13 @@ v9fs_vfs_symlink_dotl(struct mnt_idmap *idmap, struct inode *dir,
int err;
kgid_t gid;
const unsigned char *name;
+ umode_t mode;
+ struct v9fs_session_info *v9ses;
struct p9_qid qid;
struct p9_fid *dfid;
struct p9_fid *fid = NULL;
+ struct inode *inode;
+ struct posix_acl *dacl = NULL, *pacl = NULL;
name = dentry->d_name.name;
p9_debug(P9_DEBUG_VFS, "%lu,%s,%s\n", dir->i_ino, name, symname);
@@ -702,6 +706,15 @@ v9fs_vfs_symlink_dotl(struct mnt_idmap *idmap, struct inode *dir,
gid = v9fs_get_fsgid_for_create(dir);
+ /* Update mode based on ACL value */
+ err = v9fs_acl_mode(dir, &mode, &dacl, &pacl);
+ if (err) {
+ p9_debug(P9_DEBUG_VFS,
+ "Failed to get acl values in symlink %d\n",
+ err);
+ goto error;
+ }
+
/* Server doesn't alter fid on TSYMLINK. Hence no need to clone it. */
err = p9_client_symlink(dfid, name, symname, gid, &qid);
@@ -712,8 +725,30 @@ v9fs_vfs_symlink_dotl(struct mnt_idmap *idmap, struct inode *dir,
v9fs_invalidate_inode_attr(dir);
+ /* instantiate inode and assign the unopened fid to the dentry */
+ fid = p9_client_walk(dfid, 1, &name, 1);
+ if (IS_ERR(fid)) {
+ err = PTR_ERR(fid);
+ p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n",
+ err);
+ goto error;
+ }
+
+ v9ses = v9fs_inode2v9ses(dir);
+ inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
+ if (IS_ERR(inode)) {
+ err = PTR_ERR(inode);
+ p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",
+ err);
+ goto error;
+ }
+ v9fs_set_create_acl(inode, fid, dacl, pacl);
+ v9fs_fid_add(dentry, &fid);
+ d_instantiate(dentry, inode);
+ err = 0;
error:
p9_fid_put(fid);
+ v9fs_put_acl(dacl, pacl);
p9_fid_put(dfid);
return err;
}
@@ -853,24 +888,23 @@ v9fs_vfs_mknod_dotl(struct mnt_idmap *idmap, struct inode *dir,
}
/**
- * v9fs_vfs_get_link_dotl - follow a symlink path
+ * v9fs_vfs_get_link_nocache_dotl - Resolve a symlink directly.
+ *
+ * To be used when symlink caching is not enabled.
+ *
* @dentry: dentry for symlink
* @inode: inode for symlink
* @done: destructor for return value
*/
-
static const char *
-v9fs_vfs_get_link_dotl(struct dentry *dentry,
- struct inode *inode,
- struct delayed_call *done)
+v9fs_vfs_get_link_nocache_dotl(struct dentry *dentry,
+ struct inode *inode,
+ struct delayed_call *done)
{
struct p9_fid *fid;
char *target;
int retval;
- if (!dentry)
- return ERR_PTR(-ECHILD);
-
p9_debug(P9_DEBUG_VFS, "%pd\n", dentry);
fid = v9fs_fid_lookup(dentry);
@@ -884,6 +918,29 @@ v9fs_vfs_get_link_dotl(struct dentry *dentry,
return target;
}
+/**
+ * v9fs_vfs_get_link_dotl - follow a symlink path
+ * @dentry: dentry for symlink
+ * @inode: inode for symlink
+ * @done: destructor for return value
+ */
+static const char *
+v9fs_vfs_get_link_dotl(struct dentry *dentry,
+ struct inode *inode,
+ struct delayed_call *done)
+{
+ struct v9fs_session_info *v9ses;
+
+ if (!dentry)
+ return ERR_PTR(-ECHILD);
+
+ v9ses = v9fs_inode2v9ses(inode);
+ if (v9ses->cache & (CACHE_META|CACHE_LOOSE))
+ return page_get_link(dentry, inode, done);
+
+ return v9fs_vfs_get_link_nocache_dotl(dentry, inode, done);
+}
+
int v9fs_refresh_inode_dotl(struct p9_fid *fid, struct inode *inode)
{
struct p9_stat_dotl *st;
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/3] 9p: Enable symlink caching in page cache
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
0 siblings, 1 reply; 18+ messages in thread
From: Christian Schoenebeck @ 2026-02-12 15:35 UTC (permalink / raw)
To: v9fs, Remi Pommarel
Cc: linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet, Remi Pommarel
On Wednesday, 21 January 2026 20:56:10 CET Remi Pommarel wrote:
> Currently, when cache=loose is enabled, file reads are cached in the
> page cache, but symlink reads are not. This patch allows the results
> of p9_client_readlink() to be stored in the page cache, eliminating
> the need for repeated 9P transactions on subsequent symlink accesses.
>
> This change improves performance for workloads that involve frequent
> symlink resolution.
>
> Signed-off-by: Remi Pommarel <repk@triplefau.lt>
> ---
> fs/9p/vfs_addr.c | 24 ++++++++++++--
> fs/9p/vfs_inode.c | 6 ++--
> fs/9p/vfs_inode_dotl.c | 73 +++++++++++++++++++++++++++++++++++++-----
> 3 files changed, 90 insertions(+), 13 deletions(-)
>
> diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
> index 862164181bac..ee672abbb02c 100644
> --- a/fs/9p/vfs_addr.c
> +++ b/fs/9p/vfs_addr.c
> @@ -70,10 +70,19 @@ static void v9fs_issue_read(struct netfs_io_subrequest
> *subreq) {
> struct netfs_io_request *rreq = subreq->rreq;
> struct p9_fid *fid = rreq->netfs_priv;
> + char *target;
> unsigned long long pos = subreq->start + subreq->transferred;
> - int total, err;
> -
> - total = p9_client_read(fid, pos, &subreq->io_iter, &err);
> + int total, err, len, n;
> +
> + if (S_ISLNK(rreq->inode->i_mode)) {
> + err = p9_client_readlink(fid, &target);
Treadlink request requires 9p2000.L. So this would break with legacy protocol
versions 9p2000 and 9p2000.u I guess:
https://wiki.qemu.org/Documentation/9p#9p_Protocol
> + len = strnlen(target, PAGE_SIZE - 1);
Usually we are bound to PATH_MAX.
Target link path is coming from 9p server, which may run another OS and
therefore target might be longer than PATH_MAX, in which case it would always
yield in -ENAMETOOLONG when client requests that link target from cache.
But OTOH there is no real alternative for storing a link target in cache that
can never be delivered to user. So I guess it is OK as-is.
Simply using PATH_MAX would make it worse, as it would potentially silently
shorten the link from host.
> + n = copy_to_iter(target, len, &subreq->io_iter);
> + if (n != len)
> + err = -EFAULT;
> + total = i_size_read(rreq->inode);
> + } else
> + total = p9_client_read(fid, pos, &subreq->io_iter, &err);
>
> /* if we just extended the file size, any portion not in
> * cache won't be on server and is zeroes */
> @@ -99,6 +108,7 @@ static void v9fs_issue_read(struct netfs_io_subrequest
> *subreq) static int v9fs_init_request(struct netfs_io_request *rreq, struct
> file *file) {
> struct p9_fid *fid;
> + struct dentry *dentry;
> bool writing = (rreq->origin == NETFS_READ_FOR_WRITE ||
> rreq->origin == NETFS_WRITETHROUGH ||
> rreq->origin == NETFS_UNBUFFERED_WRITE ||
> @@ -115,6 +125,14 @@ static int v9fs_init_request(struct netfs_io_request
> *rreq, struct file *file) if (!fid)
> goto no_fid;
> p9_fid_get(fid);
> + } else if (S_ISLNK(rreq->inode->i_mode)) {
> + dentry = d_find_alias(rreq->inode);
> + if (!dentry)
> + goto no_fid;
> + fid = v9fs_fid_lookup(dentry);
> + dput(dentry);
> + if (IS_ERR(fid))
> + goto no_fid;
> } else {
> fid = v9fs_fid_find_inode(rreq->inode, writing, INVALID_UID, true);
> if (!fid)
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index a82a71be309b..e1b762f3e081 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -302,10 +302,12 @@ int v9fs_init_inode(struct v9fs_session_info *v9ses,
> goto error;
> }
>
> - if (v9fs_proto_dotl(v9ses))
> + if (v9fs_proto_dotl(v9ses)) {
> inode->i_op = &v9fs_symlink_inode_operations_dotl;
> - else
> + inode_nohighmem(inode);
What is that for?
> + } else {
> inode->i_op = &v9fs_symlink_inode_operations;
> + }
>
> break;
> case S_IFDIR:
> diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
> index 6312b3590f74..486b11dbada3 100644
> --- a/fs/9p/vfs_inode_dotl.c
> +++ b/fs/9p/vfs_inode_dotl.c
> @@ -686,9 +686,13 @@ v9fs_vfs_symlink_dotl(struct mnt_idmap *idmap, struct
> inode *dir, int err;
> kgid_t gid;
> const unsigned char *name;
> + umode_t mode;
> + struct v9fs_session_info *v9ses;
> struct p9_qid qid;
> struct p9_fid *dfid;
> struct p9_fid *fid = NULL;
> + struct inode *inode;
> + struct posix_acl *dacl = NULL, *pacl = NULL;
>
> name = dentry->d_name.name;
> p9_debug(P9_DEBUG_VFS, "%lu,%s,%s\n", dir->i_ino, name, symname);
> @@ -702,6 +706,15 @@ v9fs_vfs_symlink_dotl(struct mnt_idmap *idmap, struct
> inode *dir,
>
> gid = v9fs_get_fsgid_for_create(dir);
>
> + /* Update mode based on ACL value */
> + err = v9fs_acl_mode(dir, &mode, &dacl, &pacl);
> + if (err) {
> + p9_debug(P9_DEBUG_VFS,
> + "Failed to get acl values in symlink %d\n",
> + err);
> + goto error;
> + }
> +
> /* Server doesn't alter fid on TSYMLINK. Hence no need to clone it. */
> err = p9_client_symlink(dfid, name, symname, gid, &qid);
>
> @@ -712,8 +725,30 @@ v9fs_vfs_symlink_dotl(struct mnt_idmap *idmap, struct
> inode *dir,
>
> v9fs_invalidate_inode_attr(dir);
>
> + /* instantiate inode and assign the unopened fid to the dentry */
> + fid = p9_client_walk(dfid, 1, &name, 1);
> + if (IS_ERR(fid)) {
> + err = PTR_ERR(fid);
> + p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n",
> + err);
> + goto error;
> + }
> +
> + v9ses = v9fs_inode2v9ses(dir);
> + inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
> + if (IS_ERR(inode)) {
> + err = PTR_ERR(inode);
> + p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",
> + err);
> + goto error;
> + }
> + v9fs_set_create_acl(inode, fid, dacl, pacl);
> + v9fs_fid_add(dentry, &fid);
> + d_instantiate(dentry, inode);
> + err = 0;
> error:
> p9_fid_put(fid);
> + v9fs_put_acl(dacl, pacl);
> p9_fid_put(dfid);
> return err;
> }
> @@ -853,24 +888,23 @@ v9fs_vfs_mknod_dotl(struct mnt_idmap *idmap, struct
> inode *dir, }
>
> /**
> - * v9fs_vfs_get_link_dotl - follow a symlink path
> + * v9fs_vfs_get_link_nocache_dotl - Resolve a symlink directly.
> + *
> + * To be used when symlink caching is not enabled.
> + *
> * @dentry: dentry for symlink
> * @inode: inode for symlink
> * @done: destructor for return value
> */
> -
> static const char *
> -v9fs_vfs_get_link_dotl(struct dentry *dentry,
> - struct inode *inode,
> - struct delayed_call *done)
> +v9fs_vfs_get_link_nocache_dotl(struct dentry *dentry,
> + struct inode *inode,
> + struct delayed_call *done)
> {
> struct p9_fid *fid;
> char *target;
> int retval;
>
> - if (!dentry)
> - return ERR_PTR(-ECHILD);
> -
> p9_debug(P9_DEBUG_VFS, "%pd\n", dentry);
>
> fid = v9fs_fid_lookup(dentry);
> @@ -884,6 +918,29 @@ v9fs_vfs_get_link_dotl(struct dentry *dentry,
> return target;
> }
>
> +/**
> + * v9fs_vfs_get_link_dotl - follow a symlink path
> + * @dentry: dentry for symlink
> + * @inode: inode for symlink
> + * @done: destructor for return value
> + */
> +static const char *
> +v9fs_vfs_get_link_dotl(struct dentry *dentry,
> + struct inode *inode,
> + struct delayed_call *done)
> +{
> + struct v9fs_session_info *v9ses;
> +
> + if (!dentry)
> + return ERR_PTR(-ECHILD);
> +
> + v9ses = v9fs_inode2v9ses(inode);
> + if (v9ses->cache & (CACHE_META|CACHE_LOOSE))
> + return page_get_link(dentry, inode, done);
> +
> + return v9fs_vfs_get_link_nocache_dotl(dentry, inode, done);
> +}
> +
> int v9fs_refresh_inode_dotl(struct p9_fid *fid, struct inode *inode)
> {
> struct p9_stat_dotl *st;
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/3] 9p: Enable symlink caching in page cache
2026-02-12 15:35 ` Christian Schoenebeck
@ 2026-02-12 21:42 ` Remi Pommarel
2026-02-15 12:36 ` Dominique Martinet
0 siblings, 1 reply; 18+ messages in thread
From: Remi Pommarel @ 2026-02-12 21:42 UTC (permalink / raw)
To: Christian Schoenebeck
Cc: v9fs, linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet
On Thu, Feb 12, 2026 at 04:35:24PM +0100, Christian Schoenebeck wrote:
> On Wednesday, 21 January 2026 20:56:10 CET Remi Pommarel wrote:
> > Currently, when cache=loose is enabled, file reads are cached in the
> > page cache, but symlink reads are not. This patch allows the results
> > of p9_client_readlink() to be stored in the page cache, eliminating
> > the need for repeated 9P transactions on subsequent symlink accesses.
> >
> > This change improves performance for workloads that involve frequent
> > symlink resolution.
> >
> > Signed-off-by: Remi Pommarel <repk@triplefau.lt>
> > ---
> > fs/9p/vfs_addr.c | 24 ++++++++++++--
> > fs/9p/vfs_inode.c | 6 ++--
> > fs/9p/vfs_inode_dotl.c | 73 +++++++++++++++++++++++++++++++++++++-----
> > 3 files changed, 90 insertions(+), 13 deletions(-)
> >
> > diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
> > index 862164181bac..ee672abbb02c 100644
> > --- a/fs/9p/vfs_addr.c
> > +++ b/fs/9p/vfs_addr.c
> > @@ -70,10 +70,19 @@ static void v9fs_issue_read(struct netfs_io_subrequest
> > *subreq) {
> > struct netfs_io_request *rreq = subreq->rreq;
> > struct p9_fid *fid = rreq->netfs_priv;
> > + char *target;
> > unsigned long long pos = subreq->start + subreq->transferred;
> > - int total, err;
> > -
> > - total = p9_client_read(fid, pos, &subreq->io_iter, &err);
> > + int total, err, len, n;
> > +
> > + if (S_ISLNK(rreq->inode->i_mode)) {
> > + err = p9_client_readlink(fid, &target);
>
> Treadlink request requires 9p2000.L. So this would break with legacy protocol
> versions 9p2000 and 9p2000.u I guess:
>
> https://wiki.qemu.org/Documentation/9p#9p_Protocol
I'm having trouble seeing how v9fs_issue_read() could be called for
S_ISLNK inodes under 9p2000 or 9p2000.u.
As I understand it, v9fs_issue_read() is only invoked through the page
cache operations via the inode’s a_ops. This seems to me that only
regular files and (now that I added a page_get_link() in
v9fs_symlink_inode_operations_dotl) symlinks using 9p2000.L can call
v9fs_issue_read(). But not symlinks using 9p2000 or 9p2000.u as I
haven't modified v9fs_symlink_inode_operations. But I may have missed
something here ?
Maybe what is misleading is that this performance optimization is only
applicable for 9p2000.L which I should possibly mention in the commit
message.
>
> > + len = strnlen(target, PAGE_SIZE - 1);
>
> Usually we are bound to PATH_MAX.
>
> Target link path is coming from 9p server, which may run another OS and
> therefore target might be longer than PATH_MAX, in which case it would always
> yield in -ENAMETOOLONG when client requests that link target from cache.
>
> But OTOH there is no real alternative for storing a link target in cache that
> can never be delivered to user. So I guess it is OK as-is.
>
> Simply using PATH_MAX would make it worse, as it would potentially silently
> shorten the link from host.
>
> > + n = copy_to_iter(target, len, &subreq->io_iter);
> > + if (n != len)
> > + err = -EFAULT;
> > + total = i_size_read(rreq->inode);
> > + } else
> > + total = p9_client_read(fid, pos, &subreq->io_iter, &err);
> >
> > /* if we just extended the file size, any portion not in
> > * cache won't be on server and is zeroes */
> > @@ -99,6 +108,7 @@ static void v9fs_issue_read(struct netfs_io_subrequest
> > *subreq) static int v9fs_init_request(struct netfs_io_request *rreq, struct
> > file *file) {
> > struct p9_fid *fid;
> > + struct dentry *dentry;
> > bool writing = (rreq->origin == NETFS_READ_FOR_WRITE ||
> > rreq->origin == NETFS_WRITETHROUGH ||
> > rreq->origin == NETFS_UNBUFFERED_WRITE ||
> > @@ -115,6 +125,14 @@ static int v9fs_init_request(struct netfs_io_request
> > *rreq, struct file *file) if (!fid)
> > goto no_fid;
> > p9_fid_get(fid);
> > + } else if (S_ISLNK(rreq->inode->i_mode)) {
> > + dentry = d_find_alias(rreq->inode);
> > + if (!dentry)
> > + goto no_fid;
> > + fid = v9fs_fid_lookup(dentry);
> > + dput(dentry);
> > + if (IS_ERR(fid))
> > + goto no_fid;
> > } else {
> > fid = v9fs_fid_find_inode(rreq->inode, writing, INVALID_UID, true);
> > if (!fid)
> > diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> > index a82a71be309b..e1b762f3e081 100644
> > --- a/fs/9p/vfs_inode.c
> > +++ b/fs/9p/vfs_inode.c
> > @@ -302,10 +302,12 @@ int v9fs_init_inode(struct v9fs_session_info *v9ses,
> > goto error;
> > }
> >
> > - if (v9fs_proto_dotl(v9ses))
> > + if (v9fs_proto_dotl(v9ses)) {
> > inode->i_op = &v9fs_symlink_inode_operations_dotl;
> > - else
> > + inode_nohighmem(inode);
>
> What is that for?
According to filesystems/porting.rst and commit 21fc61c73c39 ("don't put
symlink bodies in pagecache into highmem") all symlinks that need to use
page_follow_link_light() (which is now more or less page_get_link())
should not add highmem pages in pagecache or deadlocks could happen. The
inode_nohighmem() prevents that.
Also from __page_get_link()
BUG_ON(mapping_gfp_mask(mapping) & __GFP_HIGHMEM);
A BUG_ON() is supposed to punish us if inode_nohighmem() is not used
here.
Of course this does not have any effect on 64bits platforms.
>
> > + } else {
> > inode->i_op = &v9fs_symlink_inode_operations;
> > + }
> >
> > break;
> > case S_IFDIR:
> > diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
> > index 6312b3590f74..486b11dbada3 100644
> > --- a/fs/9p/vfs_inode_dotl.c
> > +++ b/fs/9p/vfs_inode_dotl.c
> > @@ -686,9 +686,13 @@ v9fs_vfs_symlink_dotl(struct mnt_idmap *idmap, struct
> > inode *dir, int err;
> > kgid_t gid;
> > const unsigned char *name;
> > + umode_t mode;
> > + struct v9fs_session_info *v9ses;
> > struct p9_qid qid;
> > struct p9_fid *dfid;
> > struct p9_fid *fid = NULL;
> > + struct inode *inode;
> > + struct posix_acl *dacl = NULL, *pacl = NULL;
> >
> > name = dentry->d_name.name;
> > p9_debug(P9_DEBUG_VFS, "%lu,%s,%s\n", dir->i_ino, name, symname);
> > @@ -702,6 +706,15 @@ v9fs_vfs_symlink_dotl(struct mnt_idmap *idmap, struct
> > inode *dir,
> >
> > gid = v9fs_get_fsgid_for_create(dir);
> >
> > + /* Update mode based on ACL value */
> > + err = v9fs_acl_mode(dir, &mode, &dacl, &pacl);
> > + if (err) {
> > + p9_debug(P9_DEBUG_VFS,
> > + "Failed to get acl values in symlink %d\n",
> > + err);
> > + goto error;
> > + }
> > +
> > /* Server doesn't alter fid on TSYMLINK. Hence no need to clone it. */
> > err = p9_client_symlink(dfid, name, symname, gid, &qid);
> >
> > @@ -712,8 +725,30 @@ v9fs_vfs_symlink_dotl(struct mnt_idmap *idmap, struct
> > inode *dir,
> >
> > v9fs_invalidate_inode_attr(dir);
> >
> > + /* instantiate inode and assign the unopened fid to the dentry */
> > + fid = p9_client_walk(dfid, 1, &name, 1);
> > + if (IS_ERR(fid)) {
> > + err = PTR_ERR(fid);
> > + p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n",
> > + err);
> > + goto error;
> > + }
> > +
> > + v9ses = v9fs_inode2v9ses(dir);
> > + inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
> > + if (IS_ERR(inode)) {
> > + err = PTR_ERR(inode);
> > + p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",
> > + err);
> > + goto error;
> > + }
> > + v9fs_set_create_acl(inode, fid, dacl, pacl);
> > + v9fs_fid_add(dentry, &fid);
> > + d_instantiate(dentry, inode);
> > + err = 0;
> > error:
> > p9_fid_put(fid);
> > + v9fs_put_acl(dacl, pacl);
> > p9_fid_put(dfid);
> > return err;
> > }
> > @@ -853,24 +888,23 @@ v9fs_vfs_mknod_dotl(struct mnt_idmap *idmap, struct
> > inode *dir, }
> >
> > /**
> > - * v9fs_vfs_get_link_dotl - follow a symlink path
> > + * v9fs_vfs_get_link_nocache_dotl - Resolve a symlink directly.
> > + *
> > + * To be used when symlink caching is not enabled.
> > + *
> > * @dentry: dentry for symlink
> > * @inode: inode for symlink
> > * @done: destructor for return value
> > */
> > -
> > static const char *
> > -v9fs_vfs_get_link_dotl(struct dentry *dentry,
> > - struct inode *inode,
> > - struct delayed_call *done)
> > +v9fs_vfs_get_link_nocache_dotl(struct dentry *dentry,
> > + struct inode *inode,
> > + struct delayed_call *done)
> > {
> > struct p9_fid *fid;
> > char *target;
> > int retval;
> >
> > - if (!dentry)
> > - return ERR_PTR(-ECHILD);
> > -
> > p9_debug(P9_DEBUG_VFS, "%pd\n", dentry);
> >
> > fid = v9fs_fid_lookup(dentry);
> > @@ -884,6 +918,29 @@ v9fs_vfs_get_link_dotl(struct dentry *dentry,
> > return target;
> > }
> >
> > +/**
> > + * v9fs_vfs_get_link_dotl - follow a symlink path
> > + * @dentry: dentry for symlink
> > + * @inode: inode for symlink
> > + * @done: destructor for return value
> > + */
> > +static const char *
> > +v9fs_vfs_get_link_dotl(struct dentry *dentry,
> > + struct inode *inode,
> > + struct delayed_call *done)
> > +{
> > + struct v9fs_session_info *v9ses;
> > +
> > + if (!dentry)
> > + return ERR_PTR(-ECHILD);
> > +
> > + v9ses = v9fs_inode2v9ses(inode);
> > + if (v9ses->cache & (CACHE_META|CACHE_LOOSE))
> > + return page_get_link(dentry, inode, done);
> > +
> > + return v9fs_vfs_get_link_nocache_dotl(dentry, inode, done);
> > +}
> > +
> > int v9fs_refresh_inode_dotl(struct p9_fid *fid, struct inode *inode)
> > {
> > struct p9_stat_dotl *st;
>
Thanks for the review,
--
Remi
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/3] 9p: Enable symlink caching in page cache
2026-02-12 21:42 ` Remi Pommarel
@ 2026-02-15 12:36 ` Dominique Martinet
2026-02-19 10:18 ` Christian Schoenebeck
0 siblings, 1 reply; 18+ messages in thread
From: Dominique Martinet @ 2026-02-15 12:36 UTC (permalink / raw)
To: Remi Pommarel
Cc: Christian Schoenebeck, v9fs, linux-fsdevel, linux-kernel,
Eric Van Hensbergen, Latchesar Ionkov
Still haven't taken time to review, just replying since I'm looking at
9p mails tonight... (The IO accounting part was sent to Linus earlier,
still intending to review and test soonish but feel free to send a v3
with Christian comments for now)
Remi Pommarel wrote on Thu, Feb 12, 2026 at 10:42:50PM +0100:
> > > + if (S_ISLNK(rreq->inode->i_mode)) {
> > > + err = p9_client_readlink(fid, &target);
> >
> > Treadlink request requires 9p2000.L. So this would break with legacy protocol
> > versions 9p2000 and 9p2000.u I guess:
> >
> > https://wiki.qemu.org/Documentation/9p#9p_Protocol
>
> I'm having trouble seeing how v9fs_issue_read() could be called for
> S_ISLNK inodes under 9p2000 or 9p2000.u.
>
> As I understand it, v9fs_issue_read() is only invoked through the page
> cache operations via the inode’s a_ops. This seems to me that only
> regular files and (now that I added a page_get_link() in
> v9fs_symlink_inode_operations_dotl) symlinks using 9p2000.L can call
> v9fs_issue_read(). But not symlinks using 9p2000 or 9p2000.u as I
> haven't modified v9fs_symlink_inode_operations. But I may have missed
> something here ?
I think that's correct, but since it's not obvious perhaps a comment
just above the p9_client_readlink() call might be useful?
Also nitpick: please add brackets to the else as well because the if
part had some (checkpatch doesn't complain either way, but I don't like
if (foo) {
...
} else
bar
formatting)
> > > diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> > > index a82a71be309b..e1b762f3e081 100644
> > > --- a/fs/9p/vfs_inode.c
> > > +++ b/fs/9p/vfs_inode.c
> > > @@ -302,10 +302,12 @@ int v9fs_init_inode(struct v9fs_session_info *v9ses,
> > > goto error;
> > > }
> > >
> > > - if (v9fs_proto_dotl(v9ses))
> > > + if (v9fs_proto_dotl(v9ses)) {
> > > inode->i_op = &v9fs_symlink_inode_operations_dotl;
> > > - else
> > > + inode_nohighmem(inode);
> >
> > What is that for?
>
> According to filesystems/porting.rst and commit 21fc61c73c39 ("don't put
> symlink bodies in pagecache into highmem") all symlinks that need to use
> page_follow_link_light() (which is now more or less page_get_link())
> should not add highmem pages in pagecache or deadlocks could happen. The
> inode_nohighmem() prevents that.
>
> Also from __page_get_link()
> BUG_ON(mapping_gfp_mask(mapping) & __GFP_HIGHMEM);
>
> A BUG_ON() is supposed to punish us if inode_nohighmem() is not used
> here.
>
> Of course this does not have any effect on 64bits platforms.
That's how it should be but it marks memory as GFP_USER, I'm curious if
it really has no other effect... Anyway, from what I've read I think it
is likely to go away (highmem on 32bit platforms) soon enough, so this
probably won't matter in the long run.
(if we really care we could flag this only for S_ISLNK(mode), but I
don't think it's worth the check)
--
Dominique Martinet | Asmadeus
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/3] 9p: Enable symlink caching in page cache
2026-02-15 12:36 ` Dominique Martinet
@ 2026-02-19 10:18 ` Christian Schoenebeck
0 siblings, 0 replies; 18+ messages in thread
From: Christian Schoenebeck @ 2026-02-19 10:18 UTC (permalink / raw)
To: Remi Pommarel, Dominique Martinet
Cc: v9fs, linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov
On Sunday, 15 February 2026 13:36:02 CET Dominique Martinet wrote:
> Still haven't taken time to review, just replying since I'm looking at
> 9p mails tonight... (The IO accounting part was sent to Linus earlier,
> still intending to review and test soonish but feel free to send a v3
> with Christian comments for now)
>
> Remi Pommarel wrote on Thu, Feb 12, 2026 at 10:42:50PM +0100:
> > > > + if (S_ISLNK(rreq->inode->i_mode)) {
> > > > + err = p9_client_readlink(fid, &target);
> > >
> > > Treadlink request requires 9p2000.L. So this would break with legacy
> > > protocol versions 9p2000 and 9p2000.u I guess:
> > >
> > > https://wiki.qemu.org/Documentation/9p#9p_Protocol
> >
> > I'm having trouble seeing how v9fs_issue_read() could be called for
> > S_ISLNK inodes under 9p2000 or 9p2000.u.
> >
> > As I understand it, v9fs_issue_read() is only invoked through the page
> > cache operations via the inode’s a_ops. This seems to me that only
> > regular files and (now that I added a page_get_link() in
> > v9fs_symlink_inode_operations_dotl) symlinks using 9p2000.L can call
> > v9fs_issue_read(). But not symlinks using 9p2000 or 9p2000.u as I
> > haven't modified v9fs_symlink_inode_operations. But I may have missed
> > something here ?
>
> I think that's correct, but since it's not obvious perhaps a comment
> just above the p9_client_readlink() call might be useful?
And maybe adding a BUG_ON(!p9_is_proto_dotl(client)) in p9_client_readlink()
[net/9p/client.c].
> Also nitpick: please add brackets to the else as well because the if
> part had some (checkpatch doesn't complain either way, but I don't like
> if (foo) {
> ...
> } else
> bar
> formatting)
>
> > > > diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> > > > index a82a71be309b..e1b762f3e081 100644
> > > > --- a/fs/9p/vfs_inode.c
> > > > +++ b/fs/9p/vfs_inode.c
> > > > @@ -302,10 +302,12 @@ int v9fs_init_inode(struct v9fs_session_info
> > > > *v9ses,
> > > >
> > > > goto error;
> > > >
> > > > }
> > > >
> > > > - if (v9fs_proto_dotl(v9ses))
> > > > + if (v9fs_proto_dotl(v9ses)) {
> > > >
> > > > inode->i_op = &v9fs_symlink_inode_operations_dotl;
> > > >
> > > > - else
> > > > + inode_nohighmem(inode);
> > >
> > > What is that for?
> >
> > According to filesystems/porting.rst and commit 21fc61c73c39 ("don't put
> > symlink bodies in pagecache into highmem") all symlinks that need to use
> > page_follow_link_light() (which is now more or less page_get_link())
> > should not add highmem pages in pagecache or deadlocks could happen. The
> > inode_nohighmem() prevents that.
> >
> > Also from __page_get_link()
> >
> > BUG_ON(mapping_gfp_mask(mapping) & __GFP_HIGHMEM);
> >
> > A BUG_ON() is supposed to punish us if inode_nohighmem() is not used
> > here.
> >
> > Of course this does not have any effect on 64bits platforms.
>
> That's how it should be but it marks memory as GFP_USER, I'm curious if
> it really has no other effect... Anyway, from what I've read I think it
> is likely to go away (highmem on 32bit platforms) soon enough, so this
> probably won't matter in the long run.
> (if we really care we could flag this only for S_ISLNK(mode), but I
> don't think it's worth the check)
Good point. Most other fs only call inode_nohighmem() for symlinks. IIUIC it's
not a good idea to do this simply for all inodes, as it may lead to deadlocks
for instance. So I think it is worth to add that check.
/Christian
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/3] 9p: Performance improvements for build workloads
2026-01-21 19:56 [PATCH v2 0/3] 9p: Performance improvements for build workloads Remi Pommarel
` (2 preceding siblings ...)
2026-01-21 19:56 ` [PATCH v2 3/3] 9p: Enable symlink caching in page cache Remi Pommarel
@ 2026-01-21 23:23 ` Dominique Martinet
2026-02-04 11:37 ` Christian Schoenebeck
4 siblings, 0 replies; 18+ messages in thread
From: Dominique Martinet @ 2026-01-21 23:23 UTC (permalink / raw)
To: Remi Pommarel
Cc: v9fs, linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov, Christian Schoenebeck
Remi Pommarel wrote on Wed, Jan 21, 2026 at 08:56:07PM +0100:
> This patchset introduces several performance optimizations for the 9p
> filesystem when used with cache=loose option (exclusive or read only
> mounts). These improvements particularly target workloads with frequent
> lookups of non-existent paths and repeated symlink resolutions.
>
> The very state of the art benchmark consisting of cloning a fresh
> hostap repository and building hostapd and wpa_supplicant for hwsim
> tests (cd tests/hwsim; time ./build.sh) in a VM running on a 9pfs rootfs
> (with trans=virtio,cache=loose options) has been used to test those
> optimizations impact.
>
> For reference, the build takes 0m56.492s on my laptop natively while it
> completes in 2m18.702sec on the VM. This represents a significant
> performance penalty considering running the same build on a VM using a
> virtiofs rootfs (with "--cache always" virtiofsd option) takes around
> 1m32.141s. This patchset aims to bring the 9pfs build time close to
> that of virtiofs, rather than the native host time, as a realistic
> expectation.
>
> This first two patches in this series focus on keeping negative dentries
> in the cache, ensuring that subsequent lookups for paths known to not
> exist do not require redundant 9P RPC calls. This optimization reduces
> the time needed for the compiler to search for header files across known
> locations. These two patches introduce a new mount option, ndentrytmo,
> which specifies the number of ms to keep the dentry in the cache. Using
> ndentrytmo=-1 (keeping the negative dentry indifinetly) shrunk build
> time to 1m46.198s.
>
> The third patch extends page cache usage to symlinks by allowing
> p9_client_readlink() results to be cached. Resolving symlink is
> apparently something done quite frequently during the build process and
> avoiding the cost of a 9P RPC call round trip for already known symlinks
> helps reduce the build time to 1m26.602s, outperforming the virtiofs
> setup.
>
> Here is summary of the different hostapd/wpa_supplicant build times:
>
> - Baseline (no patch): 2m18.702s
> - negative dentry caching (patches 1-2): 1m46.198s (23% improvement)
> - Above + symlink caching (patches 1-3): 1m26.302s (an additional 18%
> improvement, 37% in total)
>
> With this ~37% performance gain, 9pfs with cache=loose can compete with
> virtiofs for (at least) this specific scenario. Although this benchmark
> is not the most typical, I do think that these caching optimizations
> could benefit a wide range of other workflows as well.
Thank you!
We've had a couple of regressions lately so I'll take a week or two to
run some proper tests first, but overall looks good to me, I just wanted
to acknowledge the patches early.
(as such it likely won't make 6.20 but should hopefully go into the next
one)
--
Dominique
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 0/3] 9p: Performance improvements for build workloads
2026-01-21 19:56 [PATCH v2 0/3] 9p: Performance improvements for build workloads Remi Pommarel
` (3 preceding siblings ...)
2026-01-21 23:23 ` [PATCH v2 0/3] 9p: Performance improvements for build workloads Dominique Martinet
@ 2026-02-04 11:37 ` Christian Schoenebeck
4 siblings, 0 replies; 18+ messages in thread
From: Christian Schoenebeck @ 2026-02-04 11:37 UTC (permalink / raw)
To: v9fs, Remi Pommarel
Cc: linux-fsdevel, linux-kernel, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet, Remi Pommarel
On Wednesday, 21 January 2026 20:56:07 CET Remi Pommarel wrote:
> This patchset introduces several performance optimizations for the 9p
> filesystem when used with cache=loose option (exclusive or read only
> mounts). These improvements particularly target workloads with frequent
> lookups of non-existent paths and repeated symlink resolutions.
[...]
> Here is summary of the different hostapd/wpa_supplicant build times:
>
> - Baseline (no patch): 2m18.702s
> - negative dentry caching (patches 1-2): 1m46.198s (23% improvement)
> - Above + symlink caching (patches 1-3): 1m26.302s (an additional 18%
> improvement, 37% in total)
>
> With this ~37% performance gain, 9pfs with cache=loose can compete with
> virtiofs for (at least) this specific scenario. Although this benchmark
> is not the most typical, I do think that these caching optimizations
> could benefit a wide range of other workflows as well.
I did a wide range of tests. In broad average I'm also seeing ~40% improvement
when compiling. Some individual sources even had 60% improvements and more. So
there is quite a big variance.
I did not encounter misbehaviours in my tests, so feel free to add:
Tested-by: Christian Schoenebeck <linux_oss@crudebyte.com>
I still need to make a proper review though.
/Christian
^ permalink raw reply [flat|nested] 18+ messages in thread