]> git.ipfire.org Git - thirdparty/xfsprogs-dev.git/blobdiff - libxfs/xfs_attr_remote.c
xfs: use ->t_firstblock in xattr ops
[thirdparty/xfsprogs-dev.git] / libxfs / xfs_attr_remote.c
index f0ca926ee558fe17f53afcc672778ef1940d81ef..1d39ace1dde6a9be94a8051d843d959235733687 100644 (file)
@@ -1,22 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (c) 2000-2005 Silicon Graphics, Inc.
  * Copyright (c) 2013 Red Hat, 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 <xfs.h>
+#include "libxfs_priv.h"
+#include "xfs_fs.h"
+#include "xfs_shared.h"
+#include "xfs_format.h"
+#include "xfs_log_format.h"
+#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_inode.h"
+#include "xfs_alloc.h"
+#include "xfs_trans.h"
+#include "xfs_bmap.h"
+#include "xfs_attr_leaf.h"
+#include "xfs_attr_remote.h"
+#include "xfs_trans_space.h"
+#include "xfs_trace.h"
+#include "xfs_cksum.h"
 
 #define ATTR_RMTVALUE_MAPSIZE  1       /* # of map entries at once */
 
  * Each contiguous block has a header, so it is not just a simple attribute
  * length to FSB conversion.
  */
-static int
+int
 xfs_attr3_rmt_blocks(
        struct xfs_mount *mp,
        int             attrlen)
 {
-       int             buflen = XFS_ATTR3_RMT_BUF_SPACE(mp,
-                                                        mp->m_sb.sb_blocksize);
-       return (attrlen + buflen - 1) / buflen;
+       if (xfs_sb_version_hascrc(&mp->m_sb)) {
+               int buflen = XFS_ATTR3_RMT_BUF_SPACE(mp, mp->m_sb.sb_blocksize);
+               return (attrlen + buflen - 1) / buflen;
+       }
+       return XFS_B_TO_FSB(mp, attrlen);
+}
+
+/*
+ * Checking of the remote attribute header is split into two parts. The verifier
+ * does CRC, location and bounds checking, the unpacking function checks the
+ * attribute parameters and owner.
+ */
+static xfs_failaddr_t
+xfs_attr3_rmt_hdr_ok(
+       void                    *ptr,
+       xfs_ino_t               ino,
+       uint32_t                offset,
+       uint32_t                size,
+       xfs_daddr_t             bno)
+{
+       struct xfs_attr3_rmt_hdr *rmt = ptr;
+
+       if (bno != be64_to_cpu(rmt->rm_blkno))
+               return __this_address;
+       if (offset != be32_to_cpu(rmt->rm_offset))
+               return __this_address;
+       if (size != be32_to_cpu(rmt->rm_bytes))
+               return __this_address;
+       if (ino != be64_to_cpu(rmt->rm_owner))
+               return __this_address;
+
+       /* ok */
+       return NULL;
 }
 
-static bool
+static xfs_failaddr_t
 xfs_attr3_rmt_verify(
-       struct xfs_buf          *bp)
+       struct xfs_mount        *mp,
+       void                    *ptr,
+       int                     fsbsize,
+       xfs_daddr_t             bno)
 {
-       struct xfs_mount        *mp = bp->b_target->bt_mount;
-       struct xfs_attr3_rmt_hdr *rmt = bp->b_addr;
+       struct xfs_attr3_rmt_hdr *rmt = ptr;
 
        if (!xfs_sb_version_hascrc(&mp->m_sb))
-               return false;
+               return __this_address;
        if (rmt->rm_magic != cpu_to_be32(XFS_ATTR3_RMT_MAGIC))
-               return false;
-       if (!uuid_equal(&rmt->rm_uuid, &mp->m_sb.sb_uuid))
-               return false;
-       if (bp->b_bn != be64_to_cpu(rmt->rm_blkno))
-               return false;
+               return __this_address;
+       if (!uuid_equal(&rmt->rm_uuid, &mp->m_sb.sb_meta_uuid))
+               return __this_address;
+       if (be64_to_cpu(rmt->rm_blkno) != bno)
+               return __this_address;
+       if (be32_to_cpu(rmt->rm_bytes) > fsbsize - sizeof(*rmt))
+               return __this_address;
        if (be32_to_cpu(rmt->rm_offset) +
-                               be32_to_cpu(rmt->rm_bytes) >XATTR_SIZE_MAX)
-               return false;
+                               be32_to_cpu(rmt->rm_bytes) > XFS_XATTR_SIZE_MAX)
+               return __this_address;
        if (rmt->rm_owner == 0)
