On Thu, Aug 29, 2024 at 12:13:09PM +1000, Dave Chinner wrote: > 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(). Yeah, that makes sense. > 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... Makes sense here too, will change both. --D > -Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx >
