[RFC PATCH 0/5] 9p: Performance improvements for build workloads

[RFC PATCH 0/5] 9p: Performance improvements for build workloads

[Remi Pommarel]

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.

The last two patches are only here to attribute time spent waiting
for server responses during a 9P RPC call to I/0 wait time in system
metrics.

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.

Further investigation may be needed to address the remaining gap with
native build performance. Using the last two patches it appears there is
still a fair amount of time spent waiting for I/O, though. This could be
related to the two systematic RPC calls made when opening a file (one to
clone the fid and another one to open the file). Maybe reusing fids or
openned files could potentially reduce client/server transactions and
bring performance even closer to native levels ? But that are just
random thoughs I haven't dig enough yet.

Any feedbacks on this approach would be welcomed,

Thanks.

Best regards,

-- 
Remi

Remi Pommarel (5):
  9p: Cache negative dentries for lookup performance
  9p: Introduce option for negative dentry cache retention time
  9p: Enable symlink caching in page cache
  wait: Introduce io_wait_event_killable()
  9p: Track 9P RPC waiting time as IO

 fs/9p/fid.c            |  11 +++--
 fs/9p/v9fs.c           |  16 +++++-
 fs/9p/v9fs.h           |   3 ++
 fs/9p/v9fs_vfs.h       |  15 ++++++
 fs/9p/vfs_dentry.c     | 109 +++++++++++++++++++++++++++++++++++------
 fs/9p/vfs_inode.c      |  14 ++++--
 fs/9p/vfs_inode_dotl.c |  94 +++++++++++++++++++++++++++++++----
 include/linux/wait.h   |  15 ++++++
 net/9p/client.c        |   4 +-
 9 files changed, 244 insertions(+), 37 deletions(-)

-- 
2.50.1

[RFC PATCH 1/5] 9p: Cache negative dentries for lookup performance

[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 | 109 +++++++++++++++++++++++++++++++++++++++------
 fs/9p/vfs_inode.c  |   7 +--
 6 files changed, 123 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 77e9c4387c1d..422bd720d165 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -171,6 +171,7 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
 
 	/* setup defaults */
 	v9ses->afid = ~0;
+	v9ses->ndentry_timeout = 0;
 	v9ses->debug = 0;
 	v9ses->cache = CACHE_NONE;
 #ifdef CONFIG_9P_FSCACHE
diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
index f28bc763847a..8c5fa0f7ba71 100644
--- a/fs/9p/v9fs.h
+++ b/fs/9p/v9fs.h
@@ -88,6 +88,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
@@ -113,6 +114,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 04795508a795..5035b8c749e2 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,15 +89,57 @@ 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 v9fs_session_info *v9ses;
 	struct p9_fid *fid;
 	struct inode *inode;
 	struct v9fs_inode *v9inode;
@@ -70,9 +147,11 @@ static int __v9fs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	v9ses = v9fs_dentry2v9ses(dentry);
+
 	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) {
@@ -95,7 +174,7 @@ static int __v9fs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
 		if (retval < 0)
 			return retval;
 	}
-out_valid:
+
 	return 1;
 }
 
@@ -121,12 +200,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 399d455d50d6..89eeb2b3ba43 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);
-- 
2.50.1

[RFC PATCH 2/5] 9p: Introduce option for negative dentry cache retention time