-               return false;
+               return __this_address;
 
-       return true;
+       return NULL;
 }
 
-static void
-xfs_attr3_rmt_read_verify(
-       struct xfs_buf  *bp)
+static int
+__xfs_attr3_rmt_read_verify(
+       struct xfs_buf  *bp,
+       bool            check_crc,
+       xfs_failaddr_t  *failaddr)
 {
        struct xfs_mount *mp = bp->b_target->bt_mount;
+       char            *ptr;
+       int             len;
+       xfs_daddr_t     bno;
+       int             blksize = mp->m_attr_geo->blksize;
 
        /* no verification of non-crc buffers */
        if (!xfs_sb_version_hascrc(&mp->m_sb))
-               return;
+               return 0;
+
+       ptr = bp->b_addr;
+       bno = bp->b_bn;
+       len = BBTOB(bp->b_length);
+       ASSERT(len >= blksize);
 
-       if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-                             XFS_ATTR3_RMT_CRC_OFF) ||
-           !xfs_attr3_rmt_verify(bp)) {
-               XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
-               xfs_buf_ioerror(bp, EFSCORRUPTED);
+       while (len > 0) {
+               if (check_crc &&
+                   !xfs_verify_cksum(ptr, blksize, XFS_ATTR3_RMT_CRC_OFF)) {
+                       *failaddr = __this_address;
+                       return -EFSBADCRC;
+               }
+               *failaddr = xfs_attr3_rmt_verify(mp, ptr, blksize, bno);
+               if (*failaddr)
+                       return -EFSCORRUPTED;
+               len -= blksize;
+               ptr += blksize;
+               bno += BTOBB(blksize);
        }
+
+       if (len != 0) {
+               *failaddr = __this_address;
+               return -EFSCORRUPTED;
+       }
+
+       return 0;
+}
+
+static void
+xfs_attr3_rmt_read_verify(
+       struct xfs_buf  *bp)
+{
+       xfs_failaddr_t  fa;
+       int             error;
+
+       error = __xfs_attr3_rmt_read_verify(bp, true, &fa);
+       if (error)
+               xfs_verifier_error(bp, error, fa);
+}
+
+static xfs_failaddr_t
+xfs_attr3_rmt_verify_struct(
+       struct xfs_buf  *bp)
+{
+       xfs_failaddr_t  fa;
+       int             error;
+
+       error = __xfs_attr3_rmt_read_verify(bp, false, &fa);
+       return error ? fa : NULL;
 }
 
 static void
