[PATCH] ceph: shrink dcache when adding a key

[PATCH] ceph: shrink dcache when adding a key

[Jeff Layton]

Any extant dentries under a directory will be invalid once a key is
added to the directory. Prune any child dentries of the parent after
adding a key.

Cc: Luís Henriques <lhenriques@suse.de>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/ioctl.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

This one is for the ceph+fscrypt series.

Luis, this seems to fix 580 and 593 for me. 595 still fails with it, but
that one is more related to file contents.

diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
index 9675ef3a6c47..12d5469c5df2 100644
--- a/fs/ceph/ioctl.c
+++ b/fs/ceph/ioctl.c
@@ -397,7 +397,10 @@ long ceph_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		ret = vet_mds_for_fscrypt(file);
 		if (ret)
 			return ret;
-		return fscrypt_ioctl_add_key(file, (void __user *)arg);
+		ret = fscrypt_ioctl_add_key(file, (void __user *)arg);
+		if (ret == 0)
+			shrink_dcache_parent(file_dentry(file));
+		return ret;
 
 	case FS_IOC_REMOVE_ENCRYPTION_KEY:
 		return fscrypt_ioctl_remove_key(file, (void __user *)arg);
-- 
2.35.1

Re: [PATCH] ceph: shrink dcache when adding a key

[Luís Henriques]

Jeff Layton <jlayton@kernel.org> writes:

> Any extant dentries under a directory will be invalid once a key is
> added to the directory. Prune any child dentries of the parent after
> adding a key.
>
> Cc: Luís Henriques <lhenriques@suse.de>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/ioctl.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> This one is for the ceph+fscrypt series.
>
> Luis, this seems to fix 580 and 593 for me. 595 still fails with it, but
> that one is more related to file contents.
>
> diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
> index 9675ef3a6c47..12d5469c5df2 100644
> --- a/fs/ceph/ioctl.c
> +++ b/fs/ceph/ioctl.c
> @@ -397,7 +397,10 @@ long ceph_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  		ret = vet_mds_for_fscrypt(file);
>  		if (ret)
>  			return ret;
> -		return fscrypt_ioctl_add_key(file, (void __user *)arg);
> +		ret = fscrypt_ioctl_add_key(file, (void __user *)arg);
> +		if (ret == 0)
> +			shrink_dcache_parent(file_dentry(file));
> +		return ret;

OK, a problem with this is that we're using the big hammer: this ioctl is
being executed on the filesystem root and not the directory you're
expecting.  This is because keys can used for more than one directory.
So, the performance penalty with this fix is probably not acceptable.

Cheers,
-- 
Luís

>  
>  	case FS_IOC_REMOVE_ENCRYPTION_KEY:
>  		return fscrypt_ioctl_remove_key(file, (void __user *)arg);
> -- 
>
> 2.35.1
>

Re: [PATCH] ceph: shrink dcache when adding a key

[Jeff Layton]

On Mon, 2022-03-28 at 17:09 +0100, Luís Henriques wrote:
> Jeff Layton <jlayton@kernel.org> writes:
> 
> > Any extant dentries under a directory will be invalid once a key is
> > added to the directory. Prune any child dentries of the parent after
> > adding a key.
> > 
> > Cc: Luís Henriques <lhenriques@suse.de>
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/ceph/ioctl.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > This one is for the ceph+fscrypt series.
> > 
> > Luis, this seems to fix 580 and 593 for me. 595 still fails with it, but
> > that one is more related to file contents.
> > 
> > diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
> > index 9675ef3a6c47..12d5469c5df2 100644
> > --- a/fs/ceph/ioctl.c
> > +++ b/fs/ceph/ioctl.c
> > @@ -397,7 +397,10 @@ long ceph_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> >  		ret = vet_mds_for_fscrypt(file);
> >  		if (ret)
> >  			return ret;
> > -		return fscrypt_ioctl_add_key(file, (void __user *)arg);
> > +		ret = fscrypt_ioctl_add_key(file, (void __user *)arg);
> > +		if (ret == 0)
> > +			shrink_dcache_parent(file_dentry(file));
> > +		return ret;
> 
> OK, a problem with this is that we're using the big hammer: this ioctl is
> being executed on the filesystem root and not the directory you're
> expecting.  This is because keys can used for more than one directory.
> So, the performance penalty with this fix is probably not acceptable.
> 

file represents an open file description, so file_dentry should be
wherever its path terminates. So, I think this should end up getting run
on the correct dentry.

Oh, but looking at the fscrypt command line application, yeah it does
seem to open the root of the fs, so this would end up purging a lot more
dentries than intended.

Fair enough...I'll drop this one. We'll need to think of some other way
to fix this.
-- 
Jeff Layton <jlayton@kernel.org>