| Message ID | 20240129204330.32346-7-krisman@suse.de |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Set casefold/fscrypt dentry operations through sb->s_d_op | expand |
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@suse.de> > > --- > 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
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; + + /* Mark the dentry as a plaintext dentry. */ dentry->d_flags &= ~DCACHE_NOKEY_NAME; /*
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@suse.de> --- Changes since v4: - Check based on the dentry itself (eric) --- include/linux/fscrypt.h | 9 +++++++++ 1 file changed, 9 insertions(+)