xfs: clean up xfs_inactive

The code to reserve log space and join the inode to the transaction is
common for all cases, so don't duplicate it.  Also remove the trivial
xfs_inactive_symlink_local helper which can simply be opencode now.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Rich Johnston <rjohnston@sgi.com>
Signed-off-by: Ben Myers <bpm@sgi.com>

1 files changed, 43 insertions, 128 deletions

diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index c22f4e0ecac1..f9a515776a9c 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -282,23 +282,15 @@ xfs_inactive_symlink_rmt(
          * free them all in one bunmapi call.
          */
         ASSERT(ip->i_d.di_nextents > 0 && ip->i_d.di_nextents <= 2);
-        if ((error = xfs_trans_reserve(tp, 0, XFS_ITRUNCATE_LOG_RES(mp), 0,
-                        XFS_TRANS_PERM_LOG_RES, XFS_ITRUNCATE_LOG_COUNT))) {
-                ASSERT(XFS_FORCED_SHUTDOWN(mp));
-                xfs_trans_cancel(tp, 0);
-                *tpp = NULL;
-                return error;
-        }
+
         /*
          * 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
          * the second transaction.  In the error paths we need it
          * held so the cancel won't rele it, see below.
          */
-        xfs_ilock(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
         size = (int)ip->i_d.di_size;
         ip->i_d.di_size = 0;
-        xfs_trans_ijoin(tp, ip, 0);
         xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
         /*
          * Find the block(s) so we can inval and unmap them.
@@ -385,67 +377,15 @@ xfs_inactive_symlink_rmt(
                 ASSERT(XFS_FORCED_SHUTDOWN(mp));
                 goto error0;
         }
-        /*
-         * Return with the inode locked but not joined to the transaction.
-         */
+
+        xfs_trans_ijoin(tp, ip, 0);
         *tpp = tp;
         return 0;
 
  error1:
         xfs_bmap_cancel(&free_list);
  error0:
-        /*
-         * Have to come here with the inode locked and either
-         * (held and in the transaction) or (not in the transaction).
-         * If the inode isn't held then cancel would iput it, but
-         * that's wrong since this is inactive and the vnode ref
-         * count is 0 already.
-         * Cancel won't do anything to the inode if held, but it still
-         * needs to be locked until the cancel is done, if it was
-         * joined to the transaction.
-         */
-        xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
-        xfs_iunlock(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
-        *tpp = NULL;
         return error;
-
-}
-
-STATIC int
-xfs_inactive_symlink_local(
-        xfs_inode_t     *ip,
-        xfs_trans_t     **tpp)
-{
-        int             error;
-
-        ASSERT(ip->i_d.di_size <= XFS_IFORK_DSIZE(ip));
-        /*
-         * We're freeing a symlink which fit into
-         * the inode.  Just free the memory used
-         * to hold the old symlink.
-         */
-        error = xfs_trans_reserve(*tpp, 0,
-                                  XFS_ITRUNCATE_LOG_RES(ip->i_mount),
-                                  0, XFS_TRANS_PERM_LOG_RES,
-                                  XFS_ITRUNCATE_LOG_COUNT);
-
-        if (error) {
-                xfs_trans_cancel(*tpp, 0);
-                *tpp = NULL;
-                return error;
-        }
-        xfs_ilock(ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
-
-        /*
-         * Zero length symlinks _can_ exist.
-         */
-        if (ip->i_df.if_bytes > 0) {
-                xfs_idata_realloc(ip,
-                                  -(ip->i_df.if_bytes),
-                                  XFS_DATA_FORK);
-                ASSERT(ip->i_df.if_bytes == 0);
-        }
-        return 0;
 }
 
 STATIC int
@@ -604,7 +544,7 @@ xfs_inactive(
         xfs_trans_t     *tp;
         xfs_mount_t     *mp;
         int             error;
-        int             truncate;
+        int             truncate = 0;
 
         /*
          * If the inode is already free, then there can be nothing
@@ -616,17 +556,6 @@ xfs_inactive(
                 return VN_INACTIVE_CACHE;
         }
 
-        /*
-         * Only do a truncate if it's a regular file with
-         * some actual space in it.  It's OK to look at the
-         * inode's fields without the lock because we're the
-         * only one with a reference to the inode.
-         */
-        truncate = ((ip->i_d.di_nlink == 0) &&
-            ((ip->i_d.di_size != 0) || XFS_ISIZE(ip) != 0 ||
-             (ip->i_d.di_nextents > 0) || (ip->i_delayed_blks > 0)) &&
-            S_ISREG(ip->i_d.di_mode));
-
         mp = ip->i_mount;
 
         error = 0;
@@ -650,72 +579,54 @@ xfs_inactive(
                 goto out;
         }
 
-        ASSERT(ip->i_d.di_nlink == 0);
+        if (S_ISREG(ip->i_d.di_mode) &&
+            (ip->i_d.di_size != 0 || XFS_ISIZE(ip) != 0 ||
+             ip->i_d.di_nextents > 0 || ip->i_delayed_blks > 0))
+                truncate = 1;
 
         error = xfs_qm_dqattach(ip, 0);
         if (error)
                 return VN_INACTIVE_CACHE;
 
         tp = xfs_trans_alloc(mp, XFS_TRANS_INACTIVE);
-        if (truncate) {
-                xfs_ilock(ip, XFS_IOLOCK_EXCL);
-
-                error = xfs_trans_reserve(tp, 0,
-                                          XFS_ITRUNCATE_LOG_RES(mp),
-                                          0, XFS_TRANS_PERM_LOG_RES,
-                                          XFS_ITRUNCATE_LOG_COUNT);
-                if (error) {
-                        /* Don't call itruncate_cleanup */
-                        ASSERT(XFS_FORCED_SHUTDOWN(mp));
-                        xfs_trans_cancel(tp, 0);
-                        xfs_iunlock(ip, XFS_IOLOCK_EXCL);
-                        return VN_INACTIVE_CACHE;
-                }
+        error = xfs_trans_reserve(tp, 0,
+                        (truncate || S_ISLNK(ip->i_d.di_mode)) ?
+                                XFS_ITRUNCATE_LOG_RES(mp) :
+                                XFS_IFREE_LOG_RES(mp),
+                        0,
+                        XFS_TRANS_PERM_LOG_RES,
+                        XFS_ITRUNCATE_LOG_COUNT);
+        if (error) {
+                ASSERT(XFS_FORCED_SHUTDOWN(mp));
+                xfs_trans_cancel(tp, 0);
+                return VN_INACTIVE_CACHE;
+        }
 
-                xfs_ilock(ip, XFS_ILOCK_EXCL);
-                xfs_trans_ijoin(tp, ip, 0);
+        xfs_ilock(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
+        xfs_trans_ijoin(tp, ip, 0);
 
+        if (S_ISLNK(ip->i_d.di_mode)) {
+                /*
+                 * Zero length symlinks _can_ exist.
+                 */
+                if (ip->i_d.di_size > XFS_IFORK_DSIZE(ip)) {
+                        error = xfs_inactive_symlink_rmt(ip, &tp);
+                        if (error)
+                                goto out_cancel;
+                } else if (ip->i_df.if_bytes > 0) {
+                        xfs_idata_realloc(ip, -(ip->i_df.if_bytes),
+                                          XFS_DATA_FORK);
+                        ASSERT(ip->i_df.if_bytes == 0);
+                }
+        } else if (truncate) {
                 ip->i_d.di_size = 0;
                 xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
 
                 error = xfs_itruncate_extents(&tp, ip, XFS_DATA_FORK, 0);
-                if (error) {
-                        xfs_trans_cancel(tp,
-                                XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
-                        xfs_iunlock(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
-                        return VN_INACTIVE_CACHE;
-                }
+                if (error)
+                        goto out_cancel;
 
                 ASSERT(ip->i_d.di_nextents == 0);
-        } else if (S_ISLNK(ip->i_d.di_mode)) {
-
-                /*
-                 * If we get an error while cleaning up a
-                 * symlink we bail out.
-                 */
-                error = (ip->i_d.di_size > XFS_IFORK_DSIZE(ip)) ?
-                        xfs_inactive_symlink_rmt(ip, &tp) :
-                        xfs_inactive_symlink_local(ip, &tp);
-
-                if (error) {
-                        ASSERT(tp == NULL);
-                        return VN_INACTIVE_CACHE;
-                }
-
-                xfs_trans_ijoin(tp, ip, 0);
-        } else {
-                error = xfs_trans_reserve(tp, 0,
-                                          XFS_IFREE_LOG_RES(mp),
-                                          0, XFS_TRANS_PERM_LOG_RES,
-                                          XFS_INACTIVE_LOG_COUNT);
-                if (error) {
-                        ASSERT(XFS_FORCED_SHUTDOWN(mp));
-                        xfs_trans_cancel(tp, 0);
-                        return VN_INACTIVE_CACHE;
-                }
-
-                xfs_ilock(ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
-                xfs_trans_ijoin(tp, ip, 0);
         }
 
         /*
@@ -781,7 +692,11 @@ xfs_inactive(
         xfs_qm_dqdetach(ip);
         xfs_iunlock(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
 
- out:
+out:
+        return VN_INACTIVE_CACHE;
+out_cancel:
+        xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
+        xfs_iunlock(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
         return VN_INACTIVE_CACHE;
 }