> +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@xxxxxx>