Re: [PATCH] ceph: always set the initial i_blkbits to CEPH_FSCRYPT_BLOCK_SHIFT

[Eric Biggers <ebiggers@kernel.org>]

On Thu, Jan 18, 2024 at 04:04:04PM +0800, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
> 
> The fscrypt code will use i_blkbits to setup the 'ci_data_unit_bits'
> when allocating the new inode, but ceph will initiate i_blkbits
> ater when filling the inode, which is too late. Since the
> 'ci_data_unit_bits' will only be used by the fscrypt framework so
> initiating i_blkbits with CEPH_FSCRYPT_BLOCK_SHIFT is safe.
> 
> Fixes: commit 5b1188847180 ("fscrypt: support crypto data unit size
>        less than filesystem block size")

The Fixes line should be one line, and the word "commit" should not be there

> URL: https://tracker.ceph.com/issues/64035
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>  fs/ceph/inode.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 62af452cdba4..d96d97b31f68 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -79,6 +79,8 @@ struct inode *ceph_new_inode(struct inode *dir, struct dentry *dentry,
>  	if (!inode)
>  		return ERR_PTR(-ENOMEM);
>  
> +	inode->i_blkbits = CEPH_FSCRYPT_BLOCK_SHIFT;
> +
>  	if (!S_ISLNK(*mode)) {
>  		err = ceph_pre_init_acls(dir, mode, as_ctx);
>  		if (err < 0)

Looks good, you can add:

    Reviewed-by: Eric Biggers <ebiggers@google.com>

Sorry for the trouble; I need to start running xfstests on CephFS!

In the future please also Cc the fscrypt mailing list on things like this.

Maybe you'd like to also send a patch that updates the comment for
fscrypt_prepare_new_inode() to clarify that i_blkbits needs to be set first?

- Eric