xfs: use ->t_firstblock in xattr ops

Similar to the dirops code, the xattr code uses an on-stack
firstblock variable for the various operations. This code rolls the
underlying transaction in various places, however, which means we
cannot simply replace the local firstblock vars with ->t_firstblock.
Doing so (without further changes) would invalidate the memory
pointed to by xfs_da_args.firstblock as soon as the first
transaction rolls.

To avoid this problem, remove xfs_da_args.firstblock and replace all
such accesses with ->t_firstblock at the same time. This ensures
that accesses to the current firstblock always occur through the
current transaction rather than a potentially invalid xfs_da_args
pointer.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 8a7e2c0308c4a2ebcd7e3df5f7ac823776e5163e..153d2e29f872fbd0a9d99ee04ee8d2e1b0929e98 100644 (file)
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -204,7 +204,6 @@ xfs_attr_set(
        struct xfs_da_args      args;
        struct xfs_defer_ops    dfops;
        struct xfs_trans_res    tres;
-       xfs_fsblock_t           firstblock;
        int                     rsvd = (flags & ATTR_ROOT) != 0;
        int                     error, err2, local;
 
@@ -219,7 +218,6 @@ xfs_attr_set(
 
        args.value = value;
        args.valuelen = valuelen;
-       args.firstblock = &firstblock;
        args.op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
        args.total = xfs_attr_calc_size(&args, &local);
 
@@ -253,7 +251,7 @@ xfs_attr_set(
                        rsvd ? XFS_TRANS_RESERVE : 0, &args.trans);
        if (error)
                return error;
-       xfs_defer_init(args.trans, &dfops, &firstblock);
+       xfs_defer_init(args.trans, &dfops, &args.trans->t_firstblock);
 
        xfs_ilock(dp, XFS_ILOCK_EXCL);
        error = xfs_trans_reserve_quota_nblks(args.trans, dp, args.total, 0,
@@ -392,7 +390,6 @@ xfs_attr_remove(
        struct xfs_mount        *mp = dp->i_mount;
        struct xfs_da_args      args;
        struct xfs_defer_ops    dfops;
-       xfs_fsblock_t           firstblock;
        int                     error;
 
        XFS_STATS_INC(mp, xs_attr_remove);
@@ -404,8 +401,6 @@ xfs_attr_remove(
        if (error)
                return error;
 
-       args.firstblock = &firstblock;
-
        /*
         * we have no control over the attribute names that userspace passes us
         * to remove, so we have to allow the name lookup prior to attribute
@@ -427,7 +422,7 @@ xfs_attr_remove(
                        &args.trans);
        if (error)
                return error;
-       xfs_defer_init(args.trans, &dfops, &firstblock);
+       xfs_defer_init(args.trans, &dfops, &args.trans->t_firstblock);
 
        xfs_ilock(dp, XFS_ILOCK_EXCL);
        /*
@@ -598,7 +593,8 @@ xfs_attr_leaf_addname(
                 * Commit that transaction so that the node_addname() call
                 * can manage its own transactions.
                 */
-               xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+               xfs_defer_init(args->trans, args->trans->t_dfops,
+                              &args->trans->t_firstblock);
                error = xfs_attr3_leaf_to_node(args);
                if (error)
                        goto out_defer_cancel;
@@ -687,8 +683,8 @@ xfs_attr_leaf_addname(
                 * If the result is small enough, shrink it all into the inode.
                 */
                if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
-                       xfs_defer_init(NULL, args->trans->t_dfops,
-                                      args->firstblock);
+                       xfs_defer_init(args->trans, args->trans->t_dfops,
+                                      &args->trans->t_firstblock);
                        error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
                        /* bp is gone due to xfs_da_shrink_inode */
                        if (error)
@@ -753,7 +749,8 @@ xfs_attr_leaf_removename(
         * If the result is small enough, shrink it all into the inode.
         */
        if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
-               xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+               xfs_defer_init(args->trans, args->trans->t_dfops,
+                              &args->trans->t_firstblock);
                error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
                /* bp is gone due to xfs_da_shrink_inode */
                if (error)
@@ -882,8 +879,8 @@ restart:
                         */
                        xfs_da_state_free(state);
                        state = NULL;
-                       xfs_defer_init(NULL, args->trans->t_dfops,
-                                      args->firstblock);
+                       xfs_defer_init(args->trans, args->trans->t_dfops,
+                                      &args->trans->t_firstblock);
                        error = xfs_attr3_leaf_to_node(args);
                        if (error)
                                goto out_defer_cancel;
@@ -910,7 +907,8 @@ restart:
                 * in the index/blkno/rmtblkno/rmtblkcnt fields and
                 * in the index2/blkno2/rmtblkno2/rmtblkcnt2 fields.
                 */
-               xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+               xfs_defer_init(args->trans, args->trans->t_dfops,
+                              &args->trans->t_firstblock);
                error = xfs_da3_split(state);
                if (error)
                        goto out_defer_cancel;
@@ -1008,8 +1006,8 @@ restart:
                 * Check to see if the tree needs to be collapsed.
                 */
                if (retval && (state->path.active > 1)) {
-                       xfs_defer_init(NULL, args->trans->t_dfops,
-                                      args->firstblock);
+                       xfs_defer_init(args->trans, args->trans->t_dfops,
+                                      &args->trans->t_firstblock);
                        error = xfs_da3_join(state);
                        if (error)
                                goto out_defer_cancel;
@@ -1134,7 +1132,8 @@ xfs_attr_node_removename(
         * Check to see if the tree needs to be collapsed.
         */
        if (retval && (state->path.active > 1)) {
-               xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+               xfs_defer_init(args->trans, args->trans->t_dfops,
+                              &args->trans->t_firstblock);
                error = xfs_da3_join(state);
                if (error)
                        goto out_defer_cancel;
@@ -1166,8 +1165,8 @@ xfs_attr_node_removename(
                        goto out;
 
                if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
-                       xfs_defer_init(NULL, args->trans->t_dfops,
-                                      args->firstblock);
+                       xfs_defer_init(args->trans, args->trans->t_dfops,
+                                      &args->trans->t_firstblock);
                        error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
                        /* bp is gone due to xfs_da_shrink_inode */
                        if (error)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index c131469db0f18d6b8fc5b4ffe0b9330110d399f5..251304f3bc5dc67f66a6ea894618ead97386725a 100644 (file)
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -802,7 +802,6 @@ xfs_attr_shortform_to_leaf(
        memset((char *)&nargs, 0, sizeof(nargs));
        nargs.dp = dp;
        nargs.geo = args->geo;
-       nargs.firstblock = args->firstblock;
        nargs.total = args->total;
        nargs.whichfork = XFS_ATTR_FORK;
        nargs.trans = args->trans;
@@ -1005,7 +1004,6 @@ xfs_attr3_leaf_to_shortform(
        memset((char *)&nargs, 0, sizeof(nargs));
        nargs.geo = args->geo;
        nargs.dp = dp;
-       nargs.firstblock = args->firstblock;
        nargs.total = args->total;
        nargs.whichfork = XFS_ATTR_FORK;
        nargs.trans = args->trans;

diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index ab7c2755ad8cd7d35ab5fcadf9c36da6bb255495..205098aeb4bca4d2f5837ce2fd233137451b48a0 100644 (file)
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -480,11 +480,13 @@ xfs_attr_rmtval_set(
                 * extent and then crash then the block may not contain the
                 * correct metadata after log recovery occurs.
                 */
-               xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+               xfs_defer_init(args->trans, args->trans->t_dfops,
+                              &args->trans->t_firstblock);
                nmap = 1;
                error = xfs_bmapi_write(args->trans, dp, (xfs_fileoff_t)lblkno,
-                                 blkcnt, XFS_BMAPI_ATTRFORK, args->firstblock,
-                                 args->total, &map, &nmap);
+                                 blkcnt, XFS_BMAPI_ATTRFORK,
+                                 &args->trans->t_firstblock, args->total, &map,
+                                 &nmap);
                if (error)
                        goto out_defer_cancel;
                xfs_defer_ijoin(args->trans->t_dfops, dp);
@@ -522,7 +524,8 @@ xfs_attr_rmtval_set(
 
                ASSERT(blkcnt > 0);
 
-               xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+               xfs_defer_init(args->trans, args->trans->t_dfops,
+                              &args->trans->t_firstblock);
                nmap = 1;
                error = xfs_bmapi_read(dp, (xfs_fileoff_t)lblkno,
                                       blkcnt, &map, &nmap,
@@ -626,10 +629,11 @@ xfs_attr_rmtval_remove(
        blkcnt = args->rmtblkcnt;
        done = 0;
        while (!done) {
-               xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+               xfs_defer_init(args->trans, args->trans->t_dfops,
+                              &args->trans->t_firstblock);
                error = xfs_bunmapi(args->trans, args->dp, lblkno, blkcnt,
-                                   XFS_BMAPI_ATTRFORK, 1, args->firstblock,
-                                   &done);
+                                   XFS_BMAPI_ATTRFORK, 1,
+                                   &args->trans->t_firstblock, &done);
                if (error)
                        goto out_defer_cancel;
                xfs_defer_ijoin(args->trans->t_dfops, args->dp);

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index da73c1a011d36992aefbe21ef4eead3569f55c95..6f9b2cddb933d11f5c6fed7fb6d057d8c200710d 100644 (file)
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1000,7 +1000,6 @@ xfs_bmap_add_attrfork_local(
                memset(&dargs, 0, sizeof(dargs));
                dargs.geo = ip->i_mount->m_dir_geo;
                dargs.dp = ip;
-               dargs.firstblock = &tp->t_firstblock;
                dargs.total = dargs.geo->fsbcount;
                dargs.whichfork = XFS_DATA_FORK;
                dargs.trans = tp;

diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 68a72e3d9f5328aa9458097737e68a8c990046f1..2f2be86c10dc16fc44b2902d522a552d1e4dd852 100644 (file)
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -2059,10 +2059,9 @@ xfs_da_grow_inode_int(
         * Try mapping it in one filesystem block.
         */
        nmap = 1;
-       ASSERT(args->firstblock != NULL);
        error = xfs_bmapi_write(tp, dp, *bno, count,
                        xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA|XFS_BMAPI_CONTIG,
-                       args->firstblock, args->total, &map, &nmap);
+                       &tp->t_firstblock, args->total, &map, &nmap);
        if (error)
                return error;
 
@@ -2084,7 +2083,7 @@ xfs_da_grow_inode_int(
                        c = (int)(*bno + count - b);
                        error = xfs_bmapi_write(tp, dp, b, c,
                                        xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA,
-                                       args->firstblock, args->total,
+                                       &tp->t_firstblock, args->total,
                                        &mapp[mapi], &nmap);
                        if (error)
                                goto out_free_map;
@@ -2394,7 +2393,7 @@ xfs_da_shrink_inode(
                 * the last block to the place we want to kill.
                 */
                error = xfs_bunmapi(tp, dp, dead_blkno, count,
-                                   xfs_bmapi_aflag(w), 0, args->firstblock,
+                                   xfs_bmapi_aflag(w), 0, &tp->t_firstblock,
                                    &done);
                if (error == -ENOSPC) {
                        if (w != XFS_DATA_FORK)

diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
index 6b8a04f3f16256d6697f79dad88600db68145ed7..59e290ef334f72bd57312796aaa179ebcdd408f9 100644 (file)
--- a/fs/xfs/libxfs/xfs_da_btree.h
+++ b/fs/xfs/libxfs/xfs_da_btree.h
@@ -57,7 +57,6 @@ typedef struct xfs_da_args {
        xfs_dahash_t    hashval;        /* hash value of name */
        xfs_ino_t       inumber;        /* input/output inode number */
        struct xfs_inode *dp;           /* directory inode to manipulate */
-       xfs_fsblock_t   *firstblock;    /* ptr to firstblock for bmap calls */
        struct xfs_trans *trans;        /* current trans (changes over time) */
        xfs_extlen_t    total;          /* total blocks needed, for 1st bmap */
        int             whichfork;      /* data or attribute fork */

diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index a3983e3eb64a2385dcb519bd2440e5003ad62c0f..5db73d96b99ed4475db70aa39be02d4578af0d9f 100644 (file)
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -272,7 +272,6 @@ xfs_dir_createname(
        args->total = total;
        args->whichfork = XFS_DATA_FORK;
        args->trans = tp;
-       args->firstblock = &tp->t_firstblock;
        args->op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
        if (!inum)
                args->op_flags |= XFS_DA_OP_JUSTCHECK;
@@ -439,7 +438,6 @@ xfs_dir_removename(
        args->hashval = dp->i_mount->m_dirnameops->hashname(name);
        args->inumber = ino;
        args->dp = dp;
-       args->firstblock = &tp->t_firstblock;
        args->total = total;
        args->whichfork = XFS_DATA_FORK;
        args->trans = tp;
@@ -502,7 +500,6 @@ xfs_dir_replace(
        args->hashval = dp->i_mount->m_dirnameops->hashname(name);
        args->inumber = inum;
        args->dp = dp;
-       args->firstblock = &tp->t_firstblock;
        args->total = total;
        args->whichfork = XFS_DATA_FORK;
        args->trans = tp;
@@ -660,7 +657,7 @@ xfs_dir2_shrink_inode(
 
        /* Unmap the fsblock(s). */
        error = xfs_bunmapi(tp, dp, da, args->geo->fsbcount, 0, 0,
-                           args->firstblock, &done);
+                           &tp->t_firstblock, &done);
        if (error) {
                /*
                 * ENOSPC actually can happen if we're in a removename with no