@@ -81,40 +170,66 @@ xfs_attr3_rmt_write_verify(
        struct xfs_buf  *bp)
 {
        struct xfs_mount *mp = bp->b_target->bt_mount;
-       struct xfs_buf_log_item *bip = bp->b_fspriv;
+       xfs_failaddr_t  fa;
+       int             blksize = mp->m_attr_geo->blksize;
+       char            *ptr;
+       int             len;
+       xfs_daddr_t     bno;
 
        /* no verification of non-crc buffers */
        if (!xfs_sb_version_hascrc(&mp->m_sb))
                return;
 
-       if (!xfs_attr3_rmt_verify(bp)) {
-               XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
-               xfs_buf_ioerror(bp, EFSCORRUPTED);
-               return;
-       }
+       ptr = bp->b_addr;
+       bno = bp->b_bn;
+       len = BBTOB(bp->b_length);
+       ASSERT(len >= blksize);
+
+       while (len > 0) {
+               struct xfs_attr3_rmt_hdr *rmt = (struct xfs_attr3_rmt_hdr *)ptr;
 
-       if (bip) {
-               struct xfs_attr3_rmt_hdr *rmt = bp->b_addr;
-               rmt->rm_lsn = cpu_to_be64(bip->bli_item.li_lsn);
+               fa = xfs_attr3_rmt_verify(mp, ptr, blksize, bno);
+               if (fa) {
+                       xfs_verifier_error(bp, -EFSCORRUPTED, fa);
+                       return;
+               }
+
+               /*
+                * Ensure we aren't writing bogus LSNs to disk. See
+                * xfs_attr3_rmt_hdr_set() for the explanation.
+                */
+               if (rmt->rm_lsn != cpu_to_be64(NULLCOMMITLSN)) {
+                       xfs_verifier_error(bp, -EFSCORRUPTED, __this_address);
+                       return;
+               }
+               xfs_update_cksum(ptr, blksize, XFS_ATTR3_RMT_CRC_OFF);
+
+               len -= blksize;
+               ptr += blksize;
+               bno += BTOBB(blksize);
        }
-       xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
-                        XFS_ATTR3_RMT_CRC_OFF);
+
+       if (len != 0)
+               xfs_verifier_error(bp, -EFSCORRUPTED, __this_address);
 }
 
 const struct xfs_buf_ops xfs_attr3_rmt_buf_ops = {
+       .name = "xfs_attr3_rmt",
        .verify_read = xfs_attr3_rmt_read_verify,
        .verify_write = xfs_attr3_rmt_write_verify,
+       .verify_struct = xfs_attr3_rmt_verify_struct,
 };
 
