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

["Darrick J. Wong" <darrick.wong@oracle.com>]

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@lst.de>