| Message ID | 20220624093730.8564-3-xiubli@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: switch to 4KB block size if quota size is not aligned to 4MB | expand |
xiubli@redhat.com writes: > From: Xiubo Li <xiubli@redhat.com> > > If the quota size is larger than but not aligned to 4MB, the statfs > will always set the block size to 4MB and round down the fragment > size. For exmaple if the quota size is 6MB, the `df` will always > show 4MB capacity. > > Make the block size to 4KB as default if quota size is set unless > the quota size is larger than or equals to 4MB and at the same time > it aligns to 4MB. > > Signed-off-by: Xiubo Li <xiubli@redhat.com> > --- > fs/ceph/quota.c | 31 ++++++++++++++++++++----------- > 1 file changed, 20 insertions(+), 11 deletions(-) > > diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c > index 64592adfe48f..c50527151913 100644 > --- a/fs/ceph/quota.c > +++ b/fs/ceph/quota.c > @@ -483,6 +483,7 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fsc, struct kstatfs *buf) > struct inode *in; > u64 total = 0, used, free; > bool is_updated = false; > + u32 block_shift = CEPH_4K_BLOCK_SHIFT; > > down_read(&mdsc->snap_rwsem); > realm = get_quota_realm(mdsc, d_inode(fsc->sb->s_root), > @@ -498,21 +499,29 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fsc, struct kstatfs *buf) > ci = ceph_inode(in); > spin_lock(&ci->i_ceph_lock); > if (ci->i_max_bytes) { > - total = ci->i_max_bytes >> CEPH_BLOCK_SHIFT; > - used = ci->i_rbytes >> CEPH_BLOCK_SHIFT; > - /* For quota size less than 4MB, use 4KB block size */ > - if (!total) { > - total = ci->i_max_bytes >> CEPH_4K_BLOCK_SHIFT; > - used = ci->i_rbytes >> CEPH_4K_BLOCK_SHIFT; > - buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; > - } > - /* It is possible for a quota to be exceeded. > + /* > + * Switch to 4MB block size if quota size is > + * larger than or equals to 4MB and at the > + * same time is aligned to 4MB. > + */ > + if (ci->i_max_bytes >= (1 << CEPH_BLOCK_SHIFT) && > + !(ci->i_max_bytes % (1 << CEPH_BLOCK_SHIFT))) Maybe worth replacing this 2nd condition with the IS_ALIGNED() macro. Other than this, these patches look good. I do have question though: is it possible that this will behaviour may break some user-space programs that expect more deterministic values for these fields (buf->f_frsize and buf->f_bsize)? Because the same filesystem will report different values depending on which dir you mount. Obviously, this isn't a problem with this particular patch, as this behaviour is already present. Cheers,
On 6/24/22 10:47 PM, Luís Henriques wrote: > xiubli@redhat.com writes: > >> From: Xiubo Li <xiubli@redhat.com> >> >> If the quota size is larger than but not aligned to 4MB, the statfs >> will always set the block size to 4MB and round down the fragment >> size. For exmaple if the quota size is 6MB, the `df` will always >> show 4MB capacity. >> >> Make the block size to 4KB as default if quota size is set unless >> the quota size is larger than or equals to 4MB and at the same time >> it aligns to 4MB. >> >> Signed-off-by: Xiubo Li <xiubli@redhat.com> >> --- >> fs/ceph/quota.c | 31 ++++++++++++++++++++----------- >> 1 file changed, 20 insertions(+), 11 deletions(-) >> >> diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c >> index 64592adfe48f..c50527151913 100644 >> --- a/fs/ceph/quota.c >> +++ b/fs/ceph/quota.c >> @@ -483,6 +483,7 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fsc, struct kstatfs *buf) >> struct inode *in; >> u64 total = 0, used, free; >> bool is_updated = false; >> + u32 block_shift = CEPH_4K_BLOCK_SHIFT; >> >> down_read(&mdsc->snap_rwsem); >> realm = get_quota_realm(mdsc, d_inode(fsc->sb->s_root), >> @@ -498,21 +499,29 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fsc, struct kstatfs *buf) >> ci = ceph_inode(in); >> spin_lock(&ci->i_ceph_lock); >> if (ci->i_max_bytes) { >> - total = ci->i_max_bytes >> CEPH_BLOCK_SHIFT; >> - used = ci->i_rbytes >> CEPH_BLOCK_SHIFT; >> - /* For quota size less than 4MB, use 4KB block size */ >> - if (!total) { >> - total = ci->i_max_bytes >> CEPH_4K_BLOCK_SHIFT; >> - used = ci->i_rbytes >> CEPH_4K_BLOCK_SHIFT; >> - buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; >> - } >> - /* It is possible for a quota to be exceeded. >> + /* >> + * Switch to 4MB block size if quota size is >> + * larger than or equals to 4MB and at the >> + * same time is aligned to 4MB. >> + */ >> + if (ci->i_max_bytes >= (1 << CEPH_BLOCK_SHIFT) && >> + !(ci->i_max_bytes % (1 << CEPH_BLOCK_SHIFT))) > Maybe worth replacing this 2nd condition with the IS_ALIGNED() macro. > Other than this, these patches look good. Sure, will fix it. > I do have question though: is it possible that this will behaviour may > break some user-space programs that expect more deterministic values for > these fields (buf->f_frsize and buf->f_bsize)? Because the same > filesystem will report different values depending on which dir you mount. Yeah, I was also thinking about this, but haven't found which use case could be broke yet. > Obviously, this isn't a problem with this particular patch, as this > behaviour is already present. Till now this works well and no test case complains it. -- Xiubo > Cheers,
diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c index 64592adfe48f..c50527151913 100644 --- a/fs/ceph/quota.c +++ b/fs/ceph/quota.c @@ -483,6 +483,7 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fsc, struct kstatfs *buf) struct inode *in; u64 total = 0, used, free; bool is_updated = false; + u32 block_shift = CEPH_4K_BLOCK_SHIFT; down_read(&mdsc->snap_rwsem); realm = get_quota_realm(mdsc, d_inode(fsc->sb->s_root), @@ -498,21 +499,29 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fsc, struct kstatfs *buf) ci = ceph_inode(in); spin_lock(&ci->i_ceph_lock); if (ci->i_max_bytes) { - total = ci->i_max_bytes >> CEPH_BLOCK_SHIFT; - used = ci->i_rbytes >> CEPH_BLOCK_SHIFT; - /* For quota size less than 4MB, use 4KB block size */ - if (!total) { - total = ci->i_max_bytes >> CEPH_4K_BLOCK_SHIFT; - used = ci->i_rbytes >> CEPH_4K_BLOCK_SHIFT; - buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; - } - /* It is possible for a quota to be exceeded. + /* + * Switch to 4MB block size if quota size is + * larger than or equals to 4MB and at the + * same time is aligned to 4MB. + */ + if (ci->i_max_bytes >= (1 << CEPH_BLOCK_SHIFT) && + !(ci->i_max_bytes % (1 << CEPH_BLOCK_SHIFT))) + block_shift = CEPH_BLOCK_SHIFT; + + total = ci->i_max_bytes >> block_shift; + used = ci->i_rbytes >> block_shift; + buf->f_frsize = 1 << block_shift; + + /* + * It is possible for a quota to be exceeded. * Report 'zero' in that case */ free = total > used ? total - used : 0; - /* For quota size less than 4KB, report the + /* + * For quota size less than 4KB, report the * total=used=4KB,free=0 when quota is full - * and total=free=4KB, used=0 otherwise */ + * and total=free=4KB, used=0 otherwise + */ if (!total) { total = 1; free = ci->i_max_bytes > ci->i_rbytes ? 1 : 0;