libxfs: pass a transaction context through listxattr

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 <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Acked-by: Dave Chinner <dchinner@redhat.com>

diff --git a/libxfs/listxattr.c b/libxfs/listxattr.c
index bedaca678439e93c62b9a33e53925eacfc6da218..34205682f022d5823be64963a92ee096d08ccc86 100644 (file)
--- 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 @@ out_bitmap:
 /* 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 cddd96af7c0c0f02b108432f1cdfe89eaa319044..933e0f529ec4b33a49c4329ee47d5539bab44949 100644 (file)
--- 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 cc66e637217f1d0116a52fc4ce4dd04704efc05c..ee29e47a87bd0730f76e9117e2aadd8c0c08109d 100644 (file)
--- 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,