On Tue, Aug 27, 2024 at 04:35:33PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > Now that we've created inode fork helpers to allocate and free btree > roots, create a new bmap btree helper to create a new bmbt root, and > refactor the extents <-> btree conversion functions to use our new > helpers. > > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx> > --- > fs/xfs/libxfs/xfs_bmap.c | 20 ++++++-------------- > fs/xfs/libxfs/xfs_bmap_btree.c | 13 +++++++++++++ > fs/xfs/libxfs/xfs_bmap_btree.h | 2 ++ > 3 files changed, 21 insertions(+), 14 deletions(-) > > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index 00cac756c9566..e3922cf75381c 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -614,7 +614,7 @@ xfs_bmap_btree_to_extents( > xfs_trans_binval(tp, cbp); > if (cur->bc_levels[0].bp == cbp) > cur->bc_levels[0].bp = NULL; > - xfs_iroot_realloc(ip, -1, whichfork); > + xfs_iroot_free(ip, whichfork); I feel like the "whichfork" interface is unnecessary here. We already have the ifp in all cases here, and so xfs_iroot_free(ifp); avoids the need to look up the ifp again in xfs_iroot_free(). The same happens with xfs_iroot_alloc() - the callers already have the ifp in a local variable, so... > ASSERT(ifp->if_broot == NULL); > ifp->if_format = XFS_DINODE_FMT_EXTENTS; > *logflagsp |= XFS_ILOG_CORE | xfs_ilog_fext(whichfork); > @@ -655,19 +655,10 @@ xfs_bmap_extents_to_btree( > ASSERT(ifp->if_format == XFS_DINODE_FMT_EXTENTS); > > /* > - * Make space in the inode incore. This needs to be undone if we fail > - * to expand the root. > - */ > - xfs_iroot_realloc(ip, 1, whichfork); > - > - /* > - * Fill in the root. > - */ > - block = ifp->if_broot; > - xfs_bmbt_init_block(ip, block, NULL, 1, 1); > - /* > - * Need a cursor. Can't allocate until bb_level is filled in. > + * Fill in the root, create a cursor. Can't allocate until bb_level is > + * filled in. > */ > + xfs_bmbt_iroot_alloc(ip, whichfork); .... this becomes xfs_bmbt_iroot_alloc(ip, ifp); i.e. once we already have an ifp resolved for the fork, it makes no sense to pass whichfork down the stack instead of the ifp... -Dave. -- Dave Chinner david@xxxxxxxxxxxxx
