[PATCH] fs: optimize chown_common by skipping unnecessary ownership changes

[PATCH] fs: optimize chown_common by skipping unnecessary ownership changes

[jayxu1990]

From: Jay Xu <jayxu1990@gmail.com>

Add early return optimization to chown_common() when the requested
uid/gid already matches the current inode ownership. This avoids
calling notify_change() and associated filesystem operations when
no actual change is needed.

The check is performed after acquiring the inode lock to ensure
atomicity and uses the kernel's uid_eq()/gid_eq() functions for
proper comparison.

This optimization provides several benefits:
- Reduces unnecessary filesystem metadata updates and journal writes
- Prevents redundant storage I/O when files are on persistent storage
- Improves performance for recursive chown operations that encounter
  files with already-correct ownership
- Avoids invoking security hooks and filesystem-specific setattr
  operations when no change is required

Signed-off-by: Jay Xu <jayxu1990@gmail.com>
---
 fs/open.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/fs/open.c b/fs/open.c
index 3d64372ecc67..82bde70c6c08 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -761,6 +761,7 @@ int chown_common(const struct path *path, uid_t user, gid_t group)
 	struct iattr newattrs;
 	kuid_t uid;
 	kgid_t gid;
+	bool needs_update = false;
 
 	uid = make_kuid(current_user_ns(), user);
 	gid = make_kgid(current_user_ns(), group);
@@ -779,6 +780,17 @@ int chown_common(const struct path *path, uid_t user, gid_t group)
 	error = inode_lock_killable(inode);
 	if (error)
 		return error;
+
+	/* Check if ownership actually needs to change */
+	if ((newattrs.ia_valid & ATTR_UID) && !uid_eq(inode->i_uid, uid))
+		needs_update = true;
+	if ((newattrs.ia_valid & ATTR_GID) && !gid_eq(inode->i_gid, gid))
+		needs_update = true;
+
+	if (!needs_update) {
+		inode_unlock(inode);
+		return 0;
+	}
 	if (!S_ISDIR(inode->i_mode))
 		newattrs.ia_valid |= ATTR_KILL_SUID | ATTR_KILL_PRIV |
 				     setattr_should_drop_sgid(idmap, inode);
-- 
2.34.1

Re: [PATCH] fs: optimize chown_common by skipping unnecessary ownership changes

[Mateusz Guzik]

On Thu, Nov 13, 2025 at 09:34:49AM +0800, jayxu1990@gmail.com wrote:
> From: Jay Xu <jayxu1990@gmail.com>
> 
> Add early return optimization to chown_common() when the requested
> uid/gid already matches the current inode ownership. This avoids
> calling notify_change() and associated filesystem operations when
> no actual change is needed.
> 

If this is useful in practice, then so is probably chmod.

> The check is performed after acquiring the inode lock to ensure
> atomicity and uses the kernel's uid_eq()/gid_eq() functions for
> proper comparison.
> 
> This optimization provides several benefits:
> - Reduces unnecessary filesystem metadata updates and journal writes
> - Prevents redundant storage I/O when files are on persistent storage
> - Improves performance for recursive chown operations that encounter
>   files with already-correct ownership
> - Avoids invoking security hooks and filesystem-specific setattr
>   operations when no change is required

The last bit is a breaking change in behavior though. For example right
now invoking chown 0:0 /bin as an unprivileged user fails. It will succeed
with your patch.

iow you can't avoid any of the security checks.

However, if there are real workloads which chown/chmod to the current
value already, perhaps it would make sense for the routine to track if
anything changed and if not to avoid dirtying the inode, which still
might save on I/O.

> 
> Signed-off-by: Jay Xu <jayxu1990@gmail.com>
> ---
>  fs/open.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/fs/open.c b/fs/open.c
> index 3d64372ecc67..82bde70c6c08 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -761,6 +761,7 @@ int chown_common(const struct path *path, uid_t user, gid_t group)
>  	struct iattr newattrs;
>  	kuid_t uid;
>  	kgid_t gid;
> +	bool needs_update = false;
>  
>  	uid = make_kuid(current_user_ns(), user);
>  	gid = make_kgid(current_user_ns(), group);
> @@ -779,6 +780,17 @@ int chown_common(const struct path *path, uid_t user, gid_t group)
>  	error = inode_lock_killable(inode);
>  	if (error)
>  		return error;
> +
> +	/* Check if ownership actually needs to change */
> +	if ((newattrs.ia_valid & ATTR_UID) && !uid_eq(inode->i_uid, uid))
> +		needs_update = true;
> +	if ((newattrs.ia_valid & ATTR_GID) && !gid_eq(inode->i_gid, gid))
> +		needs_update = true;
> +
> +	if (!needs_update) {
> +		inode_unlock(inode);
> +		return 0;
> +	}
>  	if (!S_ISDIR(inode->i_mode))
>  		newattrs.ia_valid |= ATTR_KILL_SUID | ATTR_KILL_PRIV |
>  				     setattr_should_drop_sgid(idmap, inode);
> -- 
> 2.34.1
>