Re: [PATCH 8/8] xfs: intent item whiteouts

[Dave Chinner <david@fromorbit.com>]

On Thu, Apr 28, 2022 at 06:22:06AM -0700, Christoph Hellwig wrote:
> > --- a/fs/xfs/xfs_bmap_item.c
> > +++ b/fs/xfs/xfs_bmap_item.c
> > @@ -39,6 +39,7 @@ STATIC void
> >  xfs_bui_item_free(
> >  	struct xfs_bui_log_item	*buip)
> >  {
> > +	kmem_free(buip->bui_item.li_lv_shadow);
> >  	kmem_cache_free(xfs_bui_cache, buip);
> >  }
> 
> Based on the discussion with Darrick:  what about splitting adding
> the frees of the shadow buffers into a separate prep patch?

Ok, I can do that easily enough.

> > +	/* clean up log items that had whiteouts */
> > +	while (!list_empty(&whiteouts)) {
> > +		struct xfs_log_item *item = list_first_entry(&whiteouts,
> > +						struct xfs_log_item, li_cil);
> > +		list_del_init(&item->li_cil);
> > +		trace_xfs_cil_whiteout_unpin(item);
> > +		item->li_ops->iop_unpin(item, 1);
> > +	}
> >  	return;
> >  
> >  out_skip:
> > @@ -1212,6 +1236,14 @@ xlog_cil_push_work(
> >  out_abort_free_ticket:
> >  	xfs_log_ticket_ungrant(log, ctx->ticket);
> >  	ASSERT(xlog_is_shutdown(log));
> > +	while (!list_empty(&whiteouts)) {
> > +		struct xfs_log_item *item = list_first_entry(&whiteouts,
> > +						struct xfs_log_item, li_cil);
> > +		list_del_init(&item->li_cil);
> > +		trace_xfs_cil_whiteout_unpin(item);
> > +		item->li_ops->iop_unpin(item, 1);
> > +	}
> 
> Sees like this would benefit from a little helper instead of
> duplicating the logic?

Yeah, when I looked over this yesterday I was in two minds whether a
helper was justified or not. Seeing as you've suggested it too, I'll
factor it out....

> Otherwise this does look surprisingly nice and simple:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks!

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com