xfs: push down inactive transaction mgmt for remote symlinks

Push down the transaction management for remote symlinks from
xfs_inactive() down to xfs_inactive_symlink_rmt(). The latter is
cleaned up to avoid transaction management intended for the
calling context (i.e., trans duplication, reservation, item
attachment).

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Ben Myers <bpm@sgi.com>

1 files changed, 40 insertions, 46 deletions

diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index f622a97a7e33..ded282b10bc2 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -424,8 +424,7 @@ xfs_symlink(
  */
 STATIC int
 xfs_inactive_symlink_rmt(
-        xfs_inode_t     *ip,
-        xfs_trans_t     **tpp)
+        struct xfs_inode *ip)
 {
         xfs_buf_t       *bp;
         int             committed;
@@ -437,11 +436,9 @@ xfs_inactive_symlink_rmt(
         xfs_mount_t     *mp;
         xfs_bmbt_irec_t mval[XFS_SYMLINK_MAPS];
         int             nmaps;
-        xfs_trans_t     *ntp;
         int             size;
         xfs_trans_t     *tp;
 
-        tp = *tpp;
         mp = ip->i_mount;
         ASSERT(ip->i_df.if_flags & XFS_IFEXTENTS);
         /*
@@ -453,6 +450,16 @@ xfs_inactive_symlink_rmt(
          */
         ASSERT(ip->i_d.di_nextents > 0 && ip->i_d.di_nextents <= 2);
 
+        tp = xfs_trans_alloc(mp, XFS_TRANS_INACTIVE);
+        error = xfs_trans_reserve(tp, &M_RES(mp)->tr_itruncate, 0, 0);
+        if (error) {
+                xfs_trans_cancel(tp, 0);
+                return error;
+        }
+
+        xfs_ilock(ip, XFS_ILOCK_EXCL);
+        xfs_trans_ijoin(tp, ip, 0);
+
         /*
          * Lock the inode, fix the size, and join it to the transaction.
          * Hold it so in the normal path, we still have it locked for
@@ -471,7 +478,7 @@ xfs_inactive_symlink_rmt(
         error = xfs_bmapi_read(ip, 0, xfs_symlink_blocks(mp, size),
                                 mval, &nmaps, 0);
         if (error)
-                goto error0;
+                goto error_trans_cancel;
         /*
          * Invalidate the block(s). No validation is done.
          */
@@ -481,22 +488,24 @@ xfs_inactive_symlink_rmt(
                         XFS_FSB_TO_BB(mp, mval[i].br_blockcount), 0);
                 if (!bp) {
                         error = ENOMEM;
-                        goto error1;
+                        goto error_bmap_cancel;
                 }
                 xfs_trans_binval(tp, bp);
         }
         /*
          * Unmap the dead block(s) to the free_list.
          */
-        if ((error = xfs_bunmapi(tp, ip, 0, size, XFS_BMAPI_METADATA, nmaps,
-                        &first_block, &free_list, &done)))
-                goto error1;
+        error = xfs_bunmapi(tp, ip, 0, size, XFS_BMAPI_METADATA, nmaps,
+                            &first_block, &free_list, &done);
+        if (error)
+                goto error_bmap_cancel;
         ASSERT(done);
         /*
          * Commit the first transaction.  This logs the EFI and the inode.
          */
-        if ((error = xfs_bmap_finish(&tp, &free_list, &committed)))
-                goto error1;
+        error = xfs_bmap_finish(&tp, &free_list, &committed);
+        if (error)
+                goto error_bmap_cancel;
         /*
          * The transaction must have been committed, since there were
          * actually extents freed by xfs_bunmapi.  See xfs_bmap_finish.
@@ -511,26 +520,13 @@ xfs_inactive_symlink_rmt(
         xfs_trans_ijoin(tp, ip, 0);
         xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
         /*
-         * Get a new, empty transaction to return to our caller.
-         */
-        ntp = xfs_trans_dup(tp);
-        /*
          * Commit the transaction containing extent freeing and EFDs.
-         * If we get an error on the commit here or on the reserve below,
-         * we need to unlock the inode since the new transaction doesn't
-         * have the inode attached.
          */
-        error = xfs_trans_commit(tp, 0);
-        tp = ntp;
+        error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);
         if (error) {
                 ASSERT(XFS_FORCED_SHUTDOWN(mp));
-                goto error0;
+                goto error_unlock;
         }
