On Thu, Jan 28, 2021 at 03:41:50PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> We don't need to look at the xfs_mount and superblock every time we
> need to do an iclog roundoff calculation. The property is fixed for
> the life of the log, so store the roundoff in the log at mount time
> and use that everywhere.
>
> On a debug build:
>
> $ size fs/xfs/xfs_log.o.*
> text data bss dec hex filename
> 27360 560 8 27928 6d18 fs/xfs/xfs_log.o.orig
> 27219 560 8 27787 6c8b fs/xfs/xfs_log.o.patched
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_log_format.h | 3 --
> fs/xfs/xfs_log.c | 60 +++++++++++++++-------------------
> fs/xfs/xfs_log_priv.h | 2 ++
> 3 files changed, 28 insertions(+), 37 deletions(-)
>
...
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 58699881c100..c5f507c24577 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1400,6 +1400,12 @@ xlog_alloc_log(
> xlog_assign_atomic_lsn(&log->l_last_sync_lsn, 1, 0);
> log->l_curr_cycle = 1; /* 0 is bad since this is initial value */
>
> + /* roundoff padding for transaction data and one for commit record */
I don't follow this comment. What do you mean by "... and one for commit
record?"
> + if (xfs_sb_version_haslogv2(&mp->m_sb) && mp->m_sb.sb_logsunit > 1)
> + log->l_iclog_roundoff = mp->m_sb.sb_logsunit;
> + else
> + log->l_iclog_roundoff = BBSIZE;
> +
> xlog_grant_head_init(&log->l_reserve_head);
> xlog_grant_head_init(&log->l_write_head);
>
...
> @@ -3485,18 +3475,20 @@ xfs_log_calc_unit_res(
> /* for commit-rec LR header - note: padding will subsume the ophdr */
> unit_bytes += log->l_iclog_hsize;
>
> - /* for roundoff padding for transaction data and one for commit record */
> - if (xfs_sb_version_haslogv2(&mp->m_sb) && mp->m_sb.sb_logsunit > 1) {
> - /* log su roundoff */
> - unit_bytes += 2 * mp->m_sb.sb_logsunit;
> - } else {
> - /* BB roundoff */
> - unit_bytes += 2 * BBSIZE;
> - }
> + /* roundoff padding for transaction data and one for commit record */
> + unit_bytes += log->l_iclog_roundoff;
>
> return unit_bytes;
> }
>
> +int
> +xfs_log_calc_unit_res(
> + struct xfs_mount *mp,
> + int unit_bytes)
> +{
> + return xlog_calc_unit_res(mp->m_log, unit_bytes);
> +}
> +
Could this be moved to the header as an inline? Otherwise looks
reasonable.
Brian
> /*
> * Allocate and initialise a new log ticket.
> */
> @@ -3513,7 +3505,7 @@ xlog_ticket_alloc(
>
> tic = kmem_cache_zalloc(xfs_log_ticket_zone, GFP_NOFS | __GFP_NOFAIL);
>
> - unit_res = xfs_log_calc_unit_res(log->l_mp, unit_bytes);
> + unit_res = xlog_calc_unit_res(log, unit_bytes);
>
> atomic_set(&tic->t_ref, 1);
> tic->t_task = current;
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index 1c6fdbf3d506..037950cf1061 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -436,6 +436,8 @@ struct xlog {
> #endif
> /* log recovery lsn tracking (for buffer submission */
> xfs_lsn_t l_recovery_lsn;
> +
> + uint32_t l_iclog_roundoff;/* padding roundoff */
> };
>
> #define XLOG_BUF_CANCEL_BUCKET(log, blkno) \
> --
> 2.28.0
>