-static int
+STATIC int
 xfs_attr3_rmt_hdr_set(
        struct xfs_mount        *mp,
+       void                    *ptr,
        xfs_ino_t               ino,
        uint32_t                offset,
        uint32_t                size,
-       struct xfs_buf          *bp)
+       xfs_daddr_t             bno)
 {
-       struct xfs_attr3_rmt_hdr *rmt = bp->b_addr;
+       struct xfs_attr3_rmt_hdr *rmt = ptr;
 
        if (!xfs_sb_version_hascrc(&mp->m_sb))
                return 0;
@@ -122,38 +237,123 @@ xfs_attr3_rmt_hdr_set(
        rmt->rm_magic = cpu_to_be32(XFS_ATTR3_RMT_MAGIC);
        rmt->rm_offset = cpu_to_be32(offset);
        rmt->rm_bytes = cpu_to_be32(size);
-       uuid_copy(&rmt->rm_uuid, &mp->m_sb.sb_uuid);
+       uuid_copy(&rmt->rm_uuid, &mp->m_sb.sb_meta_uuid);
        rmt->rm_owner = cpu_to_be64(ino);
-       rmt->rm_blkno = cpu_to_be64(bp->b_bn);
-       bp->b_ops = &xfs_attr3_rmt_buf_ops;
+       rmt->rm_blkno = cpu_to_be64(bno);
+
+       /*
+        * Remote attribute blocks are written synchronously, so we don't
+        * have an LSN that we can stamp in them that makes any sense to log
+        * recovery. To ensure that log recovery handles overwrites of these
+        * blocks sanely (i.e. once they've been freed and reallocated as some
+        * other type of metadata) we need to ensure that the LSN has a value
+        * that tells log recovery to ignore the LSN and overwrite the buffer
+        * with whatever is in it's log. To do this, we use the magic
+        * NULLCOMMITLSN to indicate that the LSN is invalid.
+        */
+       rmt->rm_lsn = cpu_to_be64(NULLCOMMITLSN);
 
        return sizeof(struct xfs_attr3_rmt_hdr);
 }
 
 /*
- * Checking of the remote attribute header is split into two parts. the verifier
- * does CRC, location and bounds checking, the unpacking function checks the
- * attribute parameters and owner.
+ * Helper functions to copy attribute data in and out of the one disk extents
  */
-static bool
-xfs_attr3_rmt_hdr_ok(
-       struct xfs_mount        *mp,
-       xfs_ino_t               ino,
-       uint32_t                offset,
-       uint32_t                size,
-       struct xfs_buf          *bp)
+STATIC int
+xfs_attr_rmtval_copyout(
+       struct xfs_mount *mp,
+       struct xfs_buf  *bp,
+       xfs_ino_t       ino,
+       int             *offset,
+       int             *valuelen,
+       uint8_t         **dst)
 {
-       struct xfs_attr3_rmt_hdr *rmt = bp->b_addr;
+       char            *src = bp->b_addr;
+       xfs_daddr_t     bno = bp->b_bn;
+       int             len = BBTOB(bp->b_length);
+       int             blksize = mp->m_attr_geo->blksize;
+
+       ASSERT(len >= blksize);
+
+       while (len > 0 && *valuelen > 0) {
+               int hdr_size = 0;
+               int byte_cnt = XFS_ATTR3_RMT_BUF_SPACE(mp, blksize);
+
+               byte_cnt = min(*valuelen, byte_cnt);
+
+               if (xfs_sb_version_hascrc(&mp->m_sb)) {
+                       if (xfs_attr3_rmt_hdr_ok(src, ino, *offset,
+                                                 byte_cnt, bno)) {
+                               xfs_alert(mp,
+"remote attribute header mismatch bno/off/len/owner (0x%llx/0x%x/Ox%x/0x%llx)",
+                                       bno, *offset, byte_cnt, ino);
+                               return -EFSCORRUPTED;
+                       }
+                       hdr_size = sizeof(struct xfs_attr3_rmt_hdr);
+               }
 
-       if (offset != be32_to_cpu(rmt->rm_offset))
-               return false;
-       if (size != be32_to_cpu(rmt->rm_bytes))
-               return false;
-       if (ino != be64_to_cpu(rmt->rm_owner))
-               return false;
+               memcpy(*dst, src + hdr_size, byte_cnt);
 
-       /* ok */
-       return true;
+               /* roll buffer forwards */
+               len -= blksize;
+               src += blksize;
+               bno += BTOBB(blksize);
+
+               /* roll attribute data forwards */
+               *valuelen -= byte_cnt;
+               *dst += byte_cnt;
+               *offset += byte_cnt;
+       }
+       return 0;
+}
+
+STATIC void
+xfs_attr_rmtval_copyin(
+       struct xfs_mount *mp,
+       struct xfs_buf  *bp,
+       xfs_ino_t       ino,
+       int             *offset,
+       int             *valuelen,
+       uint8_t         **src)
+{
+       char            *dst = bp->b_addr;
+       xfs_daddr_t     bno = bp->b_bn;
+       int             len = BBTOB(bp->b_length);
+       int             blksize = mp->m_attr_geo->blksize;
+
+       ASSERT(len >= blksize);
+
+       while (len > 0 && *valuelen > 0) {
+               int hdr_size;
+               int byte_cnt = XFS_ATTR3_RMT_BUF_SPACE(mp, blksize);
+
+               byte_cnt = min(*valuelen, byte_cnt);
+               hdr_size = xfs_attr3_rmt_hdr_set(mp, dst, ino, *offset,
+                                                byte_cnt, bno);
+
+               memcpy(dst + hdr_size, *src, byte_cnt);
+
+               /*
+                * If this is the last block, zero the remainder of it.
+                * Check that we are actually the last block, too.
+                */
+               if (byte_cnt + hdr_size < blksize) {
+                       ASSERT(*valuelen - byte_cnt == 0);
+                       ASSERT(len == blksize);
+                       memset(dst + hdr_size + byte_cnt, 0,
+                                       blksize - hdr_size - byte_cnt);
+               }
+
+               /* roll buffer forwards */
+               len -= blksize;
+               dst += blksize;
+               bno += BTOBB(blksize);
+
+               /* roll attribute data forwards */
+               *valuelen -= byte_cnt;
+               *src += byte_cnt;
+               *offset += byte_cnt;
+       }
 }
 
 /*
@@ -167,69 +367,55 @@ xfs_attr_rmtval_get(
        struct xfs_bmbt_irec    map[ATTR_RMTVALUE_MAPSIZE];
        struct xfs_mount        *mp = args->dp->i_mount;
        struct xfs_buf          *bp;
-       xfs_daddr_t             dblkno;
        xfs_dablk_t             lblkno = args->rmtblkno;
-       void                    *dst = args->value;
-       int                     valuelen = args->valuelen;
+       uint8_t                 *dst = args->value;
+       int                     valuelen;
        int                     nmap;
        int                     error;
-       int                     blkcnt;
+       int                     blkcnt = args->rmtblkcnt;
        int                     i;
        int                     offset = 0;
 
        trace_xfs_attr_rmtval_get(args);
 
        ASSERT(!(args->flags & ATTR_KERNOVAL));
+       ASSERT(args->rmtvaluelen == args->valuelen);
 
+       valuelen = args->rmtvaluelen;
        while (valuelen > 0) {
                nmap = ATTR_RMTVALUE_MAPSIZE;
                error = xfs_bmapi_read(args->dp, (xfs_fileoff_t)lblkno,
-                                      args->rmtblkcnt, map, &nmap,
+                                      blkcnt, map, &nmap,
                                       XFS_BMAPI_ATTRFORK);
                if (error)
                        return error;
                ASSERT(nmap >= 1);
 
                for (i = 0; (i < nmap) && (valuelen > 0); i++) {
-                       int     byte_cnt;
-                       char    *src;
+                       xfs_daddr_t     dblkno;
+                       int             dblkcnt;
 
                        ASSERT((map[i].br_startblock != DELAYSTARTBLOCK) &&
                               (map[i].br_startblock != HOLESTARTBLOCK));
                        dblkno = XFS_FSB_TO_DADDR(mp, map[i].br_startblock);
-                       blkcnt = XFS_FSB_TO_BB(mp, map[i].br_blockcount);
-                       error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp,
-                                                  dblkno, blkcnt, 0, &bp,
+                       dblkcnt = XFS_FSB_TO_BB(mp, map[i].br_blockcount);
+                       error = xfs_trans_read_buf(mp, args->trans,
+                                                  mp->m_ddev_targp,
+                                                  dblkno, dblkcnt, 0, &bp,
                                                   &xfs_attr3_rmt_buf_ops);
                        if (error)
                                return error;
 
-                       byte_cnt = min_t(int, valuelen, BBTOB(bp->b_length));
-                       byte_cnt = XFS_ATTR3_RMT_BUF_SPACE(mp, byte_cnt);
-
-                       src = bp->b_addr;
-                       if (xfs_sb_version_hascrc(&mp->m_sb)) {
-                               if (!xfs_attr3_rmt_hdr_ok(mp, args->dp->i_ino,
-                                                       offset, byte_cnt, bp)) {
-                                       xfs_alert(mp,
-"remote attribute header does not match required off/len/owner (0x%x/Ox%x,0x%llx)",
-                                               offset, byte_cnt, args->dp->i_ino);
-                                       xfs_buf_relse(bp);
-                                       return EFSCORRUPTED;
-
-                               }
-
-                               src += sizeof(struct xfs_attr3_rmt_hdr);
-                       }
-
-                       memcpy(dst, src, byte_cnt);
-                       xfs_buf_relse(bp);
-
-                       offset += byte_cnt;
-                       dst += byte_cnt;
-                       valuelen -= byte_cnt;
+                       error = xfs_attr_rmtval_copyout(mp, bp, args->dp->i_ino,
+                                                       &offset, &valuelen,
+                                                       &dst);
+                       xfs_trans_brelse(args->trans, bp);
+                       if (error)
+                               return error;
 
+                       /* roll attribute extent map forwards */
                        lblkno += map[i].br_blockcount;
+                       blkcnt -= map[i].br_blockcount;
                }
        }
        ASSERT(valuelen == 0);
