On Thu, Jul 27, 2023 at 03:24:16PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> Create a new xrep_newbt structure to encapsulate a fake root for
> creating a staged btree cursor as well as to track all the blocks that
> we need to reserve in order to build that btree.
>
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
.....
> +/* Allocate disk space for our new file-based btree. */
> +STATIC int
> +xrep_newbt_alloc_file_blocks(
> + struct xrep_newbt *xnr,
> + uint64_t nr_blocks)
> +{
> + struct xfs_scrub *sc = xnr->sc;
> + int error = 0;
> +
> + while (nr_blocks > 0) {
> + struct xfs_alloc_arg args = {
> + .tp = sc->tp,
> + .mp = sc->mp,
> + .oinfo = xnr->oinfo,
> + .minlen = 1,
> + .maxlen = nr_blocks,
> + .prod = 1,
> + .resv = xnr->resv,
> + };
> + struct xfs_perag *pag;
> +
> + xrep_newbt_validate_file_alloc_hint(xnr);
> +
> + error = xfs_alloc_vextent_start_ag(&args, xnr->alloc_hint);
> + if (error)
> + return error;
> + if (args.fsbno == NULLFSBLOCK)
> + return -ENOSPC;
> +
> + trace_xrep_newbt_alloc_file_blocks(sc->mp, args.agno,
> + args.agbno, args.len, xnr->oinfo.oi_owner);
> +
> + pag = xfs_perag_get(sc->mp, args.agno);
I don't think we should allow callers to trust args.agno and
args.agbno after the allocation has completed. The result of the
allocation is returned in args.fsbno, and there is no guarantee that
args.agno and args.agbno will be valid at the completion of the
allocation.
i.e. we set args.agno and args.agbno internally based on the target
that is passed to xfs_alloc_vextent_start_ag(), and they change
internally depending on the iterations being done during allocation.
IOWs, those two fields are internal allocation state and not
actually return values that the caller can rely on.
Hence I think this needs to do:
agno = XFS_FSB_TO_AGNO(mp, args.fsbno);
agbno = XFS_FSB_TO_AGBNO(mp, args.fsbno);
before using those values.
> +
> +/*
> + * How many extent freeing items can we attach to a transaction before we want
> + * to finish the chain so that unreserving new btree blocks doesn't overrun
> + * the transaction reservation?
> + */
> +#define XREP_REAP_MAX_NEWBT_EFIS (128)
Should there be a common define for this for repair operations?
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
