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
