Re: [PATCH 14/28] xfs: refactor recovered EFI log item playback

Christoph Hellwig
2020-05-06T15:18:13Z

> +static const struct xfs_item_ops xfs_efi_item_ops = {
> + .iop_size = xfs_efi_item_size,
> + .iop_format = xfs_efi_item_format,
> + .iop_unpin = xfs_efi_item_unpin,
> + .iop_release = xfs_efi_item_release,
> + .iop_recover = xfs_efi_item_recover,
> +};
> +
> +

I guess we can drop the second empty line here.

> switch (lip->li_type) {
> case XFS_LI_EFI:
> - error = xlog_recover_process_efi(log->l_mp, ailp, lip);
> + error = lip->li_ops->iop_recover(lip, parent_tp);
> break;
> case XFS_LI_RUI:
> error = xlog_recover_process_rui(log->l_mp, ailp, lip);
> @@ -2893,7 +2853,9 @@ xlog_recover_cancel_intents(
>
> switch (lip->li_type) {
> case XFS_LI_EFI:
> - xlog_recover_cancel_efi(log->l_mp, ailp, lip);
> + spin_unlock(&ailp->ail_lock);
> + lip->li_ops->iop_release(lip);
> + spin_lock(&ailp->ail_lock);

This looks a little weird, as I'd expect the default statement
to handle the "generic" case. But then again this is all transitional,
so except for minor nitpick above:

Reviewed-by: Christoph Hellwig <hch@lst.de>