Re: [PATCH v1 1/1] ceph: Fix incorrect statfs report for small quota

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 09, 2021 at 02:40:41PM +0530, khiremat@xxxxxxxxxx wrote:
> From: Kotresh HR <khiremat@xxxxxxxxxx>
> 
> Problem:
> The statfs reports incorrect free/available space
> for quota less then CEPH_BLOCK size (4M).
> 
> Solution:
> For quotas less than CEPH_BLOCK size, it is
> decided to go with binary use/free of full block.
> For quota size less than CEPH_BLOCK size, report
> the total=used=CEPH_BLOCK,free=0 when quota is
> full and total=free=CEPH_BLOCK, used=0 otherwise.

This sounds good to me, although it's still not really accurate, as it
will always use the block size granularity.  However, using these small
values for quotas isn't probably quite common anyway, so meh.

> Signed-off-by: Kotresh HR <khiremat@xxxxxxxxxx>
> ---
>  fs/ceph/quota.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c
> index 620c691af40e..d49ba82d08bf 100644
> --- a/fs/ceph/quota.c
> +++ b/fs/ceph/quota.c
> @@ -505,6 +505,22 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fsc, struct kstatfs *buf)
>  			buf->f_bfree = free;
>  			buf->f_bavail = free;
>  			is_updated = true;
> +		} else if (ci->i_max_bytes) {
> +			/* For quota size less than CEPH_BLOCK size, report
> +			 * the total=used=CEPH_BLOCK,free=0 when quota is full and
> +			 * total=free=CEPH_BLOCK, used=0 otherwise  */
> +			total = ci->i_max_bytes;
> +			used = ci->i_rbytes;
> +
> +			buf->f_blocks = 1;
> +			if (total > used) {
> +				buf->f_bfree = 1;
> +				buf->f_bavail = 1;
> +			} else {
> +				buf->f_bfree = 0;
> +				buf->f_bavail = 0;
> +			}
> +			is_updated = true;
>  		}
>  		iput(in);
>  	}
> -- 
> 2.31.1
> 

I think it would be more correct to move this logic into the spinlock
protected code.  Which would even make the code simpler, something like:

	spin_lock(&ci->i_ceph_lock);
	if (ci->i_max_bytes) {
		total = ci->i_max_bytes >> CEPH_BLOCK_SHIFT;
		if (total) {
			/* ... */
		} else {
			total = 1;
			free = ci->i_max_bytes > ci->i_rbytes ? 1 : 0;
		}
	}
	spin_unlock(&ci->i_ceph_lock);

I guess the 'if (total)' condition (and the 'is_updated' variable) can
also be removed.

Cheers,
--
Luís



[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux