[5/7] libxfs: pass a transaction context through listxattr

Commit Message

Darrick J. Wong Aug. 6, 2024, 6:20 p.m. UTC

From: Darrick J. Wong <[e-mail]>

Pass a transaction context so that a new caller can walk the attr names
and query the values all in one go without deadlocking on nested buffer
access.

While we're at it, make the existing xfs_repair callers try to use
empty transactions so that we don't deadlock on cycles in the xattr
structure.

Signed-off-by: Darrick J. Wong <[e-mail]>
Reviewed-by: Christoph Hellwig <[e-mail]>
Acked-by: Dave Chinner <[e-mail]>
---
 libxfs/listxattr.c |   40 +++++++++++++++++++++++-----------------
 libxfs/listxattr.h |    6 ++++--
 repair/pptr.c      |    7 ++++++-
 3 files changed, 33 insertions(+), 20 deletions(-)

Patch

diff --git a/libxfs/listxattr.c b/libxfs/listxattr.c
index bedaca678..34205682f 100644
--- a/libxfs/listxattr.c
+++ b/libxfs/listxattr.c
@@ -11,6 +11,7 @@ 
 /* Call a function for every entry in a shortform xattr structure. */
 STATIC int
 xattr_walk_sf(
+	struct xfs_trans		*tp,
 	struct xfs_inode		*ip,
 	xattr_walk_fn			attr_fn,
 	void				*priv)
