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@xxxxxx>
Thanks!
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
