On Tue, Mar 19, 2024 at 01:15:24PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Rather than passing a mix of xfs_mount and xlog (and sometimes both)
> through the call chain of buffer log item recovery, just pass
> the struct xlog and pull the xfs_mount from that only at the leaf
> functions where it is needed. This makes it all just a little
> cleaner.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Looks good to me,
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_buf_item_recover.c | 94 +++++++++++++++++------------------
> 1 file changed, 47 insertions(+), 47 deletions(-)
>
> diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
> index 9225baa62755..edd03b08c969 100644
> --- a/fs/xfs/xfs_buf_item_recover.c
> +++ b/fs/xfs/xfs_buf_item_recover.c
> @@ -209,7 +209,7 @@ xlog_recover_buf_commit_pass1(
> */
> static int
> xlog_recover_validate_buf_type(
> - struct xfs_mount *mp,
> + struct xlog *log,
> struct xfs_buf *bp,
> struct xfs_buf_log_format *buf_f,
> xfs_lsn_t current_lsn)
> @@ -228,7 +228,7 @@ xlog_recover_validate_buf_type(
> * inconsistent state resulting in verification failures. Hence for now
> * just avoid the verification stage for non-crc filesystems
> */
> - if (!xfs_has_crc(mp))
> + if (!xfs_has_crc(log->l_mp))
> return 0;
>
> magic32 = be32_to_cpu(*(__be32 *)bp->b_addr);
> @@ -396,7 +396,7 @@ xlog_recover_validate_buf_type(
> break;
> #endif /* CONFIG_XFS_RT */
> default:
> - xfs_warn(mp, "Unknown buffer type %d!",
> + xfs_warn(log->l_mp, "Unknown buffer type %d!",
> xfs_blft_from_flags(buf_f));
> break;
> }
> @@ -410,7 +410,7 @@ xlog_recover_validate_buf_type(
> return 0;
>
> if (warnmsg) {
> - xfs_warn(mp, warnmsg);
> + xfs_warn(log->l_mp, warnmsg);
> xfs_buf_corruption_error(bp, __this_address);
> return -EFSCORRUPTED;
> }
> @@ -428,7 +428,7 @@ xlog_recover_validate_buf_type(
> */
> ASSERT(bp->b_ops);
> bp->b_flags |= _XBF_LOGRECOVERY;
> - xfs_buf_item_init(bp, mp);
> + xfs_buf_item_init(bp, log->l_mp);
> bp->b_log_item->bli_item.li_lsn = current_lsn;
> return 0;
> }
> @@ -441,7 +441,7 @@ xlog_recover_validate_buf_type(
> */
> static void
> xlog_recover_buffer(
> - struct xfs_mount *mp,
> + struct xlog *log,
> struct xlog_recover_item *item,
> struct xfs_buf *bp,
> struct xfs_buf_log_format *buf_f)
> @@ -450,7 +450,7 @@ xlog_recover_buffer(
> int bit;
> int nbits;
>
> - trace_xfs_log_recover_buf_reg_buf(mp->m_log, buf_f);
> + trace_xfs_log_recover_buf_reg_buf(log, buf_f);
>
> bit = 0;
> i = 1; /* 0 is the buf format structure */
> @@ -492,14 +492,14 @@ xlog_recover_buffer(
>
> static int
> xlog_recover_do_reg_buffer(
> - struct xfs_mount *mp,
> + struct xlog *log,
> struct xlog_recover_item *item,
> struct xfs_buf *bp,
> struct xfs_buf_log_format *buf_f,
> xfs_lsn_t current_lsn)
> {
> - xlog_recover_buffer(mp, item, bp, buf_f);
> - return xlog_recover_validate_buf_type(mp, bp, buf_f, current_lsn);
> + xlog_recover_buffer(log, item, bp, buf_f);
> + return xlog_recover_validate_buf_type(log, bp, buf_f, current_lsn);
> }
>
> /*
> @@ -513,7 +513,6 @@ xlog_recover_do_reg_buffer(
> */
> static bool
> xlog_recover_this_dquot_buffer(
> - struct xfs_mount *mp,
> struct xlog *log,
> struct xlog_recover_item *item,
> struct xfs_buf *bp,
> @@ -526,7 +525,7 @@ xlog_recover_this_dquot_buffer(
> /*
> * Filesystems are required to send in quota flags at mount time.
> */
> - if (!mp->m_qflags)
> + if (!log->l_mp->m_qflags)
> return false;
>
> type = 0;
> @@ -550,7 +549,7 @@ xlog_recover_this_dquot_buffer(
> */
> static int
> xlog_recover_verify_dquot_buf_item(
> - struct xfs_mount *mp,
> + struct xlog *log,
> struct xlog_recover_item *item,
> struct xfs_buf *bp,
> struct xfs_buf_log_format *buf_f)
> @@ -564,7 +563,7 @@ xlog_recover_verify_dquot_buf_item(
> case XFS_BLFT_GDQUOT_BUF:
> break;
> default:
> - xfs_alert(mp,
> + xfs_alert(log->l_mp,
> "XFS: dquot buffer log format type mismatch in %s.",
> __func__);
> xfs_buf_corruption_error(bp, __this_address);
> @@ -573,19 +572,19 @@ xlog_recover_verify_dquot_buf_item(
>
> for (i = 1; i < item->ri_total; i++) {
> if (item->ri_buf[i].i_addr == NULL) {
> - xfs_alert(mp,
> + xfs_alert(log->l_mp,
> "XFS: NULL dquot in %s.", __func__);
> return -EFSCORRUPTED;
> }
> if (item->ri_buf[i].i_len < sizeof(struct xfs_disk_dquot)) {
> - xfs_alert(mp,
> + xfs_alert(log->l_mp,
> "XFS: dquot too small (%d) in %s.",
> item->ri_buf[i].i_len, __func__);
> return -EFSCORRUPTED;
> }
> - fa = xfs_dquot_verify(mp, item->ri_buf[i].i_addr, -1);
> + fa = xfs_dquot_verify(log->l_mp, item->ri_buf[i].i_addr, -1);
> if (fa) {
> - xfs_alert(mp,
> + xfs_alert(log->l_mp,
> "dquot corrupt at %pS trying to replay into block 0x%llx",
> fa, xfs_buf_daddr(bp));
> return -EFSCORRUPTED;
> @@ -612,11 +611,12 @@ xlog_recover_verify_dquot_buf_item(
> */
> static int
> xlog_recover_do_inode_buffer(
> - struct xfs_mount *mp,
> + struct xlog *log,
> struct xlog_recover_item *item,
> struct xfs_buf *bp,
> struct xfs_buf_log_format *buf_f)
> {
> + struct xfs_sb *sbp = &log->l_mp->m_sb;
> int i;
> int item_index = 0;
> int bit = 0;
> @@ -628,7 +628,7 @@ xlog_recover_do_inode_buffer(
> xfs_agino_t *logged_nextp;
> xfs_agino_t *buffer_nextp;
>
> - trace_xfs_log_recover_buf_inode_buf(mp->m_log, buf_f);
> + trace_xfs_log_recover_buf_inode_buf(log, buf_f);
>
> /*
> * If the magic number doesn't match, something has gone wrong. Don't
> @@ -641,12 +641,12 @@ xlog_recover_do_inode_buffer(
> * Post recovery validation only works properly on CRC enabled
> * filesystems.
> */
> - if (xfs_has_crc(mp))
> + if (xfs_has_crc(log->l_mp))
> bp->b_ops = &xfs_inode_buf_ops;
>
> - inodes_per_buf = BBTOB(bp->b_length) >> mp->m_sb.sb_inodelog;
> + inodes_per_buf = BBTOB(bp->b_length) >> sbp->sb_inodelog;
> for (i = 0; i < inodes_per_buf; i++) {
> - next_unlinked_offset = (i * mp->m_sb.sb_inodesize) +
> + next_unlinked_offset = (i * sbp->sb_inodesize) +
> offsetof(struct xfs_dinode, di_next_unlinked);
>
> while (next_unlinked_offset >=
> @@ -695,8 +695,8 @@ xlog_recover_do_inode_buffer(
> */
> logged_nextp = item->ri_buf[item_index].i_addr +
> next_unlinked_offset - reg_buf_offset;
> - if (XFS_IS_CORRUPT(mp, *logged_nextp == 0)) {
> - xfs_alert(mp,
> + if (XFS_IS_CORRUPT(log->l_mp, *logged_nextp == 0)) {
> + xfs_alert(log->l_mp,
> "Bad inode buffer log record (ptr = "PTR_FMT", bp = "PTR_FMT"). "
> "Trying to replay bad (0) inode di_next_unlinked field.",
> item, bp);
> @@ -711,8 +711,8 @@ xlog_recover_do_inode_buffer(
> * have to leave the inode in a consistent state for whoever
> * reads it next....
> */
> - xfs_dinode_calc_crc(mp,
> - xfs_buf_offset(bp, i * mp->m_sb.sb_inodesize));
> + xfs_dinode_calc_crc(log->l_mp,
> + xfs_buf_offset(bp, i * sbp->sb_inodesize));
>
> }
>
> @@ -734,7 +734,7 @@ xlog_recover_do_inode_buffer(
> * it stale so that it won't be found on overlapping buffer lookups and
> * caller knows not to queue it for delayed write.
> */
> - if (BBTOB(bp->b_length) != M_IGEO(mp)->inode_cluster_size) {
> + if (BBTOB(bp->b_length) != M_IGEO(log->l_mp)->inode_cluster_size) {
> int error;
>
> error = xfs_bwrite(bp);
> @@ -792,7 +792,7 @@ xlog_recovery_is_dir_buf(
> */
> static void
> xlog_recover_do_partial_dabuf(
> - struct xfs_mount *mp,
> + struct xlog *log,
> struct xlog_recover_item *item,
> struct xfs_buf *bp,
> struct xfs_buf_log_format *buf_f)
> @@ -802,7 +802,7 @@ xlog_recover_do_partial_dabuf(
> * and rely on post pass2 recovery cache purge to clean these out of
> * memory.
> */
> - xlog_recover_buffer(mp, item, bp, buf_f);
> + xlog_recover_buffer(log, item, bp, buf_f);
> }
>
> /*
> @@ -827,7 +827,7 @@ xlog_recover_do_partial_dabuf(
> */
> static xfs_lsn_t
> xlog_recover_get_buf_lsn(
> - struct xfs_mount *mp,
> + struct xlog *log,
> struct xfs_buf *bp,
> struct xfs_buf_log_format *buf_f)
> {
> @@ -839,7 +839,7 @@ xlog_recover_get_buf_lsn(
> uint16_t blft;
>
> /* v4 filesystems always recover immediately */
> - if (!xfs_has_crc(mp))
> + if (!xfs_has_crc(log->l_mp))
> goto recover_immediately;
>
> /*
> @@ -916,7 +916,7 @@ xlog_recover_get_buf_lsn(
> * the relevant UUID in the superblock.
> */
> lsn = be64_to_cpu(((struct xfs_dsb *)blk)->sb_lsn);
> - if (xfs_has_metauuid(mp))
> + if (xfs_has_metauuid(log->l_mp))
> uuid = &((struct xfs_dsb *)blk)->sb_meta_uuid;
> else
> uuid = &((struct xfs_dsb *)blk)->sb_uuid;
> @@ -926,7 +926,7 @@ xlog_recover_get_buf_lsn(
> }
>
> if (lsn != (xfs_lsn_t)-1) {
> - if (!uuid_equal(&mp->m_sb.sb_meta_uuid, uuid))
> + if (!uuid_equal(&log->l_mp->m_sb.sb_meta_uuid, uuid))
> goto recover_immediately;
> return lsn;
> }
> @@ -945,7 +945,7 @@ xlog_recover_get_buf_lsn(
> }
>
> if (lsn != (xfs_lsn_t)-1) {
> - if (!uuid_equal(&mp->m_sb.sb_meta_uuid, uuid))
> + if (!uuid_equal(&log->l_mp->m_sb.sb_meta_uuid, uuid))
> goto recover_immediately;
> return lsn;
> }
> @@ -977,14 +977,14 @@ xlog_recover_get_buf_lsn(
> */
> static bool
> xlog_recover_this_buffer(
> - struct xfs_mount *mp,
> + struct xlog *log,
> struct xfs_buf *bp,
> struct xfs_buf_log_format *buf_f,
> xfs_lsn_t current_lsn)
> {
> xfs_lsn_t lsn;
>
> - lsn = xlog_recover_get_buf_lsn(mp, bp, buf_f);
> + lsn = xlog_recover_get_buf_lsn(log, bp, buf_f);
> if (!lsn)
> return true;
> if (lsn == -1)
> @@ -992,8 +992,8 @@ xlog_recover_this_buffer(
> if (XFS_LSN_CMP(lsn, current_lsn) < 0)
> return true;
>
> - trace_xfs_log_recover_buf_skip(mp->m_log, buf_f);
> - xlog_recover_validate_buf_type(mp, bp, buf_f, NULLCOMMITLSN);
> + trace_xfs_log_recover_buf_skip(log, buf_f);
> + xlog_recover_validate_buf_type(log, bp, buf_f, NULLCOMMITLSN);
>
> /*
> * We're skipping replay of this buffer log item due to the log
> @@ -1065,22 +1065,22 @@ xlog_recover_buf_commit_pass2(
> * to simplify the rest of the code.
> */
> if (buf_f->blf_flags & XFS_BLF_INODE_BUF) {
> - error = xlog_recover_do_inode_buffer(mp, item, bp, buf_f);
> + error = xlog_recover_do_inode_buffer(log, item, bp, buf_f);
> if (error || (bp->b_flags & XBF_STALE))
> goto out_release;
> goto out_write;
> }
>
> if (buf_f->blf_flags & XFS_BLF_DQUOT_BUF) {
> - if (!xlog_recover_this_dquot_buffer(mp, log, item, bp, buf_f))
> + if (!xlog_recover_this_dquot_buffer(log, item, bp, buf_f))
> goto out_release;
>
> - error = xlog_recover_verify_dquot_buf_item(mp, item, bp, buf_f);
> + error = xlog_recover_verify_dquot_buf_item(log, item, bp, buf_f);
> if (error)
> goto out_release;
>
> - xlog_recover_buffer(mp, item, bp, buf_f);
> - error = xlog_recover_validate_buf_type(mp, bp, buf_f,
> + xlog_recover_buffer(log, item, bp, buf_f);
> + error = xlog_recover_validate_buf_type(log, bp, buf_f,
> NULLCOMMITLSN);
> if (error)
> goto out_release;
> @@ -1095,14 +1095,14 @@ xlog_recover_buf_commit_pass2(
> */
> if (xlog_recovery_is_dir_buf(buf_f) &&
> mp->m_dir_geo->blksize != BBTOB(buf_f->blf_len)) {
> - xlog_recover_do_partial_dabuf(mp, item, bp, buf_f);
> + xlog_recover_do_partial_dabuf(log, item, bp, buf_f);
> goto out_write;
> }
>
> /*
> * Whole buffer recovery, dependent on the LSN in the on-disk structure.
> */
> - if (!xlog_recover_this_buffer(mp, bp, buf_f, current_lsn)) {
> + if (!xlog_recover_this_buffer(log, bp, buf_f, current_lsn)) {
> /*
> * We may have verified this buffer even though we aren't
> * recovering it. Return the verifier error for early detection
> @@ -1113,7 +1113,7 @@ xlog_recover_buf_commit_pass2(
> }
>
>
> - error = xlog_recover_do_reg_buffer(mp, item, bp, buf_f, current_lsn);
> + error = xlog_recover_do_reg_buffer(log, item, bp, buf_f, current_lsn);
> if (error)
> goto out_release;
>
> --
> 2.43.0
>
>