-        /*
-         * transaction commit worked ok so we can drop the extra ticket
-         * reference that we gained in xfs_trans_dup()
-         */
-        xfs_log_ticket_put(tp->t_ticket);
 
         /*
          * Remove the memory for extent descriptions (just bookkeeping).
@@ -538,23 +534,16 @@ xfs_inactive_symlink_rmt(
         if (ip->i_df.if_bytes)
                 xfs_idata_realloc(ip, -ip->i_df.if_bytes, XFS_DATA_FORK);
         ASSERT(ip->i_df.if_bytes == 0);
-        /*
-         * Put an itruncate log reservation in the new transaction
-         * for our caller.
-         */
-        error = xfs_trans_reserve(tp, &M_RES(mp)->tr_itruncate, 0, 0);
-        if (error) {
-                ASSERT(XFS_FORCED_SHUTDOWN(mp));
-                goto error0;
-        }
 
-        xfs_trans_ijoin(tp, ip, 0);
-        *tpp = tp;
+        xfs_iunlock(ip, XFS_ILOCK_EXCL);
         return 0;
 
- error1:
+error_bmap_cancel:
         xfs_bmap_cancel(&free_list);
- error0:
+error_trans_cancel:
+        xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
+error_unlock:
+        xfs_iunlock(ip, XFS_ILOCK_EXCL);
         return error;
 }
 
@@ -563,41 +552,46 @@ xfs_inactive_symlink_rmt(
  */
 int
 xfs_inactive_symlink(
-        struct xfs_inode        *ip,
-        struct xfs_trans        **tp)
+        struct xfs_inode        *ip)
 {
         struct xfs_mount        *mp = ip->i_mount;
         int                     pathlen;
 
         trace_xfs_inactive_symlink(ip);
 
-        ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
-
         if (XFS_FORCED_SHUTDOWN(mp))
                 return XFS_ERROR(EIO);
 
+        xfs_ilock(ip, XFS_ILOCK_EXCL);
+
         /*
          * Zero length symlinks _can_ exist.
          */
         pathlen = (int)ip->i_d.di_size;
-        if (!pathlen)
+        if (!pathlen) {
+                xfs_iunlock(ip, XFS_ILOCK_EXCL);
                 return 0;
+        }
 
         if (pathlen < 0 || pathlen > MAXPATHLEN) {
                 xfs_alert(mp, "%s: inode (0x%llx) bad symlink length (%d)",
                          __func__, (unsigned long long)ip->i_ino, pathlen);
+                xfs_iunlock(ip, XFS_ILOCK_EXCL);
                 ASSERT(0);
                 return XFS_ERROR(EFSCORRUPTED);
         }
 
         if (ip->i_df.if_flags & XFS_IFINLINE) {
-                if (ip->i_df.if_bytes > 0)
+                if (ip->i_df.if_bytes > 0) 
                         xfs_idata_realloc(ip, -(ip->i_df.if_bytes),
                                           XFS_DATA_FORK);
+                xfs_iunlock(ip, XFS_ILOCK_EXCL);
                 ASSERT(ip->i_df.if_bytes == 0);
                 return 0;
         }
 
+        xfs_iunlock(ip, XFS_ILOCK_EXCL);
+
         /* remove the remote symlink */
-        return xfs_inactive_symlink_rmt(ip, tp);
+        return xfs_inactive_symlink_rmt(ip);
 }