| Message ID | 20240118080404.783677-1-xiubli@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: always set the initial i_blkbits to CEPH_FSCRYPT_BLOCK_SHIFT | expand |
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
On 1/25/24 09:53, Eric Biggers wrote: > 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 Sure, I will fix it. >> 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! Thanks Eric, no worry, this will only be seen when the 'test_dummy_encryption' is enabled. > In the future please also Cc the fscrypt mailing list on things like this. Sure, I meant to Cc you but I think I forgot that. Sorry. > 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? Will fix it. Thanks Eric. - Xiubo > - Eric >
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)