[PATCH 0/2] ceph: switch to 4KB block size if quota size is not aligned to 4MB

[PATCH 0/2] ceph: switch to 4KB block size if quota size is not aligned to 4MB

[xiubli]

From: Xiubo Li <xiubli@redhat.com>


Xiubo Li (2):
  ceph: make f_bsize always equal to f_frsize
  ceph: switch to 4KB block size if quota size is not aligned to 4MB

 fs/ceph/quota.c | 31 ++++++++++++++++++++-----------
 fs/ceph/super.c | 22 +++++++++++-----------
 2 files changed, 31 insertions(+), 22 deletions(-)

-- 
2.36.0.rc1

[PATCH 1/2] ceph: make f_bsize always equal to f_frsize

[xiubli]

From: Xiubo Li <xiubli@redhat.com>

The f_frsize maybe changed in the quota size is less than the defualt
4MB.

Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/super.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 74f9cc5f37e9..88d7e67130b8 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -70,17 +70,6 @@ static int ceph_statfs(struct dentry *dentry, struct kstatfs *buf)
 
 	/* fill in kstatfs */
 	buf->f_type = CEPH_SUPER_MAGIC;  /* ?? */
-
-	/*
-	 * express utilization in terms of large blocks to avoid
-	 * overflow on 32-bit machines.
-	 *
-	 * NOTE: for the time being, we make bsize == frsize to humor
-	 * not-yet-ancient versions of glibc that are broken.
-	 * Someday, we will probably want to report a real block
-	 * size...  whatever that may mean for a network file system!
-	 */
-	buf->f_bsize = 1 << CEPH_BLOCK_SHIFT;
 	buf->f_frsize = 1 << CEPH_BLOCK_SHIFT;
 
 	/*
@@ -95,6 +84,17 @@ static int ceph_statfs(struct dentry *dentry, struct kstatfs *buf)
 		buf->f_bavail = le64_to_cpu(st.kb_avail) >> (CEPH_BLOCK_SHIFT-10);
 	}
 
+	/*
+	 * express utilization in terms of large blocks to avoid
+	 * overflow on 32-bit machines.
+	 *
+	 * NOTE: for the time being, we make bsize == frsize to humor
+	 * not-yet-ancient versions of glibc that are broken.
+	 * Someday, we will probably want to report a real block
+	 * size...  whatever that may mean for a network file system!
+	 */
+	buf->f_bsize = buf->f_frsize;
+
 	buf->f_files = le64_to_cpu(st.num_objects);
 	buf->f_ffree = -1;
 	buf->f_namelen = NAME_MAX;
-- 
2.36.0.rc1

[PATCH 2/2] ceph: switch to 4KB block size if quota size is not aligned to 4MB

[xiubli]

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)))
+				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;
-- 
2.36.0.rc1

Re: [PATCH 2/2] ceph: switch to 4KB block size if quota size is not aligned to 4MB

[Luís Henriques]

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,
-- 
Luís

> +				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;
> -- 
>
> 2.36.0.rc1
>

Re: [PATCH 2/2] ceph: switch to 4KB block size if quota size is not aligned to 4MB

[Xiubo Li]

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,