Re: [PATCH RFC 0/2] ceph: size handling for the fscrypt

From: Jeff Layton <jlayton@kernel.org>
To: xiubli@redhat.com
Cc: idryomov@gmail.com, ukernel@gmail.com, pdonnell@redhat.com,
	ceph-devel@vger.kernel.org
Subject: Re: [PATCH RFC 0/2] ceph: size handling for the fscrypt
Date: Tue, 07 Sep 2021 08:35:06 -0400	[thread overview]
Message-ID: <02f2f77423ec1e6e5b23b452716b21c36a5b67da.camel@kernel.org> (raw)
In-Reply-To: <20210903081510.982827-1-xiubli@redhat.com>

On Fri, 2021-09-03 at 16:15 +0800, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
> 
> This patch series is based Jeff's ceph-fscrypt-size-experimental
> branch in https://git.kernel.org/pub/scm/linux/kernel/git/jlayton/linux.git.
> 
> This is just a draft patch and need to rebase or recode after Jeff
> finished his huge patch set.
> 
> Post the patch out for advices and ideas. Thanks.
> 

I'll take a look. Going forward though, it'd probably be best for you to
just take over development of the entire ceph-fscrypt-size series
instead of trying to develop on top of my branch.

That branch is _very_ rough anyway. Just clone the branch into your tree
and then you can drop or change patches in it as you see fit.

> ====
> 
> This approach will not do the rmw immediately after the file is
> truncated. If the truncate size is aligned to the BLOCK SIZE, so
> there no need to do the rmw and only in unaligned case will the
> rmw is needed.
> 
> And the 'fscrypt_file' field will be cleared after the rmw is done.
> If the 'fscrypt_file' is none zero that means after the kclient
> reading that block to local buffer or pagecache it needs to do the
> zeroing of that block in range of [fscrypt_file, round_up(fscrypt_file,
> BLOCK SIZE)).
> 
> Once any kclient has dirty that block and write it back to ceph, the
> 'fscrypt_file' field will be cleared and set to 0. More detail please
> see the commit comments in the second patch.
> 

That sounds odd. How do you know where the file ends once you zero out
fscrypt_file?

/me goes to look at the patches...

> There also need on small work in Jeff's MDS PR in cap flushing code
> to clear the 'fscrypt_file'.
> 
> 
> Xiubo Li (2):
>   Revert "ceph: make client zero partial trailing block on truncate"
>   ceph: truncate the file contents when needed when file scrypted
> 
>  fs/ceph/addr.c  | 19 ++++++++++++++-
>  fs/ceph/caps.c  | 24 ++++++++++++++++++
>  fs/ceph/file.c  | 65 ++++++++++++++++++++++++++++++++++++++++++++++---
>  fs/ceph/inode.c | 48 +++++++++++++++++++-----------------
>  fs/ceph/super.h | 13 +++++++---
>  5 files changed, 138 insertions(+), 31 deletions(-)
> 

-- 
Jeff Layton <jlayton@kernel.org>