Re: [PATCH 06/11] xfs: move xlog_state_ioerror()

[Brian Foster <bfoster@redhat.com>]

On Wed, Mar 04, 2020 at 06:53:56PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> To clean up unmount record writing error handling, we need to move
> xlog_state_ioerror() higher up in the file. Also move the setting of
> the XLOG_IO_ERROR state to inside the function.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/xfs_log.c | 59 ++++++++++++++++++++++++------------------------
>  1 file changed, 30 insertions(+), 29 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 2e9f3baa7cc8..0de3c32d42b6 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -874,6 +874,36 @@ xfs_log_mount_cancel(
>  	xfs_log_unmount(mp);
>  }
>  
> +/*
> + * Mark all iclogs IOERROR. l_icloglock is held by the caller.
> + */
> +STATIC int
> +xlog_state_ioerror(
> +	struct xlog	*log)
> +{
> +	xlog_in_core_t	*iclog, *ic;

Might as well kill off the typedef usage, though it sounds like
Christoph has code to remove this. Eh, that discussion aside this looks
fine to me:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> +
> +	log->l_flags |= XLOG_IO_ERROR;
> +
> +	iclog = log->l_iclog;
> +	if (iclog->ic_state != XLOG_STATE_IOERROR) {
> +		/*
> +		 * Mark all the incore logs IOERROR.
> +		 * From now on, no log flushes will result.
> +		 */
> +		ic = iclog;
> +		do {
> +			ic->ic_state = XLOG_STATE_IOERROR;
> +			ic = ic->ic_next;
> +		} while (ic != iclog);
> +		return 0;
> +	}
> +	/*
> +	 * Return non-zero, if state transition has already happened.
> +	 */
> +	return 1;
> +}
> +
>  /*
>   * Mark the filesystem clean by writing an unmount record to the head of the
>   * log.
> @@ -3770,34 +3800,6 @@ xlog_verify_iclog(
>  }	/* xlog_verify_iclog */
>  #endif
>  
> -/*
> - * Mark all iclogs IOERROR. l_icloglock is held by the caller.
> - */
> -STATIC int
> -xlog_state_ioerror(
> -	struct xlog	*log)
> -{
> -	xlog_in_core_t	*iclog, *ic;
> -
> -	iclog = log->l_iclog;
> -	if (iclog->ic_state != XLOG_STATE_IOERROR) {
> -		/*
> -		 * Mark all the incore logs IOERROR.
> -		 * From now on, no log flushes will result.
> -		 */
> -		ic = iclog;
> -		do {
> -			ic->ic_state = XLOG_STATE_IOERROR;
> -			ic = ic->ic_next;
> -		} while (ic != iclog);
> -		return 0;
> -	}
> -	/*
> -	 * Return non-zero, if state transition has already happened.
> -	 */
> -	return 1;
> -}
> -
>  /*
>   * This is called from xfs_force_shutdown, when we're forcibly
>   * shutting down the filesystem, typically because of an IO error.
> @@ -3868,7 +3870,6 @@ xfs_log_force_umount(
>  	 * Mark the log and the iclogs with IO error flags to prevent any
>  	 * further log IO from being issued or completed.
>  	 */
> -	log->l_flags |= XLOG_IO_ERROR;
>  	retval = xlog_state_ioerror(log);
>  	spin_unlock(&log->l_icloglock);
>  
> -- 
> 2.24.0.rc0
>