Re: [PATCH 1/3] xfs: directly return if the delta equal to zero

[Brian Foster <bfoster@redhat.com>]

On Tue, Sep 22, 2020 at 05:04:00PM +0800, xiakaixu1987@gmail.com wrote:
> From: Kaixu Xia <kaixuxia@tencent.com>
> 
> It is useless to go on when the variable delta equal to zero in
> xfs_trans_mod_dquot(), so just return if the value equal to zero.
> 
> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
> ---
>  fs/xfs/xfs_trans_dquot.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index 133fc6fc3edd..23c34af71825 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -215,10 +215,11 @@ xfs_trans_mod_dquot(
>  	if (qtrx->qt_dquot == NULL)
>  		qtrx->qt_dquot = dqp;
>  
> -	if (delta) {
> -		trace_xfs_trans_mod_dquot_before(qtrx);
> -		trace_xfs_trans_mod_dquot(tp, dqp, field, delta);
> -	}
> +	if (!delta)
> +		return;
> +


This does slightly change behavior in that this function currently
unconditionally results in logging the associated dquot in the
transaction. I'm not sure anything really depends on that with a delta
== 0, but it might be worth documenting in the commit log.

Also, it does seem a little odd to bail out after we've potentially
allocated ->t_dqinfo as well as assigned the current dquot a slot in the
transaction. I think that means the effect of this change is lost if
another dquot happens to be modified (with delta != 0) in the same
transaction (which might also be an odd thing to do).

Brian

> +	trace_xfs_trans_mod_dquot_before(qtrx);
> +	trace_xfs_trans_mod_dquot(tp, dqp, field, delta);
>  
>  	switch (field) {
>  
> @@ -284,8 +285,7 @@ xfs_trans_mod_dquot(
>  		ASSERT(0);
>  	}
>  
> -	if (delta)
> -		trace_xfs_trans_mod_dquot_after(qtrx);
> +	trace_xfs_trans_mod_dquot_after(qtrx);
>  
>  	tp->t_flags |= XFS_TRANS_DQ_DIRTY;
>  }
> -- 
> 2.20.0
>