linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] permission check avoidance during lookup
@ 2025-11-06 18:00 Mateusz Guzik
  2025-11-06 18:00 ` [PATCH v2 1/4] fs: speed up path lookup with cheaper MAY_EXEC checks Mateusz Guzik
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Mateusz Guzik @ 2025-11-06 18:00 UTC (permalink / raw)
  To: brauner
  Cc: viro, jack, linux-kernel, linux-fsdevel, linux-ext4, tytso,
	torvalds, josef, linux-btrfs, Mateusz Guzik

To quote from patch 1:
<quote>
Vast majority of real-world lookups happen on directories which are
traversable by anyone. Figuring out that this holds for a given inode
can be done when instantiating it or changing permissions, avoiding the
overhead during lookup. Stats below.

A simple microbench of stating /usr/include/linux/fs.h on ext4 in a loop
on Sapphire Rapids (ops/s):
before: 3640352
after:  3797258 (+4%)
</quote>

During a kernel build about 90% of all lookups managed to skip
permission checks in my setup, see the commit message for a breakdown.

WARNING: more testing is needed for correctness, but I'm largely happy
with the state as is.

WARNING: I'm assuming the following bit is applied:
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 78ea864fa8cd..eaf776cd4175 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5518,6 +5518,10 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
                goto bad_inode;
        brelse(iloc.bh);
 
+       /* Initialize the "no ACL's" state for the simple cases */
+       if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR) && !ei->i_file_acl)
+               cache_no_acl(inode);
+
        unlock_new_inode(inode);
        return inode;

Lack of the patch does not affect correctness, but it does make the
patch ineffective for ext4. I did not include it in the posting as other
people promised to sort it out.

Discussion is here with an ack from Jan:
https://lore.kernel.org/linux-fsdevel/kn44smk4dgaj5rqmtcfr7ruecixzrik6omur2l2opitn7lbvfm@rm4y24fcfzbz/T/#m30d6cea6be48e95c0d824e98a328fb90c7a5766d
and full thread:
https://lore.kernel.org/linux-fsdevel/kn44smk4dgaj5rqmtcfr7ruecixzrik6omur2l2opitn7lbvfm@rm4y24fcfzbz/T/#t

v2:
- productize
- btrfs and tmpfs support

Mateusz Guzik (4):
  fs: speed up path lookup with cheaper MAY_EXEC checks
  ext4: opt-in for IOP_MAY_FAST_EXEC
  btrfs: opt-in for IOP_MAY_FAST_EXEC
  tmpfs: opt-in for IOP_MAY_FAST_EXEC

 fs/attr.c          |  1 +
 fs/btrfs/inode.c   | 12 +++++-
 fs/ext4/inode.c    |  2 +
 fs/ext4/namei.c    |  1 +
 fs/namei.c         | 95 +++++++++++++++++++++++++++++++++++++++++++++-
 fs/posix_acl.c     |  1 +
 fs/xattr.c         |  1 +
 include/linux/fs.h | 21 +++++++---
 mm/shmem.c         |  9 +++++
 9 files changed, 134 insertions(+), 9 deletions(-)

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 1/4] fs: speed up path lookup with cheaper MAY_EXEC checks
  2025-11-06 18:00 [PATCH v2 0/4] permission check avoidance during lookup Mateusz Guzik
@ 2025-11-06 18:00 ` Mateusz Guzik
  2025-11-06 22:45   ` Mateusz Guzik
  2025-11-06 18:01 ` [PATCH v2 2/4] ext4: opt-in for IOP_MAY_FAST_EXEC Mateusz Guzik
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 7+ messages in thread
From: Mateusz Guzik @ 2025-11-06 18:00 UTC (permalink / raw)
  To: brauner
  Cc: viro, jack, linux-kernel, linux-fsdevel, linux-ext4, tytso,
	torvalds, josef, linux-btrfs, Mateusz Guzik

Vast majority of real-world lookups happen on directories which are
traversable by anyone. Figuring out that this holds for a given inode
can be done when instantiating it or changing permissions, avoiding the
overhead during lookup. Stats below.

A simple microbench of stating /usr/include/linux/fs.h on ext4 in a loop
on Sapphire Rapids (ops/s):
before: 3640352
after:  3797258 (+4%)

Filesystems interested in utilizing the feature call inode_enable_fast_may_exec().

Explicit opt-in is necessary as some filesystems have custom inode
permission check hooks which happen to be of no significance for
MAY_EXEC. With an opt-in we know it can be safely ignored. Otherwise any
inode with such a func present would need to be excluded.

inodes which end up skipping perm checks during kernel build grouped
per-fs (0 means checks executed, 1 means skipped):

@[devpts]:
[0, 1)                 2 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|

@[cgroup2]:
[0, 1)                68 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|

@[tmpfs]:
[0, 1)                84 |@@@@@@@@@                                           |
[1, ...)             451 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|

@[sysfs]:
[0, 1)              4532 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|

@[devtmpfs]:
[0, 1)              3609 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@   |
[1, ...)            3790 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|

@[proc]:
[0, 1)             19855 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|

@[ext4]:
[0, 1)            484292 |@@@                                                 |
[1, ...)         7775413 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|

@[btrfs]:
[0, 1)           5628821 |@@@@@                                               |
[1, ...)        52551904 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|

Note that devpts, cgroup2, sysfs and proc are not opting in and thus got
no hits.

gathered with:
bpftrace -e 'kprobe:security_inode_permission { @[str(((struct inode *)arg0)->i_sb->s_type->name)] = lhist(((struct inode *)arg0)->i_opflags & 0x80 ? 1 : 0, 0, 1, 1); }'

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
 fs/attr.c          |  1 +
 fs/namei.c         | 95 +++++++++++++++++++++++++++++++++++++++++++++-
 fs/posix_acl.c     |  1 +
 fs/xattr.c         |  1 +
 include/linux/fs.h | 21 +++++++---
 5 files changed, 111 insertions(+), 8 deletions(-)

diff --git a/fs/attr.c b/fs/attr.c
index 795f231d00e8..572363ff9c6d 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -549,6 +549,7 @@ int notify_change(struct mnt_idmap *idmap, struct dentry *dentry,
 
 	if (!error) {
 		fsnotify_change(dentry, ia_valid);
+		inode_recalc_fast_may_exec(inode);
 		security_inode_post_setattr(idmap, dentry, ia_valid);
 	}
 
diff --git a/fs/namei.c b/fs/namei.c
index a9f9d0453425..bc4bd9114c49 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -602,6 +602,97 @@ int inode_permission(struct mnt_idmap *idmap,
 }
 EXPORT_SYMBOL(inode_permission);
 
+/**
+ * inode_permission_may_exec - Check traversal right for given inode
+ *
+ * This is a special case routine for may_lookup(). Use inode_permission()
+ * instead even if MAY_EXEC is the only thing you want to check for.
+ */
+static __always_inline int inode_permission_may_exec(struct mnt_idmap *idmap,
+	struct inode *inode, int mask)
+{
+	mask |= MAY_EXEC;
+
+	if (!(READ_ONCE(inode->i_opflags) & IOP_FAST_MAY_EXEC))
+		return inode_permission(idmap, inode, mask);
+
+#ifdef CONFIG_DEBUG_VFS
+	/*
+	 * We expect everyone has the execute permission and that there are no
+	 * acls. For debug purposes we validate this indeed holds.
+	 *
+	 * However, we may be racing against setattr and/or setacl, in which case
+	 * we will have to redo the check with the appropriate lock held to avoid
+	 * false-positives.
+	 */
+	unsigned int mode = READ_ONCE(inode->i_mode);
+
+	VFS_BUG_ON_INODE(!S_ISDIR(mode), inode);
+	if (((mode & 0111) != 0111) || !no_acl_inode(inode)) {
+		/*
+		 * If we are in RCU mode may_lookup() will unlazy and try
+		 * again. Worst case if we are still racing the lock will be
+		 * taken below when we get back here.
+		 */
+		if (mask & MAY_NOT_BLOCK)
+			return -ECHILD;
+		inode_lock(inode);
+		if (inode->i_opflags & IOP_FAST_MAY_EXEC) {
+			VFS_BUG_ON_INODE((inode->i_mode & 0111) != 0111, inode);
+			VFS_BUG_ON_INODE(!no_acl_inode(inode), inode);
+		}
+		inode_unlock(inode);
+		return inode_permission(idmap, inode, mask);
+	}
+#endif
+	return security_inode_permission(inode, mask);
+}
+
+/**
+ * inode_recalc_fast_may_exec - recalc IOP_FAST_MAY_EXEC
+ * @inode: Inode to set/unset the bit on
+ *
+ * To be called if the fs considers the inode eligible for short-circuited
+ * permission checks.
+ */
+void inode_recalc_fast_may_exec(struct inode *inode)
+{
+	unsigned int mode;
+	bool wantbit = false;
+
+	if (!(inode_state_read_once(inode) & I_NEW))
+		lockdep_assert_held_write(&inode->i_rwsem);
+
+	if (!(inode->i_flags & S_CAN_FAST_MAY_EXEC)) {
+		VFS_BUG_ON_INODE(inode->i_opflags & IOP_FAST_MAY_EXEC, inode);
+		return;
+	}
+
+	mode = inode->i_mode;
+	if (!S_ISDIR(mode)) {
+		VFS_BUG_ON_INODE(inode->i_opflags & IOP_FAST_MAY_EXEC, inode);
+		return;
+	}
+
+	if (((mode & 0111) == 0111) && no_acl_inode(inode))
+		wantbit = true;
+
+	if (wantbit) {
+		if (inode->i_opflags & IOP_FAST_MAY_EXEC)
+			return;
+		spin_lock(&inode->i_lock);
+		inode->i_opflags |= IOP_FAST_MAY_EXEC;
+		spin_unlock(&inode->i_lock);
+	} else {
+		if (!(inode->i_opflags & IOP_FAST_MAY_EXEC))
+			return;
+		spin_lock(&inode->i_lock);
+		inode->i_opflags &= ~IOP_FAST_MAY_EXEC;
+		spin_unlock(&inode->i_lock);
+	}
+}
+EXPORT_SYMBOL(inode_recalc_fast_may_exec);
+
 /**
  * path_get - get a reference to a path
  * @path: path to get the reference to
@@ -1855,7 +1946,7 @@ static inline int may_lookup(struct mnt_idmap *idmap,
 	int err, mask;
 
 	mask = nd->flags & LOOKUP_RCU ? MAY_NOT_BLOCK : 0;
-	err = inode_permission(idmap, nd->inode, mask | MAY_EXEC);
+	err = inode_permission_may_exec(idmap, nd->inode, mask);
 	if (likely(!err))
 		return 0;
 
@@ -1870,7 +1961,7 @@ static inline int may_lookup(struct mnt_idmap *idmap,
 	if (err != -ECHILD)	// hard error
 		return err;
 
-	return inode_permission(idmap, nd->inode, MAY_EXEC);
+	return inode_permission_may_exec(idmap, nd->inode, 0);
 }
 
 static int reserve_stack(struct nameidata *nd, struct path *link)
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 4050942ab52f..da27dd536058 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -1135,6 +1135,7 @@ int vfs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
 		error = -EIO;
 	if (!error) {
 		fsnotify_xattr(dentry);
+		inode_recalc_fast_may_exec(inode);
 		security_inode_post_set_acl(dentry, acl_name, kacl);
 	}
 
diff --git a/fs/xattr.c b/fs/xattr.c
index 8851a5ef34f5..917946a7f367 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -235,6 +235,7 @@ int __vfs_setxattr_noperm(struct mnt_idmap *idmap,
 				       size, flags);
 		if (!error) {
 			fsnotify_xattr(dentry);
+			inode_recalc_fast_may_exec(inode);
 			security_inode_post_setxattr(dentry, name, value,
 						     size, flags);
 		}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 03e450dd5211..4f9962dfe2e6 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -647,13 +647,14 @@ is_uncached_acl(struct posix_acl *acl)
 	return (long)acl & 1;
 }
 
-#define IOP_FASTPERM	0x0001
-#define IOP_LOOKUP	0x0002
-#define IOP_NOFOLLOW	0x0004
-#define IOP_XATTR	0x0008
+#define IOP_FASTPERM		0x0001
+#define IOP_LOOKUP		0x0002
+#define IOP_NOFOLLOW		0x0004
+#define IOP_XATTR		0x0008
 #define IOP_DEFAULT_READLINK	0x0010
-#define IOP_MGTIME	0x0020
-#define IOP_CACHED_LINK	0x0040
+#define IOP_MGTIME		0x0020
+#define IOP_CACHED_LINK		0x0040
+#define IOP_FAST_MAY_EXEC	0x0080
 
 /*
  * Inode state bits.  Protected by inode->i_lock
@@ -2128,6 +2129,7 @@ extern loff_t vfs_dedupe_file_range_one(struct file *src_file, loff_t src_pos,
 #define S_VERITY	(1 << 16) /* Verity file (using fs/verity/) */
 #define S_KERNEL_FILE	(1 << 17) /* File is in use by the kernel (eg. fs/cachefiles) */
 #define S_ANON_INODE	(1 << 19) /* Inode is an anonymous inode */
+#define S_CAN_FAST_MAY_EXEC (1 << 20) /* Inode is eligible for IOP_FAST_MAY_EXEC */
 
 /*
  * Note that nosuid etc flags are inode-specific: setting some file-system
@@ -2904,6 +2906,13 @@ static inline int inode_init_always(struct super_block *sb, struct inode *inode)
 	return inode_init_always_gfp(sb, inode, GFP_NOFS);
 }
 
+void inode_recalc_fast_may_exec(struct inode *);
+static inline void inode_enable_fast_may_exec(struct inode *inode)
+{
+	inode->i_flags |= S_CAN_FAST_MAY_EXEC;
+	inode_recalc_fast_may_exec(inode);
+}
+
 extern void inode_init_once(struct inode *);
 extern void address_space_init_once(struct address_space *mapping);
 extern struct inode * igrab(struct inode *);
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/4] ext4: opt-in for IOP_MAY_FAST_EXEC
  2025-11-06 18:00 [PATCH v2 0/4] permission check avoidance during lookup Mateusz Guzik
  2025-11-06 18:00 ` [PATCH v2 1/4] fs: speed up path lookup with cheaper MAY_EXEC checks Mateusz Guzik
@ 2025-11-06 18:01 ` Mateusz Guzik
  2025-11-06 18:01 ` [PATCH v2 3/4] btrfs: " Mateusz Guzik
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Mateusz Guzik @ 2025-11-06 18:01 UTC (permalink / raw)
  To: brauner
  Cc: viro, jack, linux-kernel, linux-fsdevel, linux-ext4, tytso,
	torvalds, josef, linux-btrfs, Mateusz Guzik

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
 fs/ext4/inode.c | 2 ++
 fs/ext4/namei.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index eaf776cd4175..7d5369f66686 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5522,6 +5522,8 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
 	if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR) && !ei->i_file_acl)
 		cache_no_acl(inode);
 
+	inode_enable_fast_may_exec(inode);
+
 	unlock_new_inode(inode);
 	return inode;
 
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 2cd36f59c9e3..870bee252e54 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -3038,6 +3038,7 @@ static struct dentry *ext4_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 	err = ext4_mark_inode_dirty(handle, dir);
 	if (err)
 		goto out_clear_inode;
+	inode_enable_fast_may_exec(inode);
 	d_instantiate_new(dentry, inode);
 	ext4_fc_track_create(handle, dentry);
 	if (IS_DIRSYNC(dir))
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 3/4] btrfs: opt-in for IOP_MAY_FAST_EXEC
  2025-11-06 18:00 [PATCH v2 0/4] permission check avoidance during lookup Mateusz Guzik
  2025-11-06 18:00 ` [PATCH v2 1/4] fs: speed up path lookup with cheaper MAY_EXEC checks Mateusz Guzik
  2025-11-06 18:01 ` [PATCH v2 2/4] ext4: opt-in for IOP_MAY_FAST_EXEC Mateusz Guzik
@ 2025-11-06 18:01 ` Mateusz Guzik
  2025-11-06 18:01 ` [PATCH v2 4/4] tmpfs: " Mateusz Guzik
  2025-11-06 19:34 ` [PATCH v2 0/4] permission check avoidance during lookup Mateusz Guzik
  4 siblings, 0 replies; 7+ messages in thread
From: Mateusz Guzik @ 2025-11-06 18:01 UTC (permalink / raw)
  To: brauner
  Cc: viro, jack, linux-kernel, linux-fsdevel, linux-ext4, tytso,
	torvalds, josef, linux-btrfs, Mateusz Guzik

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
 fs/btrfs/inode.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 42da39c1e5b5..42df687a0126 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5852,6 +5852,8 @@ struct btrfs_inode *btrfs_iget(u64 ino, struct btrfs_root *root)
 	if (ret)
 		return ERR_PTR(ret);
 
+	if (S_ISDIR(inode->vfs_inode.i_mode))
+		inode_enable_fast_may_exec(&inode->vfs_inode);
 	unlock_new_inode(&inode->vfs_inode);
 	return inode;
 }
@@ -6803,8 +6805,11 @@ static int btrfs_create_common(struct inode *dir, struct dentry *dentry,
 	}
 
 	ret = btrfs_create_new_inode(trans, &new_inode_args);
-	if (!ret)
+	if (!ret) {
+		if (S_ISDIR(inode->i_mode))
+			inode_enable_fast_may_exec(inode);
 		d_instantiate_new(dentry, inode);
+	}
 
 	btrfs_end_transaction(trans);
 	btrfs_btree_balance_dirty(fs_info);
@@ -9163,6 +9168,11 @@ int btrfs_prealloc_file_range_trans(struct inode *inode,
 					   min_size, actual_len, alloc_hint, trans);
 }
 
+/*
+ * NOTE: in case you are adding MAY_EXEC check for directories:
+ * inode_enable_fast_may_exec() is issued when inodes get instantiated, meaning
+ * calls to this place can be elided.
+ */
 static int btrfs_permission(struct mnt_idmap *idmap,
 			    struct inode *inode, int mask)
 {
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 4/4] tmpfs: opt-in for IOP_MAY_FAST_EXEC
  2025-11-06 18:00 [PATCH v2 0/4] permission check avoidance during lookup Mateusz Guzik
                   ` (2 preceding siblings ...)
  2025-11-06 18:01 ` [PATCH v2 3/4] btrfs: " Mateusz Guzik
@ 2025-11-06 18:01 ` Mateusz Guzik
  2025-11-06 19:34 ` [PATCH v2 0/4] permission check avoidance during lookup Mateusz Guzik
  4 siblings, 0 replies; 7+ messages in thread
From: Mateusz Guzik @ 2025-11-06 18:01 UTC (permalink / raw)
  To: brauner
  Cc: viro, jack, linux-kernel, linux-fsdevel, linux-ext4, tytso,
	torvalds, josef, linux-btrfs, Mateusz Guzik

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
 mm/shmem.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/mm/shmem.c b/mm/shmem.c
index c819cecf1ed9..265456bc6bf0 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3106,6 +3106,15 @@ static struct inode *__shmem_get_inode(struct mnt_idmap *idmap,
 	}
 
 	lockdep_annotate_inode_mutex_key(inode);
+
+	if (S_ISDIR(mode)) {
+		WARN_ON_ONCE(inode_state_read_once(inode) & I_NEW);
+		/* satisfy an assert inside */
+		inode_state_set_raw(inode, I_NEW);
+		inode_enable_fast_may_exec(inode);
+		inode_state_clear_raw(inode, I_NEW);
+	}
+
 	return inode;
 }
 
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 0/4] permission check avoidance during lookup
  2025-11-06 18:00 [PATCH v2 0/4] permission check avoidance during lookup Mateusz Guzik
                   ` (3 preceding siblings ...)
  2025-11-06 18:01 ` [PATCH v2 4/4] tmpfs: " Mateusz Guzik