@@ -247,17 +433,13 @@ xfs_attr_rmtval_set(
        struct xfs_inode        *dp = args->dp;
        struct xfs_mount        *mp = dp->i_mount;
        struct xfs_bmbt_irec    map;
-       struct xfs_buf          *bp;
-       xfs_daddr_t             dblkno;
        xfs_dablk_t             lblkno;
        xfs_fileoff_t           lfileoff = 0;
-       void                    *src = args->value;
+       uint8_t                 *src = args->value;
        int                     blkcnt;
        int                     valuelen;
        int                     nmap;
        int                     error;
-       int                     hdrcnt = 0;
-       bool                    crcs = xfs_sb_version_hascrc(&mp->m_sb);
        int                     offset = 0;
 
        trace_xfs_attr_rmtval_set(args);
@@ -266,24 +448,14 @@ xfs_attr_rmtval_set(
         * Find a "hole" in the attribute address space large enough for
         * us to drop the new attribute's value into. Because CRC enable
         * attributes have headers, we can't just do a straight byte to FSB
-        * conversion. We calculate the worst case block count in this case
-        * and we may not need that many, so we have to handle this when
-        * allocating the blocks below. 
+        * conversion and have to take the header space into account.
         */
-       if (!crcs)
-               blkcnt = XFS_B_TO_FSB(mp, args->valuelen);
-       else
-               blkcnt = xfs_attr3_rmt_blocks(mp, args->valuelen);
-
+       blkcnt = xfs_attr3_rmt_blocks(mp, args->rmtvaluelen);
        error = xfs_bmap_first_unused(args->trans, args->dp, blkcnt, &lfileoff,
                                                   XFS_ATTR_FORK);
        if (error)
                return error;
 
-       /* Start with the attribute data. We'll allocate the rest afterwards. */
-       if (crcs)
-               blkcnt = XFS_B_TO_FSB(mp, args->valuelen);
-
        args->rmtblkno = lblkno = (xfs_dablk_t)lfileoff;
        args->rmtblkcnt = blkcnt;
 
@@ -291,68 +463,44 @@ xfs_attr_rmtval_set(
         * Roll through the "value", allocating blocks on disk as required.
         */
        while (blkcnt > 0) {
-               int     committed;
-
                /*
                 * Allocate a single extent, up to the size of the value.
+                *
+                * Note that we have to consider this a data allocation as we
+                * write the remote attribute without logging the contents.
+                * Hence we must ensure that we aren't using blocks that are on
+                * the busy list so that we don't overwrite blocks which have
+                * recently been freed but their transactions are not yet
+                * committed to disk. If we overwrite the contents of a busy
+                * extent and then crash then the block may not contain the
+                * correct metadata after log recovery occurs.
                 */
-               xfs_bmap_init(args->flist, 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 | XFS_BMAPI_METADATA,
-                                 args->firstblock, args->total, &map, &nmap,
-                                 args->flist);
-               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);
+                                 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);
+               error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+               if (error)
+                       goto out_defer_cancel;
 
                ASSERT(nmap == 1);
                ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
                       (map.br_startblock != HOLESTARTBLOCK));
                lblkno += map.br_blockcount;
                blkcnt -= map.br_blockcount;
-               hdrcnt++;
-
-               /*
-                * If we have enough blocks for the attribute data, calculate
-                * how many extra blocks we need for headers. We might run
-                * through this multiple times in the case that the additional
-                * headers in the blocks needed for the data fragments spills
-                * into requiring more blocks. e.g. for 512 byte blocks, we'll
-                * spill for another block every 9 headers we require in this
-                * loop.
-                */
-               if (crcs && blkcnt == 0) {
-                       int total_len;
-
-                       total_len = args->valuelen +
-                                   hdrcnt * sizeof(struct xfs_attr3_rmt_hdr);
-                       blkcnt = XFS_B_TO_FSB(mp, total_len);
-                       blkcnt -= args->rmtblkcnt;
-                       args->rmtblkcnt += blkcnt;
-               }
 
                /*
                 * Start the next trans in the chain.
                 */
-               error = xfs_trans_roll(&args->trans, dp);
+               error = xfs_trans_roll_inode(&args->trans, dp);
                if (error)
-                       return (error);
+                       return error;
        }
 
        /*
@@ -362,62 +510,54 @@ xfs_attr_rmtval_set(
         * the INCOMPLETE flag.
         */
        lblkno = args->rmtblkno;
