diff mbox series

[06/10] xfs: refactor the allocation and freeing of incore inode fork btree roots

Message ID 172480131609.2291268.5922161016077004451.stgit@frogsfrogsfrogs (mailing list archive)
State Not Applicable, archived
Headers show
Series [01/10] xfs: fix C++ compilation errors in xfs_fs.h | expand

Commit Message

Darrick J. Wong Aug. 27, 2024, 11:35 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

Refactor the code that allocates and freese the incore inode fork btree
roots.  This will help us disentangle some of the weird logic when we're
creating and tearing down inode-based btrees.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_inode_fork.c |   39 ++++++++++++++++++++++++++++++---------
 fs/xfs/libxfs/xfs_inode_fork.h |    3 +++
 2 files changed, 33 insertions(+), 9 deletions(-)

Comments

Christoph Hellwig Aug. 28, 2024, 4:15 a.m. UTC | #1 
Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Christoph Hellwig Aug. 28, 2024, 4:17 a.m. UTC | #2 
> +/* Allocate a new incore ifork btree root. */
> +void
> +xfs_iroot_alloc(
> +	struct xfs_inode	*ip,
> +	int			whichfork,
> +	size_t			bytes)
> +{
> +	struct xfs_ifork	*ifp = xfs_ifork_ptr(ip, whichfork);
> +
> +	ifp->if_broot = kmalloc(bytes,
> +				GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL);
> +	ifp->if_broot_bytes = bytes;
> +}

.. actually.  Maybe this shuld return ifp->if_broot?  I guess that
would helpful in a few callers to directly assign that to the variable
for the root block.  Similar to what I did with xfs_idata_realloc a
while ago.
Darrick J. Wong Aug. 28, 2024, 9:50 p.m. UTC | #3 
On Wed, Aug 28, 2024 at 06:17:12AM +0200, Christoph Hellwig wrote:
> > +/* Allocate a new incore ifork btree root. */
> > +void
> > +xfs_iroot_alloc(
> > +	struct xfs_inode	*ip,
> > +	int			whichfork,
> > +	size_t			bytes)
> > +{
> > +	struct xfs_ifork	*ifp = xfs_ifork_ptr(ip, whichfork);
> > +
> > +	ifp->if_broot = kmalloc(bytes,
> > +				GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL);
> > +	ifp->if_broot_bytes = bytes;
> > +}
> 
> .. actually.  Maybe this shuld return ifp->if_broot?  I guess that
> would helpful in a few callers to directly assign that to the variable
> for the root block.  Similar to what I did with xfs_idata_realloc a
> while ago.

Yeah, that would be useful later on.  I'll change that now.

--D
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index acb1e9cc45b76..60646a6c32ec7 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -211,9 +211,7 @@  xfs_iformat_btree(
 		return -EFSCORRUPTED;
 	}
 
-	ifp->if_broot_bytes = size;
-	ifp->if_broot = kmalloc(size,
-				GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL);
+	xfs_iroot_alloc(ip, whichfork, size);
 	ASSERT(ifp->if_broot != NULL);
 	/*
 	 * Copy and convert from the on-disk structure
@@ -362,6 +360,33 @@  xfs_iformat_attr_fork(
 	return error;
 }
 
+/* Allocate a new incore ifork btree root. */
+void
+xfs_iroot_alloc(
+	struct xfs_inode	*ip,
+	int			whichfork,
+	size_t			bytes)
+{
+	struct xfs_ifork	*ifp = xfs_ifork_ptr(ip, whichfork);
+
+	ifp->if_broot = kmalloc(bytes,
+				GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOFAIL);
+	ifp->if_broot_bytes = bytes;
+}
+
+/* Free all the memory and state associated with an incore ifork btree root. */
+void
+xfs_iroot_free(
+	struct xfs_inode	*ip,
+	int			whichfork)
+{
+	struct xfs_ifork	*ifp = xfs_ifork_ptr(ip, whichfork);
+
+	ifp->if_broot_bytes = 0;
+	kfree(ifp->if_broot);
+	ifp->if_broot = NULL;
+}
+
 /*
  * Reallocate the space for if_broot based on the number of records
  * being added or deleted as indicated in rec_diff.  Move the records
@@ -410,9 +435,7 @@  xfs_iroot_realloc(
 		 */
 		if (ifp->if_broot_bytes == 0) {
 			new_size = xfs_bmap_broot_space_calc(mp, rec_diff);
-			ifp->if_broot = kmalloc(new_size,
-						GFP_KERNEL | __GFP_NOFAIL);
-			ifp->if_broot_bytes = (int)new_size;
+			xfs_iroot_alloc(ip, whichfork, new_size);
 			return;
 		}
 
@@ -450,9 +473,7 @@  xfs_iroot_realloc(
 
 	new_size = xfs_bmap_broot_space_calc(mp, new_max);
 	if (new_size == 0) {
-		kfree(ifp->if_broot);
-		ifp->if_broot = NULL;
-		ifp->if_broot_bytes = 0;
+		xfs_iroot_free(ip, whichfork);
 		return;
 	}
 
diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index 2373d12fd474f..3f228a00b67dd 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -170,6 +170,9 @@  void		xfs_iflush_fork(struct xfs_inode *, struct xfs_dinode *,
 void		xfs_idestroy_fork(struct xfs_ifork *ifp);
 void *		xfs_idata_realloc(struct xfs_inode *ip, int64_t byte_diff,
 				int whichfork);
+void		xfs_iroot_alloc(struct xfs_inode *ip, int whichfork,
+				size_t bytes);
+void		xfs_iroot_free(struct xfs_inode *ip, int whichfork);
 void		xfs_iroot_realloc(struct xfs_inode *, int, int);
 int		xfs_iread_extents(struct xfs_trans *, struct xfs_inode *, int);
 int		xfs_iextents_copy(struct xfs_inode *, struct xfs_bmbt_rec *,