@ 2025-11-06 19:34 ` Mateusz Guzik
  4 siblings, 0 replies; 7+ messages in thread
From: Mateusz Guzik @ 2025-11-06 19:34 UTC (permalink / raw)
  To: brauner
  Cc: viro, jack, linux-kernel, linux-fsdevel, linux-ext4, tytso,
	torvalds, josef, linux-btrfs

On Thu, Nov 6, 2025 at 7:01 PM Mateusz Guzik <mjguzik@gmail.com> wrote:
>
> To quote from patch 1:
> <quote>
> Vast majority of real-world lookups happen on directories which are
> traversable by anyone. Figuring out that this holds for a given inode
> can be done when instantiating it or changing permissions, avoiding the
> overhead during lookup. Stats below.
>
> A simple microbench of stating /usr/include/linux/fs.h on ext4 in a loop
> on Sapphire Rapids (ops/s):
> before: 3640352
> after:  3797258 (+4%)
> </quote>
>
> During a kernel build about 90% of all lookups managed to skip
> permission checks in my setup, see the commit message for a breakdown.
>
> WARNING: more testing is needed for correctness, but I'm largely happy
> with the state as is.
>

Forgot to explain more in the commit message, so here it is right now:
how almost the entirety of inode_permission() can get elided for
inodes which qualify for it.
inode_permission()
{
        retval = sb_permission(inode->i_sb, inode, mask);
        if (unlikely(retval))
                return retval;

sb_permission starts with a check for mask & MAY_WRITE. Since mask is
MAY_EXEC, this does not need to execute.

        if (unlikely(mask & MAY_WRITE)) {

Same here.

        retval = do_inode_permission(idmap, inode, mask);
        if (unlikely(retval))
                return retval;

do_inode_permission starts with a check for IOP_FASTPERM. This is of
no relevance as we are skipping the perm checks and the behavior is as
if generic_permission() was always called.

Then generic_permission:
        ret = acl_permission_check(idmap, inode, mask);
        if (ret != -EACCES)
                return ret;

We don't have to check the error code as we expect the perm is granted.

acl_permission_check:
       if (!((mask & 7) * 0111 & ~mode)) {
                if (no_acl_inode(inode))
                        return 0;
                if (!IS_POSIXACL(inode))
                        return 0;
        }

We don't need this as we already pre-checked the perm is at least 0111
and there are no acls set.

back to inode_permission:
        retval = devcgroup_inode_permission(inode, mask);
        if (unlikely(retval))
                return retval;

This checks if we are dealing with a device. The IOP_FAST_MAY_EXEC is
only legally set on directories, so it is an invariant we are not
dealing with a device and don't need to check that.

Finally this:
        return security_inode_permission(inode, mask);

.. *does* execute in the new scheme.

However, LSM has notpatchable calls inside and only 2 users, normally
not present on Ubuntu et al. Or to put it differently, this is a func
call to a nop slide on most kernels and with some extra work can also
get elided.

> WARNING: I'm assuming the following bit is applied:
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 78ea864fa8cd..eaf776cd4175 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5518,6 +5518,10 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
>                 goto bad_inode;
>         brelse(iloc.bh);
>
> +       /* Initialize the "no ACL's" state for the simple cases */
> +       if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR) && !ei->i_file_acl)
> +               cache_no_acl(inode);
> +
>         unlock_new_inode(inode);
>         return inode;
>
> Lack of the patch does not affect correctness, but it does make the
> patch ineffective for ext4. I did not include it in the posting as other
> people promised to sort it out.
>
> Discussion is here with an ack from Jan:
> https://lore.kernel.org/linux-fsdevel/kn44smk4dgaj5rqmtcfr7ruecixzrik6omur2l2opitn7lbvfm@rm4y24fcfzbz/T/#m30d6cea6be48e95c0d824e98a328fb90c7a5766d
> and full thread:
> https://lore.kernel.org/linux-fsdevel/kn44smk4dgaj5rqmtcfr7ruecixzrik6omur2l2opitn7lbvfm@rm4y24fcfzbz/T/#t
>
> v2:
> - productize
> - btrfs and tmpfs support
>
> Mateusz Guzik (4):
>   fs: speed up path lookup with cheaper MAY_EXEC checks
>   ext4: opt-in for IOP_MAY_FAST_EXEC
>   btrfs: opt-in for IOP_MAY_FAST_EXEC
>   tmpfs: opt-in for IOP_MAY_FAST_EXEC
>
>  fs/attr.c          |  1 +
>  fs/btrfs/inode.c   | 12 +++++-
>  fs/ext4/inode.c    |  2 +
>  fs/ext4/namei.c    |  1 +
>  fs/namei.c         | 95 +++++++++++++++++++++++++++++++++++++++++++++-
>  fs/posix_acl.c     |  1 +
>  fs/xattr.c         |  1 +
>  include/linux/fs.h | 21 +++++++---
>  mm/shmem.c         |  9 +++++
>  9 files changed, 134 insertions(+), 9 deletions(-)
>
> --
> 2.48.1
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/4] fs: speed up path lookup with cheaper MAY_EXEC checks
  2025-11-06 18:00 ` [PATCH v2 1/4] fs: speed up path lookup with cheaper MAY_EXEC checks Mateusz Guzik
