Re: [PATCH v3 07/13] fs: drop i_crypt_info from struct inode

[Eric Biggers <ebiggers@kernel.org>]

On Tue, Jul 22, 2025 at 09:27:25PM +0200, Christian Brauner wrote:
> @@ -799,12 +799,11 @@ void fscrypt_put_encryption_info(struct inode *inode)
>  {
>  	struct fscrypt_inode_info **crypt_info;
>  
> -	if (inode->i_sb->s_op->i_fscrypt)
> +	if (inode->i_sb->s_op->i_fscrypt) {
>  		crypt_info = fscrypt_addr(inode);
> -	else
> -		crypt_info = &inode->i_crypt_info;
> -	put_crypt_info(*crypt_info);
> -	*crypt_info = NULL;
> +		put_crypt_info(*crypt_info);
> +		*crypt_info = NULL;
> +	}
>  }

This could use an IS_ENCRYPTED(inode) check at the beginning, to
minimize the overhead on unencrypted files.  Before we just loaded
inode:i_crypt_info, but now that accessing the fscrypt_inode_info will
be more expensive it would be worthwhile to check IS_ENCRYPTED() first.

>  static inline struct fscrypt_inode_info *
> @@ -232,15 +224,14 @@ fscrypt_get_inode_info(const struct inode *inode)
>  {
>  	/*
>  	 * Pairs with the cmpxchg_release() in fscrypt_setup_encryption_info().
> -	 * I.e., another task may publish ->i_crypt_info concurrently, executing
> +	 * I.e., another task may publish ->i_fscrypt_info concurrently, executing
>  	 * a RELEASE barrier.  We need to use smp_load_acquire() here to safely
>  	 * ACQUIRE the memory the other task published.
>  	 */
>  
>  	if (inode->i_sb->s_op->i_fscrypt)
>  		return smp_load_acquire(fscrypt_addr(inode));
> -
> -	return smp_load_acquire(&inode->i_crypt_info);
> +	return NULL;

The conditional here shouldn't be needed, since this should be called
only on filesystems that support encryption.  Did you find a case where
it isn't?

- Eric