Re: [PATCH 18/47] xfs: refactor redo intent item processing

Christoph Hellwig
2016-08-01T08:10:33Z

> +int
> +xfs_efi_recover(
> + struct xfs_mount *mp,
> + struct xfs_efi_log_item *efip)
> +{
> + struct xfs_efd_log_item *efdp;
> + struct xfs_trans *tp;
> + int i;
> + int error = 0;
> + xfs_extent_t *extp;
> + xfs_fsblock_t startblock_fsb;
> +
> + ASSERT(!test_bit(XFS_EFI_RECOVERED, &efip->efi_flags));
> +
> + /*
> + * First check the validity of the extents described by the
> + * EFI. If any are bad, then assume that all are bad and
> + * just toss the EFI.
> + */
> + for (i = 0; i < efip->efi_format.efi_nextents; i++) {
> + extp = &(efip->efi_format.efi_extents[i]);
> + startblock_fsb = XFS_BB_TO_FSB(mp,
> + XFS_FSB_TO_DADDR(mp, extp->ext_start));
> + if ((startblock_fsb == 0) ||
> + (extp->ext_len == 0) ||
> + (startblock_fsb >= mp->m_sb.sb_dblocks) ||
> + (extp->ext_len >= mp->m_sb.sb_agblocks)) {
> + /*
> + * This will pull the EFI from the AIL and
> + * free the memory associated with it.
> + */
> + set_bit(XFS_EFI_RECOVERED, &efip->efi_flags);
> + xfs_efi_release(efip);
> + return -EIO;
> + }
> + }

I know it's just a code move, but there are lots of superflous braces
here, please remove them.

> + error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
> + if (error)
> + return error;
> + efdp = xfs_trans_get_efd(tp, efip, efip->efi_format.efi_nextents);
> +
> + for (i = 0; i < efip->efi_format.efi_nextents; i++) {
> + extp = &(efip->efi_format.efi_extents[i]);

and here..

> + ASSERT(XFS_LSN_CMP(last_lsn, lip->li_lsn) >= 0);

This new check seems useful, but nothing in the changelog mentions
why it has been added.

Otherwise this looks fine to me.