On Wed, May 06, 2020 at 08:18:12AM -0700, Christoph Hellwig wrote:
> > +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:
Hmm, that does make more sense; I'll change it.
--D
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