-       valuelen = args->valuelen;
+       blkcnt = args->rmtblkcnt;
+       valuelen = args->rmtvaluelen;
        while (valuelen > 0) {
-               int     byte_cnt;
-               char    *buf;
+               struct xfs_buf  *bp;
+               xfs_daddr_t     dblkno;
+               int             dblkcnt;
 
-               /*
-                * Try to remember where we decided to put the value.
-                */
-               xfs_bmap_init(args->flist, args->firstblock);
+               ASSERT(blkcnt > 0);
+
+               xfs_defer_init(args->trans, args->trans->t_dfops,
+                              &args->trans->t_firstblock);
                nmap = 1;
                error = xfs_bmapi_read(dp, (xfs_fileoff_t)lblkno,
-                                      args->rmtblkcnt, &map, &nmap,
+                                      blkcnt, &map, &nmap,
                                       XFS_BMAPI_ATTRFORK);
                if (error)
-                       return(error);
+                       return error;
                ASSERT(nmap == 1);
                ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
                       (map.br_startblock != HOLESTARTBLOCK));
 
                dblkno = XFS_FSB_TO_DADDR(mp, map.br_startblock),
-               blkcnt = XFS_FSB_TO_BB(mp, map.br_blockcount);
+               dblkcnt = XFS_FSB_TO_BB(mp, map.br_blockcount);
 
