linux-kernel - [PATCH] xfs: use inode_set_cached

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241123075105.1082661-1-mjguzik@gmail.com>
Date: Sat, 23 Nov 2024 08:51:05 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: dchinner@...hat.com
Cc: cem@...nel.org,
	djwong@...nel.org,
	linux-kernel@...r.kernel.org,
	linux-xfs@...r.kernel.org,
	Mateusz Guzik <mjguzik@...il.com>
Subject: [PATCH] xfs: use inode_set_cached_link()

For cases where caching is applicable this dodges inode locking, memory
allocation and memcpy + strlen.

Throughput of readlink on Saphire Rappids (ops/s):
before:	3641273
after:	4009524 (+10%)

Signed-off-by: Mateusz Guzik <mjguzik@...il.com>
---

First a minor note that in the stock case strlen is called on the buffer
and I verified that i_disk_size is the value which is computed.

The important note is that I'm assuming the pointed to area is stable
for the duration of the inode's lifetime -- that is if the read off
symlink is fine *or* it was just created and is eligible caching, it
wont get invalidated as long as the inode is in memory. If this does not
hold then this submission is wrong and it would be nice(tm) to remedy
it.

This depends on stuff which landed in vfs-6.14.misc, but is not in next
nor fs-next yet.

For benchmark code see bottom of https://lore.kernel.org/linux-fsdevel/20241120112037.822078-1-mjguzik@gmail.com/

 fs/xfs/xfs_iops.c    |  1 +
 fs/xfs/xfs_symlink.c | 24 ++++++++++++++++++++++++
 fs/xfs/xfs_symlink.h |  1 +
 3 files changed, 26 insertions(+)

diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 207e0dadffc3..1d0a3797f876 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1394,6 +1394,7 @@ xfs_setup_iops(
 		break;
 	case S_IFLNK:
 		inode->i_op = &xfs_symlink_inode_operations;
+		xfs_setup_cached_symlink(ip);
 		break;
 	default:
 		inode->i_op = &xfs_inode_operations;
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index 4252b07cd251..59bf1b9ccb20 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -28,6 +28,30 @@
 #include "xfs_parent.h"
 #include "xfs_defer.h"
 
+void
+xfs_setup_cached_symlink(
+	struct xfs_inode	*ip)
+{
+	struct inode		*inode = &ip->i_vnode;
+	xfs_fsize_t		pathlen;
+
+	/*
+	 * If we have the symlink readily accessible let the VFS know where to
+	 * find it. This avoids calls to xfs_readlink().
+	 */
+	pathlen = ip->i_disk_size;
+	if (pathlen <= 0 || pathlen > XFS_SYMLINK_MAXLEN)
+		return;
+
+	if (ip->i_df.if_format != XFS_DINODE_FMT_LOCAL)
+		return;
+
+	if (XFS_IS_CORRUPT(ip->i_mount, !ip->i_df.if_data))
+		return;
+
+	inode_set_cached_link(inode, ip->i_df.if_data, pathlen);
+}
+
 int
 xfs_readlink(
 	struct xfs_inode	*ip,
diff --git a/fs/xfs/xfs_symlink.h b/fs/xfs/xfs_symlink.h
index 0d29a50e66fd..0e45a8a33829 100644
--- a/fs/xfs/xfs_symlink.h
+++ b/fs/xfs/xfs_symlink.h
@@ -12,5 +12,6 @@ int xfs_symlink(struct mnt_idmap *idmap, struct xfs_inode *dp,
 		umode_t mode, struct xfs_inode **ipp);
 int xfs_readlink(struct xfs_inode *ip, char *link);
 int xfs_inactive_symlink(struct xfs_inode *ip);
+void xfs_setup_cached_symlink(struct xfs_inode *ip);
 
 #endif /* __XFS_SYMLINK_H */
-- 
2.43.0