Re: [PATCH 04/11] xfs: merge xlog_commit_record with xlog_write_done()

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

On Wed, Mar 04, 2020 at 06:53:54PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> xlog_write_done() is just a thin wrapper around
> xlog_commit_record(), so they can be merged together easily. Convert
> all the xlog_commit_record() callers to use xlog_write_done() and
> merge the implementations.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

I think Christoph disagrees but I actually prefer to see the incremental
steps:

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

> fs/xfs/xfs_log.c | 60 +++++++++++++++---------------------------------
> 1 file changed, 19 insertions(+), 41 deletions(-)
>
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 702b38e4db6e..100eeaed4a7d 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -24,13 +24,6 @@
> kmem_zone_t *xfs_log_ticket_zone;
>
> /* Local miscellaneous function prototypes */
> -STATIC int
> -xlog_commit_record(
> - struct xlog *log,
> - struct xlog_ticket *ticket,
> - struct xlog_in_core **iclog,
> - xfs_lsn_t *commitlsnp);
> -
> STATIC struct xlog *
> xlog_alloc_log(
> struct xfs_mount *mp,
> @@ -493,7 +486,8 @@ xfs_log_reserve(
> */
>
> /*
> - * Write a commit record to the log to close off a running log write.
> + * Write out the commit record of a transaction associated with the given
> + * ticket to close off a running log write. Return the lsn of the commit record.
> */
> int
> xlog_write_done(
> @@ -502,10 +496,26 @@ xlog_write_done(
> struct xlog_in_core **iclog,
> xfs_lsn_t *lsn)
> {
> + struct xfs_log_iovec reg = {
> + .i_addr = NULL,
> + .i_len = 0,
> + .i_type = XLOG_REG_TYPE_COMMIT,
> + };
> + struct xfs_log_vec vec = {
> + .lv_niovecs = 1,
> + .lv_iovecp = &reg,
> + };
> + int error;
> +
> + ASSERT_ALWAYS(iclog);
> +
> if (XLOG_FORCED_SHUTDOWN(log))
> return -EIO;
>
> - return xlog_commit_record(log, ticket, iclog, lsn);
> + error = xlog_write(log, &vec, ticket, lsn, iclog, XLOG_COMMIT_TRANS);
> + if (error)
> + xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
> + return error;
> }
>
> /*
> @@ -1529,38 +1539,6 @@ xlog_alloc_log(
> return ERR_PTR(error);
> } /* xlog_alloc_log */
>
> -
> -/*
> - * Write out the commit record of a transaction associated with the given
> - * ticket. Return the lsn of the commit record.
> - */
> -STATIC int
> -xlog_commit_record(
> - struct xlog *log,
> - struct xlog_ticket *ticket,
> - struct xlog_in_core **iclog,
> - xfs_lsn_t *commitlsnp)
> -{
> - struct xfs_mount *mp = log->l_mp;
> - int error;
> - struct xfs_log_iovec reg = {
> - .i_addr = NULL,
> - .i_len = 0,
> - .i_type = XLOG_REG_TYPE_COMMIT,
> - };
> - struct xfs_log_vec vec = {
> - .lv_niovecs = 1,
> - .lv_iovecp = &reg,
> - };
> -
> - ASSERT_ALWAYS(iclog);
> - error = xlog_write(log, &vec, ticket, commitlsnp, iclog,
> - XLOG_COMMIT_TRANS);
> - if (error)
> - xfs_force_shutdown(mp, SHUTDOWN_LOG_IO_ERROR);
> - return error;
> -}
> -
> /*
> * Push on the buffer cache code if we ever use more than 75% of the on-disk
> * log space. This code pushes on the lsn which would supposedly free up
> --
> 2.24.0.rc0
>