]> git.ipfire.org Git - thirdparty/xfsprogs-dev.git/blobdiff - libxfs/xfs_attr.c
xfs: remove unnecessary dfops init calls in xattr code
[thirdparty/xfsprogs-dev.git] / libxfs / xfs_attr.c
index f8c24f6d89f1c1d1701b0dd4ae80564a72524d65..c14e8a346fcea41da713123b422b54fed027af58 100644 (file)
@@ -1,19 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (c) 2000-2005 Silicon Graphics, Inc.
  * All Rights Reserved.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write the Free Software Foundation,
- * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 #include "libxfs_priv.h"
 #include "xfs_fs.h"
@@ -23,6 +11,7 @@
 #include "xfs_trans_resv.h"
 #include "xfs_bit.h"
 #include "xfs_mount.h"
+#include "xfs_defer.h"
 #include "xfs_da_format.h"
 #include "xfs_da_btree.h"
 #include "xfs_attr_sf.h"
@@ -108,6 +97,25 @@ xfs_inode_hasattr(
  * Overall external interface routines.
  *========================================================================*/
 
+/* Retrieve an extended attribute and its value.  Must have ilock. */
+int
+xfs_attr_get_ilocked(
+       struct xfs_inode        *ip,
+       struct xfs_da_args      *args)
+{
+       ASSERT(xfs_isilocked(ip, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
+
+       if (!xfs_inode_hasattr(ip))
+               return -ENOATTR;
+       else if (ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL)
+               return xfs_attr_shortform_getvalue(args);
+       else if (xfs_bmap_one_block(ip, XFS_ATTR_FORK))
+               return xfs_attr_leaf_get(args);
+       else
+               return xfs_attr_node_get(args);
+}
+
+/* Retrieve an extended attribute by name, and its value. */
 int
 xfs_attr_get(
        struct xfs_inode        *ip,
@@ -120,30 +128,22 @@ xfs_attr_get(
        uint                    lock_mode;
        int                     error;
 
-       XFS_STATS_INC(xs_attr_get);
+       XFS_STATS_INC(ip->i_mount, xs_attr_get);
 
        if (XFS_FORCED_SHUTDOWN(ip->i_mount))
                return -EIO;
 
-       if (!xfs_inode_hasattr(ip))
-               return -ENOATTR;
-
        error = xfs_attr_args_init(&args, ip, name, flags);
        if (error)
                return error;
 
        args.value = value;
        args.valuelen = *valuelenp;
+       /* Entirely possible to look up a name which doesn't exist */
+       args.op_flags = XFS_DA_OP_OKNOENT;
 
        lock_mode = xfs_ilock_attr_map_shared(ip);
-       if (!xfs_inode_hasattr(ip))
-               error = -ENOATTR;
-       else if (ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL)
-               error = xfs_attr_shortform_getvalue(&args);
-       else if (xfs_bmap_one_block(ip, XFS_ATTR_FORK))
-               error = xfs_attr_leaf_get(&args);
-       else
-               error = xfs_attr_node_get(&args);
+       error = xfs_attr_get_ilocked(ip, &args);
        xfs_iunlock(ip, lock_mode);
 
        *valuelenp = args.valuelen;
@@ -195,14 +195,13 @@ xfs_attr_set(
        int                     flags)
 {
        struct xfs_mount        *mp = dp->i_mount;
+       struct xfs_buf          *leaf_bp = NULL;
        struct xfs_da_args      args;
-       struct xfs_bmap_free    flist;
        struct xfs_trans_res    tres;
-       xfs_fsblock_t           firstblock;
        int                     rsvd = (flags & ATTR_ROOT) != 0;
-       int                     error, err2, committed, local;
+       int                     error, err2, local;
 
-       XFS_STATS_INC(xs_attr_set);
+       XFS_STATS_INC(mp, xs_attr_set);
 
        if (XFS_FORCED_SHUTDOWN(dp->i_mount))
                return -EIO;
@@ -213,12 +212,10 @@ xfs_attr_set(
 
        args.value = value;
        args.valuelen = valuelen;
-       args.firstblock = &firstblock;
-       args.flist = &flist;
        args.op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
        args.total = xfs_attr_calc_size(&args, &local);
 
-       error = xfs_qm_dqattach(dp, 0);
+       error = xfs_qm_dqattach(dp);
        if (error)
                return error;
 
@@ -235,43 +232,27 @@ xfs_attr_set(
                        return error;
        }
 
-       /*
-        * Start our first transaction of the day.
-        *
-        * All future transactions during this code must be "chained" off
-        * this one via the trans_dup() call.  All transactions will contain
-        * the inode, and the inode will always be marked with trans_ihold().
-        * Since the inode will be locked in all transactions, we must log
-        * the inode in every transaction to let it float upward through
-        * the log.
-        */
-       args.trans = xfs_trans_alloc(mp, XFS_TRANS_ATTR_SET);
+       tres.tr_logres = M_RES(mp)->tr_attrsetm.tr_logres +
+                        M_RES(mp)->tr_attrsetrt.tr_logres * args.total;
+       tres.tr_logcount = XFS_ATTRSET_LOG_COUNT;
+       tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
 
        /*
         * Root fork attributes can use reserved data blocks for this
         * operation if necessary
         */
-
-       if (rsvd)
-               args.trans->t_flags |= XFS_TRANS_RESERVE;
-
-       tres.tr_logres = M_RES(mp)->tr_attrsetm.tr_logres +
-                        M_RES(mp)->tr_attrsetrt.tr_logres * args.total;
-       tres.tr_logcount = XFS_ATTRSET_LOG_COUNT;
-       tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
-       error = xfs_trans_reserve(args.trans, &tres, args.total, 0);
-       if (error) {
-               xfs_trans_cancel(args.trans, 0);
+       error = xfs_trans_alloc(mp, &tres, args.total, 0,
+                       rsvd ? XFS_TRANS_RESERVE : 0, &args.trans);
+       if (error)
                return error;
-       }
-       xfs_ilock(dp, XFS_ILOCK_EXCL);
 
+       xfs_ilock(dp, XFS_ILOCK_EXCL);
        error = xfs_trans_reserve_quota_nblks(args.trans, dp, args.total, 0,
                                rsvd ? XFS_QMOPT_RES_REGBLKS | XFS_QMOPT_FORCE_RES :
                                       XFS_QMOPT_RES_REGBLKS);
        if (error) {
                xfs_iunlock(dp, XFS_ILOCK_EXCL);
-               xfs_trans_cancel(args.trans, XFS_TRANS_RELEASE_LOG_RES);
+               xfs_trans_cancel(args.trans);
                return error;
        }
 
@@ -315,8 +296,7 @@ xfs_attr_set(
                                xfs_trans_ichgtime(args.trans, dp,
                                                        XFS_ICHGTIME_CHG);
                        }
-                       err2 = xfs_trans_commit(args.trans,
-                                                XFS_TRANS_RELEASE_LOG_RES);
+                       err2 = xfs_trans_commit(args.trans);
                        xfs_iunlock(dp, XFS_ILOCK_EXCL);
 
                        return error ? error : err2;
@@ -326,35 +306,31 @@ xfs_attr_set(
                 * It won't fit in the shortform, transform to a leaf block.
                 * GROT: another possible req'mt for a double-split btree op.
                 */
-               xfs_bmap_init(args.flist, args.firstblock);
-               error = xfs_attr_shortform_to_leaf(&args);
-               if (!error) {
-                       error = xfs_bmap_finish(&args.trans, args.flist,
-                                               &committed);
-               }
-               if (error) {
-                       ASSERT(committed);
-                       args.trans = NULL;
-                       xfs_bmap_cancel(&flist);
+               error = xfs_attr_shortform_to_leaf(&args, &leaf_bp);
+               if (error)
                        goto out;
-               }
-
                /*
-                * bmap_finish() may have committed the last trans and started
-                * a new one.  We need the inode to be in all transactions.
+                * Prevent the leaf buffer from being unlocked so that a
+                * concurrent AIL push cannot grab the half-baked leaf
+                * buffer and run into problems with the write verifier.
                 */
-               if (committed)
-                       xfs_trans_ijoin(args.trans, dp, 0);
+               xfs_trans_bhold(args.trans, leaf_bp);
+               xfs_defer_bjoin(args.trans->t_dfops, leaf_bp);
+               xfs_defer_ijoin(args.trans->t_dfops, dp);
+               error = xfs_defer_finish(&args.trans, args.trans->t_dfops);
+               if (error)
+                       goto out;
 
                /*
                 * Commit the leaf transformation.  We'll need another (linked)
-                * transaction to add the new attribute to the leaf.
+                * transaction to add the new attribute to the leaf, which
+                * means that we have to hold & join the leaf buffer here too.
                 */
-
-               error = xfs_trans_roll(&args.trans, dp);
+               error = xfs_trans_roll_inode(&args.trans, dp);
                if (error)
                        goto out;
-
+               xfs_trans_bjoin(args.trans, leaf_bp);
+               leaf_bp = NULL;
        }
 
        if (xfs_bmap_one_block(dp, XFS_ATTR_FORK))
@@ -378,16 +354,16 @@ xfs_attr_set(
         * Commit the last in the sequence of transactions.
         */
        xfs_trans_log_inode(args.trans, dp, XFS_ILOG_CORE);
-       error = xfs_trans_commit(args.trans, XFS_TRANS_RELEASE_LOG_RES);
+       error = xfs_trans_commit(args.trans);
        xfs_iunlock(dp, XFS_ILOCK_EXCL);
 
        return error;
 
 out:
-       if (args.trans) {
-               xfs_trans_cancel(args.trans,
-                       XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_ABORT);
-       }
+       if (leaf_bp)
+               xfs_trans_brelse(args.trans, leaf_bp);
+       if (args.trans)
+               xfs_trans_cancel(args.trans);
        xfs_iunlock(dp, XFS_ILOCK_EXCL);
        return error;
 }
@@ -404,25 +380,17 @@ xfs_attr_remove(
 {
        struct xfs_mount        *mp = dp->i_mount;
        struct xfs_da_args      args;
-       struct xfs_bmap_free    flist;
-       xfs_fsblock_t           firstblock;
        int                     error;
 
-       XFS_STATS_INC(xs_attr_remove);
+       XFS_STATS_INC(mp, xs_attr_remove);
 
        if (XFS_FORCED_SHUTDOWN(dp->i_mount))
                return -EIO;
 
-       if (!xfs_inode_hasattr(dp))
-               return -ENOATTR;
-
        error = xfs_attr_args_init(&args, dp, name, flags);
        if (error)
                return error;
 
-       args.firstblock = &firstblock;
-       args.flist = &flist;
-
        /*
         * 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
@@ -430,36 +398,20 @@ xfs_attr_remove(
         */
        args.op_flags = XFS_DA_OP_OKNOENT;
 
-       error = xfs_qm_dqattach(dp, 0);
+       error = xfs_qm_dqattach(dp);
        if (error)
                return error;
 
-       /*
-        * Start our first transaction of the day.
-        *
-        * All future transactions during this code must be "chained" off
-        * this one via the trans_dup() call.  All transactions will contain
-        * the inode, and the inode will always be marked with trans_ihold().
-        * Since the inode will be locked in all transactions, we must log
-        * the inode in every transaction to let it float upward through
-        * the log.
-        */
-       args.trans = xfs_trans_alloc(mp, XFS_TRANS_ATTR_RM);
-
        /*
         * Root fork attributes can use reserved data blocks for this
         * operation if necessary
         */
-
-       if (flags & ATTR_ROOT)
-               args.trans->t_flags |= XFS_TRANS_RESERVE;
-
-       error = xfs_trans_reserve(args.trans, &M_RES(mp)->tr_attrrm,
-                                 XFS_ATTRRM_SPACE_RES(mp), 0);
-       if (error) {
-               xfs_trans_cancel(args.trans, 0);
+       error = xfs_trans_alloc(mp, &M_RES(mp)->tr_attrrm,
+                       XFS_ATTRRM_SPACE_RES(mp), 0,
+                       (flags & ATTR_ROOT) ? XFS_TRANS_RESERVE : 0,
+                       &args.trans);
+       if (error)
                return error;
-       }
 
        xfs_ilock(dp, XFS_ILOCK_EXCL);
        /*
@@ -496,16 +448,14 @@ xfs_attr_remove(
         * Commit the last in the sequence of transactions.
         */
        xfs_trans_log_inode(args.trans, dp, XFS_ILOG_CORE);
-       error = xfs_trans_commit(args.trans, XFS_TRANS_RELEASE_LOG_RES);
+       error = xfs_trans_commit(args.trans);
        xfs_iunlock(dp, XFS_ILOCK_EXCL);
 
        return error;
 
 out:
-       if (args.trans) {
-               xfs_trans_cancel(args.trans,
-                       XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_ABORT);
-       }
+       if (args.trans)
+               xfs_trans_cancel(args.trans);
        xfs_iunlock(dp, XFS_ILOCK_EXCL);
        return error;
 }
@@ -532,7 +482,14 @@ xfs_attr_shortform_addname(xfs_da_args_t *args)
                if (args->flags & ATTR_CREATE)
                        return retval;
                retval = xfs_attr_shortform_remove(args);
-               ASSERT(retval == 0);
+               if (retval)
+                       return retval;
+               /*
+                * Since we have removed the old attr, clear ATTR_REPLACE so
+                * that the leaf format add routine won't trip over the attr
+                * not being around.
+                */
+               args->flags &= ~ATTR_REPLACE;
        }
 
        if (args->namelen >= XFS_ATTR_SF_ENTSIZE_MAX ||
@@ -562,11 +519,12 @@ xfs_attr_shortform_addname(xfs_da_args_t *args)
  * if bmap_one_block() says there is only one block (ie: no remote blks).
  */
 STATIC int
-xfs_attr_leaf_addname(xfs_da_args_t *args)
+xfs_attr_leaf_addname(
+       struct xfs_da_args      *args)
 {
-       xfs_inode_t *dp;
-       struct xfs_buf *bp;
-       int retval, error, committed, forkoff;
+       struct xfs_inode        *dp;
+       struct xfs_buf          *bp;
+       int                     retval, error, forkoff;
 
        trace_xfs_attr_leaf_addname(args);
 
@@ -624,31 +582,19 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
                 * Commit that transaction so that the node_addname() call
                 * can manage its own transactions.
                 */
-               xfs_bmap_init(args->flist, args->firstblock);
                error = xfs_attr3_leaf_to_node(args);
-               if (!error) {
-                       error = xfs_bmap_finish(&args->trans, args->flist,
-                                               &committed);
-               }
-               if (error) {
-                       ASSERT(committed);
-                       args->trans = NULL;
-                       xfs_bmap_cancel(args->flist);
-                       return error;
-               }
-
-               /*
-                * bmap_finish() may have committed the last trans and started
-                * a new one.  We need the inode to be in all transactions.
-                */
-               if (committed)
-                       xfs_trans_ijoin(args->trans, dp, 0);
+               if (error)
+                       goto out_defer_cancel;
+               xfs_defer_ijoin(args->trans->t_dfops, dp);
+               error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+               if (error)
+                       goto out_defer_cancel;
 
                /*
                 * Commit the current trans (including the inode) and start
                 * a new one.
                 */
-               error = xfs_trans_roll(&args->trans, dp);
+               error = xfs_trans_roll_inode(&args->trans, dp);
                if (error)
                        return error;
 
@@ -663,7 +609,7 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
         * Commit the transaction that added the attr name so that
         * later routines can manage their own transactions.
         */
-       error = xfs_trans_roll(&args->trans, dp);
+       error = xfs_trans_roll_inode(&args->trans, dp);
        if (error)
                return error;
 
@@ -724,34 +670,20 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
                 * If the result is small enough, shrink it all into the inode.
                 */
                if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
-                       xfs_bmap_init(args->flist, args->firstblock);
                        error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
                        /* bp is gone due to xfs_da_shrink_inode */
-                       if (!error) {
-                               error = xfs_bmap_finish(&args->trans,
-                                                       args->flist,
-                                                       &committed);
-                       }
-                       if (error) {
-                               ASSERT(committed);
-                               args->trans = NULL;
-                               xfs_bmap_cancel(args->flist);
-                               return error;
-                       }
-
-                       /*
-                        * bmap_finish() may have committed the last trans
-                        * and started a new one.  We need the inode to be
-                        * in all transactions.
-                        */
-                       if (committed)
-                               xfs_trans_ijoin(args->trans, dp, 0);
+                       if (error)
+                               goto out_defer_cancel;
+                       xfs_defer_ijoin(args->trans->t_dfops, dp);
+                       error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+                       if (error)
+                               goto out_defer_cancel;
                }
 
                /*
                 * Commit the remove and start the next trans in series.
                 */
-               error = xfs_trans_roll(&args->trans, dp);
+               error = xfs_trans_roll_inode(&args->trans, dp);
 
        } else if (args->rmtblkno > 0) {
                /*
@@ -760,6 +692,9 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
                error = xfs_attr3_leaf_clearflag(args);
        }
        return error;
+out_defer_cancel:
+       xfs_defer_cancel(args->trans->t_dfops);
+       return error;
 }
 
 /*
@@ -769,11 +704,12 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
  * if bmap_one_block() says there is only one block (ie: no remote blks).
  */
 STATIC int
-xfs_attr_leaf_removename(xfs_da_args_t *args)
+xfs_attr_leaf_removename(
+       struct xfs_da_args      *args)
 {
-       xfs_inode_t *dp;
-       struct xfs_buf *bp;
-       int error, committed, forkoff;
+       struct xfs_inode        *dp;
+       struct xfs_buf          *bp;
+       int                     error, forkoff;
 
        trace_xfs_attr_leaf_removename(args);
 
@@ -798,28 +734,19 @@ xfs_attr_leaf_removename(xfs_da_args_t *args)
         * If the result is small enough, shrink it all into the inode.
         */
        if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
-               xfs_bmap_init(args->flist, args->firstblock);
                error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
                /* bp is gone due to xfs_da_shrink_inode */
-               if (!error) {
-                       error = xfs_bmap_finish(&args->trans, args->flist,
-                                               &committed);
-               }
-               if (error) {
-                       ASSERT(committed);
-                       args->trans = NULL;
-                       xfs_bmap_cancel(args->flist);
-                       return error;
-               }
-
-               /*
-                * bmap_finish() may have committed the last trans and started
-                * a new one.  We need the inode to be in all transactions.
-                */
-               if (committed)
-                       xfs_trans_ijoin(args->trans, dp, 0);
+               if (error)
+                       goto out_defer_cancel;
+               xfs_defer_ijoin(args->trans->t_dfops, dp);
+               error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+               if (error)
+                       goto out_defer_cancel;
        }
        return 0;
+out_defer_cancel:
+       xfs_defer_cancel(args->trans->t_dfops);
+       return error;
 }
 
 /*
@@ -869,13 +796,14 @@ xfs_attr_leaf_get(xfs_da_args_t *args)
  * add a whole extra layer of confusion on top of that.
  */
 STATIC int
-xfs_attr_node_addname(xfs_da_args_t *args)
+xfs_attr_node_addname(
+       struct xfs_da_args      *args)
 {
-       xfs_da_state_t *state;
-       xfs_da_state_blk_t *blk;
-       xfs_inode_t *dp;
-       xfs_mount_t *mp;
-       int committed, retval, error;
+       struct xfs_da_state     *state;
+       struct xfs_da_state_blk *blk;
+       struct xfs_inode        *dp;
+       struct xfs_mount        *mp;
+       int                     retval, error;
 
        trace_xfs_attr_node_addname(args);
 
@@ -934,33 +862,20 @@ restart:
                         */
                        xfs_da_state_free(state);
                        state = NULL;
-                       xfs_bmap_init(args->flist, args->firstblock);
                        error = xfs_attr3_leaf_to_node(args);
-                       if (!error) {
-                               error = xfs_bmap_finish(&args->trans,
-                                                       args->flist,
-                                                       &committed);
-                       }
-                       if (error) {
-                               ASSERT(committed);
-                               args->trans = NULL;
-                               xfs_bmap_cancel(args->flist);
-                               goto out;
-                       }
-
-                       /*
-                        * bmap_finish() may have committed the last trans
-                        * and started a new one.  We need the inode to be
-                        * in all transactions.
-                        */
-                       if (committed)
-                               xfs_trans_ijoin(args->trans, dp, 0);
+                       if (error)
+                               goto out_defer_cancel;
+                       xfs_defer_ijoin(args->trans->t_dfops, dp);
+                       error = xfs_defer_finish(&args->trans,
+                                                args->trans->t_dfops);
+                       if (error)
+                               goto out_defer_cancel;
 
                        /*
                         * Commit the node conversion and start the next
                         * trans in the chain.
                         */
-                       error = xfs_trans_roll(&args->trans, dp);
+                       error = xfs_trans_roll_inode(&args->trans, dp);
                        if (error)
                                goto out;
 
@@ -973,25 +888,13 @@ restart:
                 * in the index/blkno/rmtblkno/rmtblkcnt fields and
                 * in the index2/blkno2/rmtblkno2/rmtblkcnt2 fields.
                 */
-               xfs_bmap_init(args->flist, args->firstblock);
                error = xfs_da3_split(state);
-               if (!error) {
-                       error = xfs_bmap_finish(&args->trans, args->flist,
-                                               &committed);
-               }
-               if (error) {
-                       ASSERT(committed);
-                       args->trans = NULL;
-                       xfs_bmap_cancel(args->flist);
-                       goto out;
-               }
-
-               /*
-                * bmap_finish() may have committed the last trans and started
-                * a new one.  We need the inode to be in all transactions.
-                */
-               if (committed)
-                       xfs_trans_ijoin(args->trans, dp, 0);
+               if (error)
+                       goto out_defer_cancel;
+               xfs_defer_ijoin(args->trans->t_dfops, dp);
+               error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+               if (error)
+                       goto out_defer_cancel;
        } else {
                /*
                 * Addition succeeded, update Btree hashvals.
@@ -1010,7 +913,7 @@ restart:
         * Commit the leaf addition or btree split and start the next
         * trans in the chain.
         */
-       error = xfs_trans_roll(&args->trans, dp);
+       error = xfs_trans_roll_inode(&args->trans, dp);
        if (error)
                goto out;
 
@@ -1082,33 +985,19 @@ restart:
                 * Check to see if the tree needs to be collapsed.
                 */
                if (retval && (state->path.active > 1)) {
-                       xfs_bmap_init(args->flist, args->firstblock);
                        error = xfs_da3_join(state);
-                       if (!error) {
-                               error = xfs_bmap_finish(&args->trans,
-                                                       args->flist,
-                                                       &committed);
-                       }
-                       if (error) {
-                               ASSERT(committed);
-                               args->trans = NULL;
-                               xfs_bmap_cancel(args->flist);
-                               goto out;
-                       }
-
-                       /*
-                        * bmap_finish() may have committed the last trans
-                        * and started a new one.  We need the inode to be
-                        * in all transactions.
-                        */
-                       if (committed)
-                               xfs_trans_ijoin(args->trans, dp, 0);
+                       if (error)
+                               goto out_defer_cancel;
+                       xfs_defer_ijoin(args->trans->t_dfops, dp);
+                       error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+                       if (error)
+                               goto out_defer_cancel;
                }
 
                /*
                 * Commit and start the next trans in the chain.
                 */
-               error = xfs_trans_roll(&args->trans, dp);
+               error = xfs_trans_roll_inode(&args->trans, dp);
                if (error)
                        goto out;
 
@@ -1128,6 +1017,9 @@ out:
        if (error)
                return error;
        return retval;
+out_defer_cancel:
+       xfs_defer_cancel(args->trans->t_dfops);
+       goto out;
 }
 
 /*
@@ -1138,13 +1030,14 @@ out:
  * the root node (a special case of an intermediate node).
  */
 STATIC int
-xfs_attr_node_removename(xfs_da_args_t *args)
+xfs_attr_node_removename(
+       struct xfs_da_args      *args)
 {
-       xfs_da_state_t *state;
-       xfs_da_state_blk_t *blk;
-       xfs_inode_t *dp;
-       struct xfs_buf *bp;
-       int retval, error, committed, forkoff;
+       struct xfs_da_state     *state;
+       struct xfs_da_state_blk *blk;
+       struct xfs_inode        *dp;
+       struct xfs_buf          *bp;
+       int                     retval, error, forkoff;
 
        trace_xfs_attr_node_removename(args);
 
@@ -1216,30 +1109,17 @@ xfs_attr_node_removename(xfs_da_args_t *args)
         * Check to see if the tree needs to be collapsed.
         */
        if (retval && (state->path.active > 1)) {
-               xfs_bmap_init(args->flist, args->firstblock);
                error = xfs_da3_join(state);
-               if (!error) {
-                       error = xfs_bmap_finish(&args->trans, args->flist,
-                                               &committed);
-               }
-               if (error) {
-                       ASSERT(committed);
-                       args->trans = NULL;
-                       xfs_bmap_cancel(args->flist);
-                       goto out;
-               }
-
-               /*
-                * bmap_finish() may have committed the last trans and started
-                * a new one.  We need the inode to be in all transactions.
-                */
-               if (committed)
-                       xfs_trans_ijoin(args->trans, dp, 0);
-
+               if (error)
+                       goto out_defer_cancel;
+               xfs_defer_ijoin(args->trans->t_dfops, dp);
+               error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+               if (error)
+                       goto out_defer_cancel;
                /*
                 * Commit the Btree join operation and start a new trans.
                 */
-               error = xfs_trans_roll(&args->trans, dp);
+               error = xfs_trans_roll_inode(&args->trans, dp);
                if (error)
                        goto out;
        }
@@ -1260,28 +1140,14 @@ xfs_attr_node_removename(xfs_da_args_t *args)
                        goto out;
 
                if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
-                       xfs_bmap_init(args->flist, args->firstblock);
                        error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
                        /* bp is gone due to xfs_da_shrink_inode */
-                       if (!error) {
-                               error = xfs_bmap_finish(&args->trans,
-                                                       args->flist,
-                                                       &committed);
-                       }
-                       if (error) {
-                               ASSERT(committed);
-                               args->trans = NULL;
-                               xfs_bmap_cancel(args->flist);
-                               goto out;
-                       }
-
-                       /*
-                        * bmap_finish() may have committed the last trans
-                        * and started a new one.  We need the inode to be
-                        * in all transactions.
-                        */
-                       if (committed)
-                               xfs_trans_ijoin(args->trans, dp, 0);
+                       if (error)
+                               goto out_defer_cancel;
+                       xfs_defer_ijoin(args->trans->t_dfops, dp);
+                       error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+                       if (error)
+                               goto out_defer_cancel;
                } else
                        xfs_trans_brelse(args->trans, bp);
        }
@@ -1290,6 +1156,9 @@ xfs_attr_node_removename(xfs_da_args_t *args)
 out:
        xfs_da_state_free(state);
        return error;
+out_defer_cancel:
+       xfs_defer_cancel(args->trans->t_dfops);
+       goto out;
 }
 
 /*