-               bp = xfs_buf_get(mp->m_ddev_targp, dblkno, blkcnt, 0);
+               bp = xfs_buf_get(mp->m_ddev_targp, dblkno, dblkcnt, 0);
                if (!bp)
-                       return ENOMEM;
+                       return -ENOMEM;
                bp->b_ops = &xfs_attr3_rmt_buf_ops;
 
-               byte_cnt = BBTOB(bp->b_length);
-               byte_cnt = XFS_ATTR3_RMT_BUF_SPACE(mp, byte_cnt);
-               if (valuelen < byte_cnt)
-                       byte_cnt = valuelen;
-
-               buf = bp->b_addr;
-               buf += xfs_attr3_rmt_hdr_set(mp, dp->i_ino, offset,
-                                            byte_cnt, bp);
-               memcpy(buf, src, byte_cnt);
-
-               if (byte_cnt < BBTOB(bp->b_length))
-                       xfs_buf_zero(bp, byte_cnt,
-                                    BBTOB(bp->b_length) - byte_cnt);
+               xfs_attr_rmtval_copyin(mp, bp, args->dp->i_ino, &offset,
+                                      &valuelen, &src);
 
                error = xfs_bwrite(bp); /* GROT: NOTE: synchronous write */
                xfs_buf_relse(bp);
                if (error)
                        return error;
 
-               src += byte_cnt;
-               valuelen -= byte_cnt;
-               offset += byte_cnt;
-               hdrcnt--;
 
+               /* roll attribute extent map forwards */
                lblkno += map.br_blockcount;
+               blkcnt -= map.br_blockcount;
        }
        ASSERT(valuelen == 0);
-       ASSERT(hdrcnt == 0);
        return 0;
+out_defer_cancel:
+       xfs_defer_cancel(args->trans->t_dfops);
+       args->trans = NULL;
+       return error;
 }
 
 /*
@@ -425,55 +565,56 @@ xfs_attr_rmtval_set(
  * out-of-line buffer that it is stored on.
  */
 int