[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 | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index 422bd720d165..7c0c2201b151 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -34,7 +34,7 @@ struct kmem_cache *v9fs_inode_cache;
 
 enum {
 	/* 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 */
@@ -52,6 +52,7 @@ static const match_table_t tokens = {
 	{Opt_dfltuid, "dfltuid=%u"},
 	{Opt_dfltgid, "dfltgid=%u"},
 	{Opt_afid, "afid=%u"},
+	{Opt_ndentrytmo, "ndentrytmo=%d"},
 	{Opt_uname, "uname=%s"},
 	{Opt_remotename, "aname=%s"},
 	{Opt_nodevmap, "nodevmap"},
@@ -110,6 +111,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)
@@ -251,6 +254,16 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
 				v9ses->afid = option;
 			}
 			break;
+		case Opt_ndentrytmo:
+			r = match_int(&args[0], &option);
+			if (r < 0) {
+				p9_debug(P9_DEBUG_ERROR,
+					 "integer field, but no integer?\n");
+				ret = r;
+			} else {
+				v9ses->ndentry_timeout = option;
+			}
+			break;
 		case Opt_uname:
 			kfree(v9ses->uname);
 			v9ses->uname = match_strdup(&args[0]);
-- 
2.50.1

[RFC PATCH 3/5] 9p: Enable symlink caching in page cache

[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/v9fs.h           |  1 +
 fs/9p/vfs_inode.c      |  7 +++-
 fs/9p/vfs_inode_dotl.c | 94 +++++++++++++++++++++++++++++++++++++-----
 3 files changed, 90 insertions(+), 12 deletions(-)

diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
index 8c5fa0f7ba71..f48a85b610e3 100644
--- a/fs/9p/v9fs.h
+++ b/fs/9p/v9fs.h
@@ -186,6 +186,7 @@ extern struct inode *v9fs_inode_from_fid(struct v9fs_session_info *v9ses,
 					 struct super_block *sb, int new);
 extern const struct inode_operations v9fs_dir_inode_operations_dotl;
 extern const struct inode_operations v9fs_file_inode_operations_dotl;
+extern const struct address_space_operations v9fs_symlink_aops_dotl;
 extern const struct inode_operations v9fs_symlink_inode_operations_dotl;
 extern const struct netfs_request_ops v9fs_req_ops;
 extern struct inode *v9fs_inode_from_fid_dotl(struct v9fs_session_info *v9ses,
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 89eeb2b3ba43..362e39b3e7eb 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -302,10 +302,13 @@ 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->i_mapping->a_ops = &v9fs_symlink_aops_dotl;
+			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 5b5fda617b80..2e2119be1d49 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -126,8 +126,10 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
 		goto error;
 
 	v9fs_stat2inode_dotl(st, inode, 0);
-	v9fs_set_netfs_context(inode);
-	v9fs_cache_inode_get_cookie(inode);
+	if (inode->i_mapping->a_ops == &v9fs_addr_operations) {
+		v9fs_set_netfs_context(inode);
+		v9fs_cache_inode_get_cookie(inode);
+	}
 	retval = v9fs_get_acl(inode, fid);
 	if (retval)
 		goto error;
@@ -858,24 +860,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);
@@ -889,6 +890,75 @@ v9fs_vfs_get_link_dotl(struct dentry *dentry,
 	return target;
 }
 
+/**
+ * v9fs_symlink_read_folio_dotl - Fetch a symlink path and store it in buffer
+ * cache.
+ * @file: file associated to symlink (NULL)
+ * @folio: folio where to store the symlink resolve result
+ */
+static int v9fs_symlink_read_folio_dotl(struct file *file,
+					struct folio *folio)
+{
+	struct inode *inode = folio->mapping->host;
+	struct dentry *dentry;
+	struct p9_fid *fid;
+	char *target;
+	size_t len;
+	int ret = -EIO;
+
+	/* Does not expect symlink inode to have a fid as it as not been
+	 * opened>
+	 */
+	dentry = d_find_alias(inode);
+	if (!dentry)
+		goto out;
+
+	fid = v9fs_fid_lookup(dentry);
+	dput(dentry);
+	if (IS_ERR(fid)) {
+		ret = PTR_ERR(fid);
+		goto out;
+	}
+
+	ret = p9_client_readlink(fid, &target);
+	p9_fid_put(fid);
+	if (ret)
+		goto out;
+
+	len = strnlen(target, PAGE_SIZE - 1);
+	target[len] = '\0';
+	memcpy_to_folio(folio, 0, target, len);
+	kfree(target);
+
+	ret = 0;
+out:
+	folio_end_read(folio, ret == 0);
+	return ret;
+}
+
+/**
+ * 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;
@@ -945,6 +1015,10 @@ const struct inode_operations v9fs_file_inode_operations_dotl = {
 	.set_acl = v9fs_iop_set_acl,
 };
 
+const struct address_space_operations v9fs_symlink_aops_dotl = {
+	.read_folio = v9fs_symlink_read_folio_dotl,
+};
+
 const struct inode_operations v9fs_symlink_inode_operations_dotl = {
 	.get_link = v9fs_vfs_get_link_dotl,
 	.getattr = v9fs_vfs_getattr_dotl,
-- 
2.50.1

[RFC PATCH 4/5] wait: Introduce io_wait_event_killable()

[Remi Pommarel]

Add io_wait_event_killable(), a variant of wait_event_killable() that
uses io_schedule() instead of schedule(). This is to be used in
situation where waiting time is to be accounted as IO wait time.

Signed-off-by: Remi Pommarel <repk@triplefau.lt>
---
 include/linux/wait.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/linux/wait.h b/include/linux/wait.h
index 09855d819418..cfeb1adee973 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -937,6 +937,21 @@ extern int do_wait_intr_irq(wait_queue_head_t *, wait_queue_entry_t *);
 	__ret;									\
 })
 
+#define __io_wait_event_killable(wq, condition)					\
+	___wait_event(wq, condition, TASK_KILLABLE, 0, 0, io_schedule())
+
+/*
+ * wait_event_killable() - link wait_event_killable but with io_schedule()
+ */
+#define io_wait_event_killable(wq_head, condition)				\
+({										\
+	int __ret = 0;								\
+	might_sleep();								\
+	if (!(condition))							\
+		__ret = __io_wait_event_killable(wq_head, condition);		\
+	__ret;									\
+})
+
 #define __wait_event_state(wq, condition, state)				\
 	___wait_event(wq, condition, state, 0, 0, schedule())
 
-- 
2.50.1

[RFC PATCH 5/5] 9p: Track 9P RPC waiting time as IO

[Remi Pommarel]

Use io_wait_event_killable() to ensure that time spent waiting for 9P
RPC transactions is accounted as IO wait time.

Signed-off-by: Remi Pommarel <repk@triplefau.lt>
---
 net/9p/client.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/9p/client.c b/net/9p/client.c
index 5c1ca57ccd28..3f0f4d6efc00 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -713,8 +713,8 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
 	}
 again:
 	/* Wait for the response */
-	err = wait_event_killable(req->wq,
-				  READ_ONCE(req->status) >= REQ_STATUS_RCVD);
+	err = io_wait_event_killable(req->wq,
+				     READ_ONCE(req->status) >= REQ_STATUS_RCVD);
 
 	/* Make sure our req is coherent with regard to updates in other
 	 * threads - echoes to wmb() in the callback
-- 
2.50.1

Re: [RFC PATCH 0/5] 9p: Performance improvements for build workloads

[Dominique Martinet]

Remi Pommarel wrote on Sun, Aug 31, 2025 at 09:03:38PM +0200:
> 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.

Sorry for slow reply, I think a negative cache and symlink cache make
sense.
I haven't tested these yet, and there's a conversion to the "new" mount
API that's brewing and will conflict with 2nd patch, but I'll be happy
to take these patches as time allows.
What was the reason this was sent as RFC, does something require more work?

I can't comment on io_wait_event_killable, it makes sense to me as well
but it's probably more appropriate to send through the scheduler tree.


> 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.

That's rather impressive!
(I assume virtiofs does not have such negative lookup or symlink cache so
they'll catch up soon enough if someone cares? But that's no reason to
refuse this with cache=loose)

> Further investigation may be needed to address the remaining gap with
> native build performance. Using the last two patches it appears there is
> still a fair amount of time spent waiting for I/O, though. This could be
> related to the two systematic RPC calls made when opening a file (one to
> clone the fid and another one to open the file). Maybe reusing fids or
> openned files could potentially reduce client/server transactions and
> bring performance even closer to native levels ? But that are just
> random thoughs I haven't dig enough yet.

Another thing I tried ages ago was making clunk asynchronous,
but that didn't go well;
protocol-wise clunk errors are ignored so I figured it was safe enough
to just fire it in the background, but it caused some regressions I
never had time to look into...

As for reusing fids, I'm not sure it's obvious because of things like
locking that basically consider one open file = one fid;
I think we're already re-using fids when we can, but I guess it's
technically possible to mark a fid as shared and only clone it if an
operation that requires an exclusive fid is done...?
I'm not sure I want to go down that hole though, sounds like an easy way
to mess up and give someone access to data they shouldn't be able to
access by sharing a fid opened by another user or something more
subtle..

-- 
Dominique Martinet | Asmadeus

Re: [RFC PATCH 0/5] 9p: Performance improvements for build workloads

[Remi Pommarel]

Hi Dominique,

On Sun, Sep 14, 2025 at 09:34:11PM +0900, Dominique Martinet wrote:
> Remi Pommarel wrote on Sun, Aug 31, 2025 at 09:03:38PM +0200:
> > 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.
> 
> Sorry for slow reply, I think a negative cache and symlink cache make
> sense.
> I haven't tested these yet, and there's a conversion to the "new" mount
> API that's brewing and will conflict with 2nd patch, but I'll be happy
> to take these patches as time allows.
> What was the reason this was sent as RFC, does something require more work?
> 
> I can't comment on io_wait_event_killable, it makes sense to me as well
> but it's probably more appropriate to send through the scheduler tree.
> 

RFC was mainly here to know if a io_wait_event_killable() would made
sense before getting the scheduler tree involved. Also as it is my first
contribution in v9fs (and fs subsystem) wanted to be sure I wasn't
missing something obvious, caching could be a complex subject to grasp.
This also comes with some drawbacks, if for example server removes a
shared file or modify a symlink the client will be desynchronized, so I
wanted first to be sure we were ok with that when using cache=loose.

I'll try to monitor the new mount API and rebase the series when that
get merged. I'll probably separate the io_wait_event_killable() in its
own patchset though.

> 
> > 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.
> 
> That's rather impressive!
> (I assume virtiofs does not have such negative lookup or symlink cache so
> they'll catch up soon enough if someone cares? But that's no reason to
> refuse this with cache=loose)
> 

virtiofs does have negative lookup (when used with cache=always) and
symlink caches (this serie is even quite a bit inspired by what fuse
does). I don't really know what makes virtiofs a bit slower here, I
haven't dig into it either though but won't be surprised it could easily
catch up.

> > Further investigation may be needed to address the remaining gap with
> > native build performance. Using the last two patches it appears there is
> > still a fair amount of time spent waiting for I/O, though. This could be
> > related to the two systematic RPC calls made when opening a file (one to
> > clone the fid and another one to open the file). Maybe reusing fids or
> > openned files could potentially reduce client/server transactions and
> > bring performance even closer to native levels ? But that are just
> > random thoughs I haven't dig enough yet.
> 
> Another thing I tried ages ago was making clunk asynchronous,
> but that didn't go well;
> protocol-wise clunk errors are ignored so I figured it was safe enough
> to just fire it in the background, but it caused some regressions I
> never had time to look into...
> 
> As for reusing fids, I'm not sure it's obvious because of things like
> locking that basically consider one open file = one fid;
> I think we're already re-using fids when we can, but I guess it's
> technically possible to mark a fid as shared and only clone it if an
> operation that requires an exclusive fid is done...?
> I'm not sure I want to go down that hole though, sounds like an easy way
> to mess up and give someone access to data they shouldn't be able to
> access by sharing a fid opened by another user or something more
> subtle..

Yes I gave that a bit more thinking and came up with quite the same
conclusion, I then gave up on this idea. The asynchronous clunk seems
interesting though, maybe I'll take a look into that.

Thanks for your time.

-- 
Remi

Re: [RFC PATCH 0/5] 9p: Performance improvements for build workloads

[Dominique Martinet]

Remi Pommarel wrote on Thu, Sep 18, 2025 at 09:17:33PM +0200:
> RFC was mainly here to know if a io_wait_event_killable() would made
> sense before getting the scheduler tree involved. Also as it is my first
> contribution in v9fs (and fs subsystem) wanted to be sure I wasn't
> missing something obvious, caching could be a complex subject to grasp.
> This also comes with some drawbacks, if for example server removes a
> shared file or modify a symlink the client will be desynchronized, so I
> wanted first to be sure we were ok with that when using cache=loose.

Ok!
I think it's completely fine for cache=loose, we're basically telling
the client we're alone in the world.

> I'll try to monitor the new mount API and rebase the series when that
> get merged. I'll probably separate the io_wait_event_killable() in its
> own patchset though.

Thanks, I need to find time to check the v9ses lifetime as I asked about
after a syzcaller bug showed up[1], so it might not be immediate, but
I'll get to it eventually

[1] https://lore.kernel.org/v9fs/aKlg5Ci4WC11GZGz@codewreck.org/T/#u

> > Another thing I tried ages ago was making clunk asynchronous,
> > but that didn't go well;
> > protocol-wise clunk errors are ignored so I figured it was safe enough
> > to just fire it in the background, but it caused some regressions I
> > never had time to look into...
> > 
> > As for reusing fids, I'm not sure it's obvious because of things like
> > locking that basically consider one open file = one fid;
> > I think we're already re-using fids when we can, but I guess it's
> > technically possible to mark a fid as shared and only clone it if an
> > operation that requires an exclusive fid is done...?
> > I'm not sure I want to go down that hole though, sounds like an easy way
> > to mess up and give someone access to data they shouldn't be able to
> > access by sharing a fid opened by another user or something more
> > subtle..
> 
> Yes I gave that a bit more thinking and came up with quite the same
> conclusion, I then gave up on this idea. The asynchronous clunk seems
> interesting though, maybe I'll take a look into that.

It's been a while, but the last time I rebased the patches was around here:
https://github.com/martinetd/linux/commits/9p-async-v2/
(the v1 branch also had clunks async, with this comment
> This has a few problems, but mostly we can't just replace all clunks
> with async ones: depending on the server, explicit close() must clunk
> to make sure the IO is flushed, so these should wait for clunk to finish.
)

If you have time to play with this, happy to consider it again, but
it'll definitely need careful testing (possibly implement the clunk part
as a non-default option? although I'm not sure how that'd fly, linux
doesn't really like options that sacrifice reliability for performance...)

Anyway, that's something I definitely don't have time for short term,
but happy to discuss :)

Cheers,
-- 
Dominique Martinet | Asmadeus