xfs
[Top] [All Lists]

[PATCH] Introduce xfs_trans_bmap_add_attrfork.

Subject: [PATCH] Introduce xfs_trans_bmap_add_attrfork.
From: Niv Sardi <xaiki@xxxxxxx>
Date: Thu, 22 May 2008 16:18:00 +1000
That takes a transaction and doesn't require everything to be locked
anymore. This doesn't commit the transaction ! so direct callers,
willing to use xfs_trans_roll() should do it themselves.

Change xfs_bmap_add_attrfork to do the initialization/allocation of
the transaction and commit arround xfs_trans_bmap_add_attrfork.

Signed-off-by: Niv Sardi <xaiki@xxxxxxx>
---
 fs/xfs/xfs_bmap.c |  100 ++++++++++++++++++++++++++++++++++------------------
 fs/xfs/xfs_bmap.h |    7 ++++
 2 files changed, 72 insertions(+), 35 deletions(-)

diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
index 1c0a5a5..d4bd6e7 100644
--- a/fs/xfs/xfs_bmap.c
+++ b/fs/xfs/xfs_bmap.c
@@ -3946,16 +3946,16 @@ xfs_bunmap_trace(
  * Must not be in a transaction, ip must not be locked.
  */
 int                                            /* error code */
-xfs_bmap_add_attrfork(
-       xfs_inode_t             *ip,            /* incore inode pointer */
+xfs_trans_bmap_add_attrfork(
+       struct xfs_trans        **tpp,          /* transaction pointer */
+       struct xfs_inode        *ip,            /* incore inode pointer */
        int                     size,           /* space new attribute needs */
        int                     rsvd)           /* xact may use reserved blks */
 {
        xfs_fsblock_t           firstblock;     /* 1st block/ag allocated */
-       xfs_bmap_free_t         flist;          /* freed extent records */
-       xfs_mount_t             *mp;            /* mount structure */
-       xfs_trans_t             *tp;            /* transaction pointer */
-       int                     blks;           /* space reservation */
+       struct xfs_bmap_free    flist;          /* freed extent records */
+       struct xfs_mount        *mp;            /* mount structure */
+       struct xfs_trans        *tp;            /* transaction pointer */
        int                     version = 1;    /* superblock attr version */
        int                     committed;      /* xaction was committed */
        int                     logflags;       /* logging flags */
@@ -3967,24 +3967,8 @@ xfs_bmap_add_attrfork(
 
        mp = ip->i_mount;
        ASSERT(!XFS_NOT_DQATTACHED(mp, ip));
-       tp = xfs_trans_alloc(mp, XFS_TRANS_ADDAFORK);
-       blks = XFS_ADDAFORK_SPACE_RES(mp);
-       if (rsvd)
-               tp->t_flags |= XFS_TRANS_RESERVE;
-       if ((error = xfs_trans_reserve(tp, blks, XFS_ADDAFORK_LOG_RES(mp), 0,
-                       XFS_TRANS_PERM_LOG_RES, XFS_ADDAFORK_LOG_COUNT)))
-               goto error0;
-       xfs_ilock(ip, XFS_ILOCK_EXCL);
-       error = XFS_TRANS_RESERVE_QUOTA_NBLKS(mp, tp, ip, blks, 0, rsvd ?
-                       XFS_QMOPT_RES_REGBLKS | XFS_QMOPT_FORCE_RES :
-                       XFS_QMOPT_RES_REGBLKS);
-       if (error) {
-               xfs_iunlock(ip, XFS_ILOCK_EXCL);
-               xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES);
-               return error;
-       }
-       if (XFS_IFORK_Q(ip))
-               goto error1;
+       ASSERT(*tpp);
+       tp = *tpp;
        if (ip->i_d.di_aformat != XFS_DINODE_FMT_EXTENTS) {
                /*
                 * For inodes coming from pre-6.2 filesystems.
@@ -3992,10 +3976,7 @@ xfs_bmap_add_attrfork(
                ASSERT(ip->i_d.di_aformat == 0);
                ip->i_d.di_aformat = XFS_DINODE_FMT_EXTENTS;
        }
-       ASSERT(ip->i_d.di_anextents == 0);
-       VN_HOLD(XFS_ITOV(ip));
-       xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
-       xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
+
        switch (ip->i_d.di_format) {
        case XFS_DINODE_FMT_DEV:
                ip->i_d.di_forkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
@@ -4015,7 +3996,7 @@ xfs_bmap_add_attrfork(
        default:
                ASSERT(0);
                error = XFS_ERROR(EINVAL);
-               goto error1;
+               goto error0;
        }
        ip->i_df.if_ext_max =
                XFS_IFORK_DSIZE(ip) / (uint)sizeof(xfs_bmbt_rec_t);
@@ -4046,7 +4027,7 @@ xfs_bmap_add_attrfork(
        if (logflags)
                xfs_trans_log_inode(tp, ip, logflags);
        if (error)
-               goto error2;
+               goto error1;
        if (!XFS_SB_VERSION_HASATTR(&mp->m_sb) ||
           (!XFS_SB_VERSION_HASATTR2(&mp->m_sb) && version == 2)) {
                __int64_t sbfields = 0;
@@ -4066,14 +4047,63 @@ xfs_bmap_add_attrfork(
                } else
                        spin_unlock(&mp->m_sb_lock);
        }
-       if ((error = xfs_bmap_finish(&tp, &flist, &committed)))
-               goto error2;
-       error = xfs_trans_commit(tp, XFS_TRANS_PERM_LOG_RES);
+       error = xfs_bmap_finish(tpp, &flist, &committed);
+       if (error)
+               goto error1;
        ASSERT(ip->i_df.if_ext_max ==
               XFS_IFORK_DSIZE(ip) / (uint)sizeof(xfs_bmbt_rec_t));
-       return error;
-error2:
+       return 0;
+error1:
        xfs_bmap_cancel(&flist);
+       ASSERT(ismrlocked(&ip->i_lock, MR_UPDATE));
+error0:
+       xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_ABORT);
+       ASSERT(ip->i_df.if_ext_max ==
+              XFS_IFORK_DSIZE(ip) / (uint)sizeof(xfs_bmbt_rec_t));
+       return error;
+}
+
+int
+xfs_bmap_add_attrfork(
+       struct xfs_inode        *ip,            /* incore inode pointer */
+       int                     size,           /* space new attribute needs */
+       int                     rsvd)           /* xact may use reserved blks */
+{
+       struct xfs_trans        *tp;            /* transaction pointer */
+       struct xfs_mount        *mp;            /* mount structure */
+       int                     blks;           /* space reservation */
+       int                     error;          /* error return value */
+
+       mp = ip->i_mount;
+       tp = xfs_trans_alloc(mp, XFS_TRANS_ADDAFORK);
+       blks = XFS_ADDAFORK_SPACE_RES(mp);
+
+       if (rsvd)
+               tp->t_flags |= XFS_TRANS_RESERVE;
+       error = xfs_trans_reserve(tp, blks, XFS_ADDAFORK_LOG_RES(mp), 0,
+                               XFS_TRANS_PERM_LOG_RES, XFS_ADDAFORK_LOG_COUNT);
+       if (error)
+               goto error0;
+
+       xfs_ilock(ip, XFS_ILOCK_EXCL);
+       error = XFS_TRANS_RESERVE_QUOTA_NBLKS(mp, tp, ip, blks, 0, rsvd ?
+                             XFS_QMOPT_RES_REGBLKS | XFS_QMOPT_FORCE_RES :
+                             XFS_QMOPT_RES_REGBLKS);
+       if (error)
+               goto error1;
+
+       if (XFS_IFORK_Q(ip))
+               goto error1;
+
+       ASSERT(ip->i_d.di_anextents == 0);
+       VN_HOLD(XFS_ITOV(ip));
+       xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
+       xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
+
+       error = xfs_trans_bmap_add_attrfork(NULL, ip, size, rsvd);
+       if (error)
+               return error;
+       return xfs_trans_commit(tp, XFS_TRANS_PERM_LOG_RES);
 error1:
        ASSERT(ismrlocked(&ip->i_lock,MR_UPDATE));
        xfs_iunlock(ip, XFS_ILOCK_EXCL);
diff --git a/fs/xfs/xfs_bmap.h b/fs/xfs/xfs_bmap.h
index 87224b7..7a21e41 100644
--- a/fs/xfs/xfs_bmap.h
+++ b/fs/xfs/xfs_bmap.h
@@ -166,6 +166,13 @@ xfs_bmap_add_attrfork(
        int                     size,   /* space needed for new attribute */
        int                     rsvd);  /* flag for reserved block allocation */
 
+int                                    /* error code */
+xfs_trans_bmap_add_attrfork(
+       struct xfs_trans        **tpp,   /* transaction */
+       struct xfs_inode        *ip,    /* incore inode pointer */
+       int                     size,   /* space needed for new attribute */
+       int                     rsvd);  /* flag for reserved block allocation */
+
 /*
  * Add the extent to the list of extents to be free at transaction end.
  * The list is maintained sorted (by block number).
-- 
1.5.6


--=-=-=


Thanks for this extensive review =)
-- 
Niv Sardi

--=-=-=--


<Prev in Thread] Current Thread [Next in Thread>
  • [PATCH] Introduce xfs_trans_bmap_add_attrfork., Niv Sardi <=