Re: [PATCH 3/6] xfs: log EFIs for all btree blocks being used to stage a btree

Dave Chinner
2023-08-07T08:41:59Z

On Thu, Jul 27, 2023 at 03:24:32PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> We need to log EFIs for every extent that we allocate for the purpose of
> staging a new btree so that if we fail then the blocks will be freed
> during log recovery. Add a function to relog the EFIs, so that repair
> can relog them all every time it creates a new btree block, which will
> help us to avoid pinning the log tail.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
.....
> +/*
> + * Set up automatic reaping of the blocks reserved for btree reconstruction in
> + * case we crash by logging a deferred free item for each extent we allocate so
> + * that we can get all of the space back if we crash before we can commit the
> + * new btree. This function returns a token that can be used to cancel
> + * automatic reaping if repair is successful.
> + */
> +static int
> +xrep_newbt_schedule_autoreap(
> + struct xrep_newbt *xnr,
> + struct xrep_newbt_resv *resv)
> +{
> + struct xfs_extent_free_item efi_item = {
> + .xefi_blockcount = resv->len,
> + .xefi_owner = xnr->oinfo.oi_owner,
> + .xefi_flags = XFS_EFI_SKIP_DISCARD,
> + .xefi_pag = resv->pag,
> + };
> + struct xfs_scrub *sc = xnr->sc;
> + struct xfs_log_item *lip;
> + LIST_HEAD(items);
> +
> + ASSERT(xnr->oinfo.oi_offset == 0);
> +
> + efi_item.xefi_startblock = XFS_AGB_TO_FSB(sc->mp, resv->pag->pag_agno,
> + resv->agbno);
> + if (xnr->oinfo.oi_flags & XFS_OWNER_INFO_ATTR_FORK)
> + efi_item.xefi_flags |= XFS_EFI_ATTR_FORK;
> + if (xnr->oinfo.oi_flags & XFS_OWNER_INFO_BMBT_BLOCK)
> + efi_item.xefi_flags |= XFS_EFI_BMBT_BLOCK;
> +
> + INIT_LIST_HEAD(&efi_item.xefi_list);
> + list_add(&efi_item.xefi_list, &items);
> +
> + xfs_perag_intent_hold(resv->pag);
> + lip = xfs_extent_free_defer_type.create_intent(sc->tp, &items, 1,
> + false);

Hmmmm.

That triggered flashing lights and sirens - I'm not sure I really
like the usage of the defer type arrays like this, nor the
duplication of the defer mechanisms for relogging, etc.

Not that I have a better idea right now - is this the final form of
this code, or is more stuff built on top of it or around it?

-Dave.
--
Dave Chinner
david@fromorbit.com