Re: [PATCH 5/5] xfs: cleanup xfs_log_unmount_write

Darrick J. Wong
2020-03-12T23:52:17Z

On Thu, Mar 12, 2020 at 03:39:59PM +0100, Christoph Hellwig wrote:
> Move the code for verifying the iclog state on a clean unmount into a
> helper, and instead of checking the iclog state just rely on the shutdown
> check as they are equivalent. Also remove the ifdef DEBUG as the
> compiler is smart enough to eliminate the dead code for non-debug builds.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Brian Foster <bfoster@redhat.com>

Looks fine , will test
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> fs/xfs/xfs_log.c | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index b56432d4a9b8..0986983ef6b5 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -946,6 +946,18 @@ xfs_log_write_unmount_record(
> }
> }
>
> +static void
> +xfs_log_unmount_verify_iclog(
> + struct xlog *log)
> +{
> + struct xlog_in_core *iclog = log->l_iclog;
> +
> + do {
> + ASSERT(iclog->ic_state == XLOG_STATE_ACTIVE);
> + ASSERT(iclog->ic_offset == 0);
> + } while ((iclog = iclog->ic_next) != log->l_iclog);
> +}
> +
> /*
> * Unmount record used to have a string "Unmount filesystem--" in the
> * data section where the "Un" was really a magic number (XLOG_UNMOUNT_TYPE).
> @@ -954,13 +966,10 @@ xfs_log_write_unmount_record(
> * As far as I know, there weren't any dependencies on the old behaviour.
> */
> static void
> -xfs_log_unmount_write(xfs_mount_t *mp)
> +xfs_log_unmount_write(
> + struct xfs_mount *mp)
> {
> - struct xlog *log = mp->m_log;
> - xlog_in_core_t *iclog;
> -#ifdef DEBUG
> - xlog_in_core_t *first_iclog;
> -#endif
> + struct xlog *log = mp->m_log;
>
> /*
> * Don't write out unmount record on norecovery mounts or ro devices.
> @@ -974,18 +983,9 @@ xfs_log_unmount_write(xfs_mount_t *mp)
>
> xfs_log_force(mp, XFS_LOG_SYNC);
>
> -#ifdef DEBUG
> - first_iclog = iclog = log->l_iclog;
> - do {
> - if (iclog->ic_state != XLOG_STATE_IOERROR) {
> - ASSERT(iclog->ic_state == XLOG_STATE_ACTIVE);
> - ASSERT(iclog->ic_offset == 0);
> - }
> - iclog = iclog->ic_next;
> - } while (iclog != first_iclog);
> -#endif
> if (XLOG_FORCED_SHUTDOWN(log))
> return;
> + xfs_log_unmount_verify_iclog(log);
> xfs_log_write_unmount_record(mp);
> }
>
> --
> 2.24.1
>