On Mon, Jan 29, 2024 at 05:43:24PM -0300, Gabriel Krisman Bertazi wrote:
> Now that we do more than just clear the DCACHE_NOKEY_NAME in
> fscrypt_handle_d_move, skip it entirely for plaintext dentries, to avoid
> extra costs.
>
> Note that VFS will call this function for any dentry, whether the volume
> has fscrypt on not. But, since we only care about DCACHE_NOKEY_NAME, we
> can check for that, to avoid touching the superblock for other fields
> that identify a fscrypt volume.
>
> Note also that fscrypt_handle_d_move is hopefully inlined back into
> __d_move, so the call cost is not significant. Considering that
> DCACHE_NOKEY_NAME is a fscrypt-specific flag, we do the check in fscrypt
> code instead of the caller.
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@xxxxxxx>
>
> ---
> Changes since v4:
> - Check based on the dentry itself (eric)
> ---
> include/linux/fscrypt.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index c1e285053b3e..ab668760d63e 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -232,6 +232,15 @@ static inline bool fscrypt_needs_contents_encryption(const struct inode *inode)
> */
> static inline void fscrypt_handle_d_move(struct dentry *dentry)
> {
> + /*
> + * VFS calls fscrypt_handle_d_move even for non-fscrypt
> + * filesystems. Since we only care about DCACHE_NOKEY_NAME
> + * dentries here, check that to bail out quickly, if possible.
> + */
> + if (!(dentry->d_flags & DCACHE_NOKEY_NAME))
> + return;
I think you're over-complicating this a bit. This should be merged with patch
5, since this is basically fixing patch 5, and the end result should look like:
/*
* When d_splice_alias() moves a directory's no-key alias to its plaintext alias
* as a result of the encryption key being added, DCACHE_NOKEY_NAME must be
* cleared and there might be an opportunity to disable d_revalidate. Note that
* we don't have to support the inverse operation because fscrypt doesn't allow
* no-key names to be the source or target of a rename().
*/
static inline void fscrypt_handle_d_move(struct dentry *dentry)
{
if (dentry->d_flags & DCACHE_NOKEY_NAME) {
dentry->d_flags &= ~DCACHE_NOKEY_NAME;
if (dentry->d_op->d_revalidate == fscrypt_d_revalidate)
dentry->d_flags &= ~DCACHE_OP_REVALIDATE;
}
}
Note that checking for NULL dentry->d_op is not necessary, since it's already
been verified that DCACHE_NOKEY_NAME is set, which means fscrypt is in use,
which means that there are dentry_operations.
- Eric
