Re: [PATCH] xfs: use more suitable method to get the quota limit value

[Dave Chinner <david@fromorbit.com>]

On Tue, Mar 17, 2020 at 05:59:30PM +0800, xiakaixu1987@gmail.com wrote:
> From: Kaixu Xia <kaixuxia@tencent.com>
> 
> It is more suitable to use min_not_zero() to get the quota limit
> value, means to choose the minimum value not the softlimit firstly.
> And the meaning is more clear even though the hardlimit value must
> be larger than softlimit value.
> 
> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
> ---
>  fs/xfs/xfs_qm_bhv.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/xfs/xfs_qm_bhv.c b/fs/xfs/xfs_qm_bhv.c
> index fc2fa41..f1711f5 100644
> --- a/fs/xfs/xfs_qm_bhv.c
> +++ b/fs/xfs/xfs_qm_bhv.c
> @@ -23,9 +23,8 @@
>  {
>  	uint64_t		limit;
>  
> -	limit = dqp->q_core.d_blk_softlimit ?
> -		be64_to_cpu(dqp->q_core.d_blk_softlimit) :
> -		be64_to_cpu(dqp->q_core.d_blk_hardlimit);
> +	limit = min_not_zero(be64_to_cpu(dqp->q_core.d_blk_softlimit),
> +				be64_to_cpu(dqp->q_core.d_blk_hardlimit));
>  	if (limit && statp->f_blocks > limit) {
>  		statp->f_blocks = limit;
>  		statp->f_bfree = statp->f_bavail =
> @@ -33,9 +32,8 @@
>  			 (statp->f_blocks - dqp->q_res_bcount) : 0;
>  	}
>  
> -	limit = dqp->q_core.d_ino_softlimit ?
> -		be64_to_cpu(dqp->q_core.d_ino_softlimit) :
> -		be64_to_cpu(dqp->q_core.d_ino_hardlimit);
> +	limit = min_not_zero(be64_to_cpu(dqp->q_core.d_ino_softlimit),
> +				be64_to_cpu(dqp->q_core.d_ino_hardlimit));

Which variable is "not zero"? The first, the second, or both?

Oh, it's both, so this is actually a change of logic:

	if (softlimit == 0)
		limit = hardlimit;
	else if (hardlimit == 0)
		limit = softlimit;
	else
		limit = min(softlimit, hardlimit);

So now if both soft and hard limit are set, the hard limit overrides
the soft limit, even when only the soft limit should apply (e.g.
during the grace period).

Hence this ends up being a user visible change in behaviour. And,
IMO, this doesn't make the code better or clearer as now I have go
find the definition of min_not_zero() to understand what the soft
limit vs hard limit quota policy logic is. That's not an improvement.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com