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

Brian Foster
2020-03-05T18:07:24Z

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
>