@ 2025-11-06 22:45   ` Mateusz Guzik
  0 siblings, 0 replies; 7+ messages in thread
From: Mateusz Guzik @ 2025-11-06 22:45 UTC (permalink / raw)
  To: brauner
  Cc: viro, jack, linux-kernel, linux-fsdevel, linux-ext4, tytso,
	torvalds, josef, linux-btrfs

On Thu, Nov 6, 2025 at 7:01 PM Mateusz Guzik <mjguzik@gmail.com> wrote:
> diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> index 4050942ab52f..da27dd536058 100644
> --- a/fs/posix_acl.c
> +++ b/fs/posix_acl.c
> @@ -1135,6 +1135,7 @@ int vfs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
>                 error = -EIO;
>         if (!error) {
>                 fsnotify_xattr(dentry);
> +               inode_recalc_fast_may_exec(inode);
>                 security_inode_post_set_acl(dentry, acl_name, kacl);
>         }
>

I just some found I missed some spots which need to call
inode_recalc_fast_may_exec().

I plugged them locally, but now I have to chew on whether this is
worth the potential bugs.

Preferably there would be magic catching all changes to i_mode and
acls requiring inode_recalc_fast_may_exec() is called before the inode
gets unlocked.

I'm going to play with implementing inode_permission_may_setattr which
starts with explicitly checking for i_mode being available for
everyone and no acls being present, maybe that's already almost
entirety of the speed up with no woes like the above to worry about.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-11-06 22:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-06 18:00 [PATCH v2 0/4] permission check avoidance during lookup Mateusz Guzik
2025-11-06 18:00 ` [PATCH v2 1/4] fs: speed up path lookup with cheaper MAY_EXEC checks Mateusz Guzik
2025-11-06 22:45   ` Mateusz Guzik
2025-11-06 18:01 ` [PATCH v2 2/4] ext4: opt-in for IOP_MAY_FAST_EXEC Mateusz Guzik
2025-11-06 18:01 ` [PATCH v2 3/4] btrfs: " Mateusz Guzik
2025-11-06 18:01 ` [PATCH v2 4/4] tmpfs: " Mateusz Guzik
2025-11-06 19:34 ` [PATCH v2 0/4] permission check avoidance during lookup Mateusz Guzik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).