On Wed, May 06, 2020 at 08:03:24AM -0700, Christoph Hellwig wrote:
> On Mon, May 04, 2020 at 06:10:45PM -0700, Darrick J. Wong wrote:
> > +const struct xlog_recover_item_ops xlog_bmap_intent_item_ops = {
> > + .item_type = XFS_LI_BUI,
> > +};
> > +
> > +const struct xlog_recover_item_ops xlog_bmap_done_item_ops = {
> > + .item_type = XFS_LI_BUD,
> > +};
>
> Pretty much everything else in this file seems to use bui/bud names.
> The same also applies to the four other intent/done pairs and their
> shortnames. Not really a major thing, but it might be worth fixing
> to fit the flow.
Ok.
> > +STATIC enum xlog_recover_reorder
> > +xlog_recover_buf_reorder(
> > + struct xlog_recover_item *item)
> > +{
> > + struct xfs_buf_log_format *buf_f = item->ri_buf[0].i_addr;
> > +
> > + if (buf_f->blf_flags & XFS_BLF_CANCEL)
> > + return XLOG_REORDER_CANCEL_LIST;
> > + if (buf_f->blf_flags & XFS_BLF_INODE_BUF)
> > + return XLOG_REORDER_INODE_BUFFER_LIST;
> > + return XLOG_REORDER_BUFFER_LIST;
> > +}
>
> While you split this out a comment explaining the reordering would
> be nice here.
Ok.
/*
* Sort buffer items for log recovery. Most buffer items should end up
* on the buffer list and are recovered first, with the following
* exceptions:
*
* 1. XFS_BLF_CANCEL buffers must be processed last because some log
* items might depend on the incor ecancellation record, and
* replaying a cancelled buffer item can remove the incore record.
*
* 2. XFS_BLF_INODE_BUF buffers are handled after most regular items so
* that we replay di_next_unlinked only after flushing the inode
* 'free' state to the inode buffer.
*
* See xlog_recover_reorder_trans for more details.
*/
--D
> Otherwise this looks great:
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