-xfs_attr_rmtval_remove(xfs_da_args_t *args)
+xfs_attr_rmtval_remove(
+       struct xfs_da_args      *args)
 {
-       xfs_mount_t *mp;
-       xfs_bmbt_irec_t map;
-       xfs_buf_t *bp;
-       xfs_daddr_t dblkno;
-       xfs_dablk_t lblkno;
-       int valuelen, blkcnt, nmap, error, done, committed;
+       struct xfs_mount        *mp = args->dp->i_mount;
+       xfs_dablk_t             lblkno;
+       int                     blkcnt;
+       int                     error;
+       int                     done;
 
        trace_xfs_attr_rmtval_remove(args);
 
-       mp = args->dp->i_mount;
-
        /*
-        * Roll through the "value", invalidating the attribute value's
-        * blocks.
+        * Roll through the "value", invalidating the attribute value's blocks.
         */
        lblkno = args->rmtblkno;
-       valuelen = args->rmtblkcnt;
-       while (valuelen > 0) {
+       blkcnt = args->rmtblkcnt;
+       while (blkcnt > 0) {
+               struct xfs_bmbt_irec    map;
+               struct xfs_buf          *bp;
+               xfs_daddr_t             dblkno;
+               int                     dblkcnt;
+               int                     nmap;
+
                /*
                 * Try to remember where we decided to put the value.
                 */
                nmap = 1;
                error = xfs_bmapi_read(args->dp, (xfs_fileoff_t)lblkno,
-                                      args->rmtblkcnt, &map, &nmap,
-                                      XFS_BMAPI_ATTRFORK);
+                                      blkcnt, &map, &nmap, XFS_BMAPI_ATTRFORK);
                if (error)
-                       return(error);
+                       return error;
                ASSERT(nmap == 1);
                ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
                       (map.br_startblock != HOLESTARTBLOCK));
 
                dblkno = XFS_FSB_TO_DADDR(mp, map.br_startblock),
-               blkcnt = XFS_FSB_TO_BB(mp, map.br_blockcount);
+               dblkcnt = XFS_FSB_TO_BB(mp, map.br_blockcount);
 
                /*
                 * If the "remote" value is in the cache, remove it.
                 */
-               bp = xfs_incore(mp->m_ddev_targp, dblkno, blkcnt, XBF_TRYLOCK);
+               bp = xfs_buf_incore(mp->m_ddev_targp, dblkno, dblkcnt, XBF_TRYLOCK);
                if (bp) {
                        xfs_buf_stale(bp);
                        xfs_buf_relse(bp);
                        bp = NULL;
                }
 
-               valuelen -= map.br_blockcount;
-
                lblkno += map.br_blockcount;
+               blkcnt -= map.br_blockcount;
        }
 
        /*
@@ -483,36 +624,28 @@ xfs_attr_rmtval_remove(xfs_da_args_t *args)
        blkcnt = args->rmtblkcnt;
        done = 0;
        while (!done) {
-               xfs_bmap_init(args->flist, 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 | XFS_BMAPI_METADATA,
-                                   1, args->firstblock, args->flist,
-                                   &done);
-               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, args->dp, 0);
+                                   XFS_BMAPI_ATTRFORK, 1,
+                                   &args->trans->t_firstblock, &done);
+               if (error)
+                       goto out_defer_cancel;
+               xfs_defer_ijoin(args->trans->t_dfops, args->dp);
+               error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+               if (error)
+                       goto out_defer_cancel;
 
                /*
                 * Close out trans and start the next one in the chain.
                 */
-               error = xfs_trans_roll(&args->trans, args->dp);
+               error = xfs_trans_roll_inode(&args->trans, args->dp);
                if (error)
-                       return (error);
+                       return error;
        }
-       return(0);
+       return 0;
+out_defer_cancel:
+       xfs_defer_cancel(args->trans->t_dfops);
+       args->trans = NULL;
+       return error;
 }
-