@@ -22,7 +23,7 @@  xattr_walk_sf(
 
 	sfe = libxfs_attr_sf_firstentry(hdr);
 	for (i = 0; i < hdr->count; i++) {
-		error = attr_fn(ip, sfe->flags, sfe->nameval, sfe->namelen,
+		error = attr_fn(tp, ip, sfe->flags, sfe->nameval, sfe->namelen,
 				&sfe->nameval[sfe->namelen], sfe->valuelen,
 				priv);
 		if (error)
@@ -37,6 +38,7 @@  xattr_walk_sf(
 /* Call a function for every entry in this xattr leaf block. */
 STATIC int
 xattr_walk_leaf_entries(
+	struct xfs_trans		*tp,
 	struct xfs_inode		*ip,
 	xattr_walk_fn			attr_fn,
 	struct xfs_buf			*bp,
@@ -75,7 +77,7 @@  xattr_walk_leaf_entries(
 			valuelen = be32_to_cpu(name_rmt->valuelen);
 		}
 
-		error = attr_fn(ip, entry->flags, name, namelen, value,
+		error = attr_fn(tp, ip, entry->flags, name, namelen, value,
 				valuelen, priv);
 		if (error)
 			return error;
@@ -91,6 +93,7 @@  xattr_walk_leaf_entries(
  */
 STATIC int
 xattr_walk_leaf(
+	struct xfs_trans		*tp,
 	struct xfs_inode		*ip,
 	xattr_walk_fn			attr_fn,
 	void				*priv)
@@ -98,18 +101,19 @@  xattr_walk_leaf(
 	struct xfs_buf			*leaf_bp;
 	int				error;
 
-	error = -libxfs_attr3_leaf_read(NULL, ip, ip->i_ino, 0, &leaf_bp);
+	error = -libxfs_attr3_leaf_read(tp, ip, ip->i_ino, 0, &leaf_bp);
 	if (error)
 		return error;
 
-	error = xattr_walk_leaf_entries(ip, attr_fn, leaf_bp, priv);
-	libxfs_trans_brelse(NULL, leaf_bp);
+	error = xattr_walk_leaf_entries(tp, ip, attr_fn, leaf_bp, priv);
+	libxfs_trans_brelse(tp, leaf_bp);
 	return error;
 }
 
 /* Find the leftmost leaf in the xattr dabtree. */
 STATIC int
 xattr_walk_find_leftmost_leaf(
+	struct xfs_trans		*tp,
 	struct xfs_inode		*ip,
 	struct bitmap			*seen_blocks,
 	struct xfs_buf			**leaf_bpp)
@@ -127,7 +131,7 @@  xattr_walk_find_leftmost_leaf(
 	for (;;) {
 		uint16_t		magic;
 
-		error = -libxfs_da3_node_read(NULL, ip, blkno, &bp,
+		error = -libxfs_da3_node_read(tp, ip, blkno, &bp,
 				XFS_ATTR_FORK);
 		if (error)
 			return error;
@@ -164,7 +168,7 @@  xattr_walk_find_leftmost_leaf(
 		/* Find the next level towards the leaves of the dabtree. */
 		btree = nodehdr.btree;
 		blkno = be32_to_cpu(btree->before);
-		libxfs_trans_brelse(NULL, bp);
+		libxfs_trans_brelse(tp, bp);
 
 		/* Make sure we haven't seen this new block already. */
 		if (bitmap_test(seen_blocks, blkno, 1))
@@ -184,13 +188,14 @@  xattr_walk_find_leftmost_leaf(
 	return 0;
 
 out_buf:
-	libxfs_trans_brelse(NULL, bp);
+	libxfs_trans_brelse(tp, bp);
 	return error;
 }
 
 /* Call a function for every entry in a node-format xattr structure. */
 STATIC int
 xattr_walk_node(
+	struct xfs_trans		*tp,
 	struct xfs_inode		*ip,
 	xattr_walk_fn			attr_fn,
 	void				*priv)
@@ -204,12 +209,12 @@  xattr_walk_node(
 
 	bitmap_alloc(&seen_blocks);
 
-	error = xattr_walk_find_leftmost_leaf(ip, seen_blocks, &leaf_bp);
+	error = xattr_walk_find_leftmost_leaf(tp, ip, seen_blocks, &leaf_bp);
 	if (error)
 		goto out_bitmap;
 
 	for (;;) {
-		error = xattr_walk_leaf_entries(ip, attr_fn, leaf_bp,
+		error = xattr_walk_leaf_entries(tp, ip, attr_fn, leaf_bp,
 				priv);
 		if (error)
 			goto out_leaf;
@@ -220,13 +225,13 @@  xattr_walk_node(
 		if (leafhdr.forw == 0)
 			goto out_leaf;
 
-		libxfs_trans_brelse(NULL, leaf_bp);
+		libxfs_trans_brelse(tp, leaf_bp);
 
 		/* Make sure we haven't seen this new leaf already. */
 		if (bitmap_test(seen_blocks, leafhdr.forw, 1))
 			goto out_bitmap;
 
-		error = -libxfs_attr3_leaf_read(NULL, ip, ip->i_ino,
+		error = -libxfs_attr3_leaf_read(tp, ip, ip->i_ino,
 				leafhdr.forw, &leaf_bp);
 		if (error)
 			goto out_bitmap;
@@ -238,7 +243,7 @@  xattr_walk_node(
 	}
 
 out_leaf:
-	libxfs_trans_brelse(NULL, leaf_bp);
+	libxfs_trans_brelse(tp, leaf_bp);
 out_bitmap:
 	bitmap_free(&seen_blocks);
 	return error;
@@ -247,6 +252,7 @@  xattr_walk_node(
 /* Call a function for every extended attribute in a file. */
 int
 xattr_walk(
+	struct xfs_trans	*tp,
 	struct xfs_inode	*ip,
 	xattr_walk_fn		attr_fn,
 	void			*priv)
@@ -257,15 +263,15 @@  xattr_walk(
 		return 0;
 
 	if (ip->i_af.if_format == XFS_DINODE_FMT_LOCAL)
-		return xattr_walk_sf(ip, attr_fn, priv);
+		return xattr_walk_sf(tp, ip, attr_fn, priv);
 
 	/* attr functions require that the attr fork is loaded */
-	error = -libxfs_iread_extents(NULL, ip, XFS_ATTR_FORK);
+	error = -libxfs_iread_extents(tp, ip, XFS_ATTR_FORK);
 	if (error)
 		return error;
 
 	if (libxfs_attr_is_leaf(ip))
-		return xattr_walk_leaf(ip, attr_fn, priv);
+		return xattr_walk_leaf(tp, ip, attr_fn, priv);
 
-	return xattr_walk_node(ip, attr_fn, priv);
+	return xattr_walk_node(tp, ip, attr_fn, priv);
 }
diff --git a/libxfs/listxattr.h b/libxfs/listxattr.h
index cddd96af7..933e0f529 100644
--- a/libxfs/listxattr.h
+++ b/libxfs/listxattr.h
@@ -6,10 +6,12 @@ 
 #ifndef __LIBXFS_LISTXATTR_H__
 #define __LIBXFS_LISTXATTR_H__
 
-typedef int (*xattr_walk_fn)(struct xfs_inode *ip, unsigned int attr_flags,
+typedef int (*xattr_walk_fn)(struct xfs_trans *tp, struct xfs_inode *ip,
+		unsigned int attr_flags,
 		const unsigned char *name, unsigned int namelen,
 		const void *value, unsigned int valuelen, void *priv);
 
-int xattr_walk(struct xfs_inode *ip, xattr_walk_fn attr_fn, void *priv);
+int xattr_walk(struct xfs_trans *tp, struct xfs_inode *ip,
+		xattr_walk_fn attr_fn, void *priv);
 
 #endif /* __LIBXFS_LISTXATTR_H__ */
diff --git a/repair/pptr.c b/repair/pptr.c
index cc66e6372..ee29e47a8 100644
--- a/repair/pptr.c
+++ b/repair/pptr.c
@@ -593,6 +593,7 @@  store_file_pptr_name(
 /* Decide if this is a directory parent pointer and stash it if so. */
 static int
 examine_xattr(
+	struct xfs_trans	*tp,
 	struct xfs_inode	*ip,
 	unsigned int		attr_flags,
 	const unsigned char	*name,
@@ -1205,6 +1206,7 @@  check_file_parent_ptrs(
 	struct xfs_inode	*ip,
 	struct file_scan	*fscan)
 {
+	struct xfs_trans	*tp = NULL;
 	int			error;
 
 	error = -init_slab(&fscan->file_pptr_recs, sizeof(struct file_pptr));
@@ -1215,7 +1217,10 @@  check_file_parent_ptrs(
 	fscan->have_garbage = false;
 	fscan->nr_file_pptrs = 0;
 
-	error = xattr_walk(ip, examine_xattr, fscan);
+	libxfs_trans_alloc_empty(ip->i_mount, &tp);
+	error = xattr_walk(tp, ip, examine_xattr, fscan);
+	if (tp)
+		libxfs_trans_cancel(tp);
 	if (error && !no_modify)
 		do_error(_("ino %llu parent pointer scan failed: %s\n"),
 				(unsigned long long)ip->i_ino,