diff mbox series

[v2,2/8] fscrypt: Drop d_revalidate if key is available

Message ID 20231215211608.6449-3-krisman@suse.de
State Not Applicable
Headers show
Series Revert setting casefolding dentry operations through s_d_op | expand

Commit Message

Gabriel Krisman Bertazi Dec. 15, 2023, 9:16 p.m. UTC
fscrypt dentries are always valid once the key is available.  Since the
key cannot be removed without evicting the dentry, we don't need to keep
retrying to revalidate it.

Signed-off-by: Gabriel Krisman Bertazi <krisman@suse.de>
---
 fs/crypto/fname.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Eric Biggers Dec. 19, 2023, 11 p.m. UTC | #1
On Fri, Dec 15, 2023 at 04:16:02PM -0500, Gabriel Krisman Bertazi wrote:
> fscrypt dentries are always valid once the key is available.  Since the
> key cannot be removed without evicting the dentry, we don't need to keep
> retrying to revalidate it.
> 
> Signed-off-by: Gabriel Krisman Bertazi <krisman@suse.de>

IIUC, this patch minimizes the overhead of fscrypt_d_revalidate() both for
encrypted and unencrypted dentries.  That's what's needed (seeing as this series
makes fscrypt_d_revalidate be installed on unencrypted dentries), but the commit
message only mentions the encrypted case.  It would be helpful to mention both.

> ---
>  fs/crypto/fname.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index 7b3fc189593a..0457ba2d7d76 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -591,8 +591,15 @@ int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
>  	 * reverting to no-key names without evicting the directory's inode
>  	 * -- which implies eviction of the dentries in the directory.
>  	 */
> -	if (!(dentry->d_flags & DCACHE_NOKEY_NAME))
> +	if (!(dentry->d_flags & DCACHE_NOKEY_NAME)) {
> +		/*
> +		 * If fscrypt is the only feature requiring
> +		 * revalidation for this dentry, we can just disable it.
> +		 */
> +		if (dentry->d_op->d_revalidate == &fscrypt_d_revalidate)

No need for the & in &fscrypt_d_revalidate.

- Eric
Al Viro Dec. 21, 2023, 7:14 a.m. UTC | #2
On Fri, Dec 15, 2023 at 04:16:02PM -0500, Gabriel Krisman Bertazi wrote:
> fscrypt dentries are always valid once the key is available.  Since the
> key cannot be removed without evicting the dentry, we don't need to keep
> retrying to revalidate it.
> 
> Signed-off-by: Gabriel Krisman Bertazi <krisman@suse.de>
> ---
>  fs/crypto/fname.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index 7b3fc189593a..0457ba2d7d76 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -591,8 +591,15 @@ int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
>  	 * reverting to no-key names without evicting the directory's inode
>  	 * -- which implies eviction of the dentries in the directory.
>  	 */
> -	if (!(dentry->d_flags & DCACHE_NOKEY_NAME))
> +	if (!(dentry->d_flags & DCACHE_NOKEY_NAME)) {
> +		/*
> +		 * If fscrypt is the only feature requiring
> +		 * revalidation for this dentry, we can just disable it.
> +		 */
> +		if (dentry->d_op->d_revalidate == &fscrypt_d_revalidate)

Umm...  What about ceph?  IOW, why do we care how had we gotten to that
function - directly via ->d_revalidate() or from ->d_revalidate() instance?
Al Viro Dec. 21, 2023, 7:19 a.m. UTC | #3
On Thu, Dec 21, 2023 at 07:14:02AM +0000, Al Viro wrote:
> On Fri, Dec 15, 2023 at 04:16:02PM -0500, Gabriel Krisman Bertazi wrote:
> > fscrypt dentries are always valid once the key is available.  Since the
> > key cannot be removed without evicting the dentry, we don't need to keep
> > retrying to revalidate it.
> > 
> > Signed-off-by: Gabriel Krisman Bertazi <krisman@suse.de>
> > ---
> >  fs/crypto/fname.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> > index 7b3fc189593a..0457ba2d7d76 100644
> > --- a/fs/crypto/fname.c
> > +++ b/fs/crypto/fname.c
> > @@ -591,8 +591,15 @@ int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
> >  	 * reverting to no-key names without evicting the directory's inode
> >  	 * -- which implies eviction of the dentries in the directory.
> >  	 */
> > -	if (!(dentry->d_flags & DCACHE_NOKEY_NAME))
> > +	if (!(dentry->d_flags & DCACHE_NOKEY_NAME)) {
> > +		/*
> > +		 * If fscrypt is the only feature requiring
> > +		 * revalidation for this dentry, we can just disable it.
> > +		 */
> > +		if (dentry->d_op->d_revalidate == &fscrypt_d_revalidate)
> 
> Umm...  What about ceph?  IOW, why do we care how had we gotten to that
> function - directly via ->d_revalidate() or from ->d_revalidate() instance?

Nevermind.
diff mbox series

Patch

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 7b3fc189593a..0457ba2d7d76 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -591,8 +591,15 @@  int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 	 * reverting to no-key names without evicting the directory's inode
 	 * -- which implies eviction of the dentries in the directory.
 	 */
-	if (!(dentry->d_flags & DCACHE_NOKEY_NAME))
+	if (!(dentry->d_flags & DCACHE_NOKEY_NAME)) {
+		/*
+		 * If fscrypt is the only feature requiring
+		 * revalidation for this dentry, we can just disable it.
+		 */
+		if (dentry->d_op->d_revalidate == &fscrypt_d_revalidate)
+			d_set_always_valid(dentry);
 		return 1;
+	}
 
 	/*
 	 * No-key name; valid if the directory's key is still unavailable.