Re: [PATCH 6/9] xfs: collapse xlog_state_set_callback in caller

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Aug 10, 2022 at 09:03:50AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> The function is called from a single place, and it isn't just
> setting the iclog state to XLOG_STATE_CALLBACK - it can mark iclogs
> clean, which moves tehm to states after CALLBACK. Hence the function

Nit: s/tehm/them/

> is now badly named, and should just be folded into the caller where
> the iclog completion logic makes a whole lot more sense.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

I had wondered what xlog_state_set_callback thought it was doing until I
looked ahead and saw this.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_log.c | 31 +++++++++++--------------------
>  1 file changed, 11 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index e420591b1a8a..5b7c91a42edf 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -2520,25 +2520,6 @@ xlog_get_lowest_lsn(
>  	return lowest_lsn;
>  }
>  
> -static void
> -xlog_state_set_callback(
> -	struct xlog		*log,
> -	struct xlog_in_core	*iclog,
> -	xfs_lsn_t		header_lsn)
> -{
> -	/*
> -	 * If there are no callbacks on this iclog, we can mark it clean
> -	 * immediately and return. Otherwise we need to run the
> -	 * callbacks.
> -	 */
> -	if (list_empty(&iclog->ic_callbacks)) {
> -		xlog_state_clean_iclog(log, iclog);
> -		return;
> -	}
> -	trace_xlog_iclog_callback(iclog, _RET_IP_);
> -	iclog->ic_state = XLOG_STATE_CALLBACK;
> -}
> -
>  /*
>   * Return true if we need to stop processing, false to continue to the next
>   * iclog. The caller will need to run callbacks if the iclog is returned in the
> @@ -2570,7 +2551,17 @@ xlog_state_iodone_process_iclog(
>  		lowest_lsn = xlog_get_lowest_lsn(log);
>  		if (lowest_lsn && XFS_LSN_CMP(lowest_lsn, header_lsn) < 0)
>  			return false;
> -		xlog_state_set_callback(log, iclog, header_lsn);
> +		/*
> +		 * If there are no callbacks on this iclog, we can mark it clean
> +		 * immediately and return. Otherwise we need to run the
> +		 * callbacks.
> +		 */
> +		if (list_empty(&iclog->ic_callbacks)) {
> +			xlog_state_clean_iclog(log, iclog);
> +			return false;
> +		}
> +		trace_xlog_iclog_callback(iclog, _RET_IP_);
> +		iclog->ic_state = XLOG_STATE_CALLBACK;
>  		return false;
>  	default:
>  		/*
> -- 
> 2.36.1
>