On Mon, Mar 16, 2020 at 03:42:32PM +0100, Christoph Hellwig wrote:
> Open code the xlog_state_want_sync logic in its two callers given that
> this function is a trivial wrapper around xlog_state_switch_iclogs.
>
> Move the lockdep assert into xlog_state_switch_iclogs to not lose this
> debugging aid, and improve the comment that documents
> xlog_state_switch_iclogs as well.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_log.c | 50 +++++++++++++++++-------------------------------
> 1 file changed, 18 insertions(+), 32 deletions(-)
>
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 865dd1e08679..761b138d97ec 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -62,11 +62,6 @@ xlog_state_switch_iclogs(
> struct xlog_in_core *iclog,
> int eventual_size);
> STATIC void
> -xlog_state_want_sync(
> - struct xlog *log,
> - struct xlog_in_core *iclog);
> -
> -STATIC void
> xlog_grant_push_ail(
> struct xlog *log,
> int need_bytes);
> @@ -938,7 +933,11 @@ xfs_log_write_unmount_record(
> spin_lock(&log->l_icloglock);
> iclog = log->l_iclog;
> atomic_inc(&iclog->ic_refcnt);
> - xlog_state_want_sync(log, iclog);
> + if (iclog->ic_state == XLOG_STATE_ACTIVE)
> + xlog_state_switch_iclogs(log, iclog, 0);
> + else
> + ASSERT(iclog->ic_state == XLOG_STATE_WANT_SYNC ||
> + iclog->ic_state == XLOG_STATE_IOERROR);
> error = xlog_state_release_iclog(log, iclog);
> xlog_wait_on_iclog(iclog);
>
> @@ -2293,7 +2292,11 @@ xlog_write_copy_finish(
> *record_cnt = 0;
> *data_cnt = 0;
>
> - xlog_state_want_sync(log, iclog);
> + if (iclog->ic_state == XLOG_STATE_ACTIVE)
> + xlog_state_switch_iclogs(log, iclog, 0);
> + else
> + ASSERT(iclog->ic_state == XLOG_STATE_WANT_SYNC ||
> + iclog->ic_state == XLOG_STATE_IOERROR);
> if (!commit_iclog)
> goto release_iclog;
> spin_unlock(&log->l_icloglock);
> @@ -3069,11 +3072,12 @@ xlog_ungrant_log_space(
> }
>
> /*
> - * This routine will mark the current iclog in the ring as WANT_SYNC
> - * and move the current iclog pointer to the next iclog in the ring.
> - * When this routine is called from xlog_state_get_iclog_space(), the
> - * exact size of the iclog has not yet been determined. All we know is
> - * that every data block. We have run out of space in this log record.
> + * Mark the current iclog in the ring as WANT_SYNC and move the current iclog
> + * pointer to the next iclog in the ring.
> + *
> + * When called from xlog_state_get_iclog_space(), the exact size of the iclog
> + * has not yet been determined, all we know is that we have run out of space in
> + * the current iclog.
> */
> STATIC void
> xlog_state_switch_iclogs(
> @@ -3082,6 +3086,8 @@ xlog_state_switch_iclogs(
> int eventual_size)
> {
> ASSERT(iclog->ic_state == XLOG_STATE_ACTIVE);
> + assert_spin_locked(&log->l_icloglock);
> +
> if (!eventual_size)
> eventual_size = iclog->ic_offset;
> iclog->ic_state = XLOG_STATE_WANT_SYNC;
> @@ -3323,26 +3329,6 @@ xfs_log_force_lsn(
> return ret;
> }
>
> -/*
> - * Called when we want to mark the current iclog as being ready to sync to
> - * disk.
> - */
> -STATIC void
> -xlog_state_want_sync(
> - struct xlog *log,
> - struct xlog_in_core *iclog)
> -{
> - assert_spin_locked(&log->l_icloglock);
> -
> - if (iclog->ic_state == XLOG_STATE_ACTIVE) {
> - xlog_state_switch_iclogs(log, iclog, 0);
> - } else {
> - ASSERT(iclog->ic_state == XLOG_STATE_WANT_SYNC ||
> - iclog->ic_state == XLOG_STATE_IOERROR);
> - }
> -}
> -
> -
> /*****************************************************************************
> *
> * TICKET functions
> --
> 2.24.1
>
