xfs: more sensible inode refcounting for ialloc

Currently we return iodes from xfs_ialloc with just a single reference held.
But we need two references, as one is dropped during transaction commit and
the second needs to be transfered to the VFS.  Change xfs_ialloc to use
xfs_iget plus xfs_trans_ijoin_ref to grab two references to the inode,
and remove the now superflous IHOLD calls from all callers.  This also
greatly simplifies the error handling in xfs_create and also allow to remove
xfs_trans_iget as no other callers are left.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Alex Elder <aelder@sgi.com>

diff --git a/fs/xfs/quota/xfs_qm.c b/fs/xfs/quota/xfs_qm.c
index 206a281..f517963 100644 (file)
--- a/fs/xfs/quota/xfs_qm.c
+++ b/fs/xfs/quota/xfs_qm.c
@@ -1229,13 +1229,6 @@ xfs_qm_qino_alloc(
                return error;
        }
 
-       /*
-        * Keep an extra reference to this quota inode. This inode is
-        * locked exclusively and joined to the transaction already.
-        */
-       ASSERT(xfs_isilocked(*ip, XFS_ILOCK_EXCL));
-       IHOLD(*ip);
-
        /*
         * Make the changes in the superblock, and log those too.
         * sbfields arg may contain fields other than *QUOTINO;

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index be7cf62..c39278b 100644 (file)
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1016,8 +1016,8 @@ xfs_ialloc(
         * This is because we're setting fields here we need
         * to prevent others from looking at until we're done.
         */
-       error = xfs_trans_iget(tp->t_mountp, tp, ino,
-                               XFS_IGET_CREATE, XFS_ILOCK_EXCL, &ip);
+       error = xfs_iget(tp->t_mountp, tp, ino, XFS_IGET_CREATE,
+                        XFS_ILOCK_EXCL, &ip);
        if (error)
                return error;
        ASSERT(ip != NULL);
@@ -1166,6 +1166,7 @@ xfs_ialloc(
        /*
         * Log the new values stuffed into the inode.
         */
+       xfs_trans_ijoin_ref(tp, ip, XFS_ILOCK_EXCL);
        xfs_trans_log_inode(tp, ip, flags);
 
        /* now that we have an i_mode we can setup inode ops and unlock */

diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index c2042b7..06a9759 100644 (file)
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -469,8 +469,6 @@ void                xfs_trans_inode_buf(xfs_trans_t *, struct xfs_buf *);
 void           xfs_trans_stale_inode_buf(xfs_trans_t *, struct xfs_buf *);
 void           xfs_trans_dquot_buf(xfs_trans_t *, struct xfs_buf *, uint);
 void           xfs_trans_inode_alloc_buf(xfs_trans_t *, struct xfs_buf *);
-int            xfs_trans_iget(struct xfs_mount *, xfs_trans_t *,
-                              xfs_ino_t , uint, uint, struct xfs_inode **);
 void           xfs_trans_ichgtime(struct xfs_trans *, struct xfs_inode *, int);
 void           xfs_trans_ijoin_ref(struct xfs_trans *, struct xfs_inode *, uint);
 void           xfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *);

diff --git a/fs/xfs/xfs_trans_inode.c b/fs/xfs/xfs_trans_inode.c
index ccb3453..16084d8 100644 (file)
--- a/fs/xfs/xfs_trans_inode.c
+++ b/fs/xfs/xfs_trans_inode.c
@@ -43,28 +43,6 @@ xfs_trans_inode_broot_debug(
 #define        xfs_trans_inode_broot_debug(ip)
 #endif
 
-/*
- * Get an inode and join it to the transaction.
- */
-int
-xfs_trans_iget(
-       xfs_mount_t     *mp,
-       xfs_trans_t     *tp,
-       xfs_ino_t       ino,
-       uint            flags,
-       uint            lock_flags,
-       xfs_inode_t     **ipp)
-{
-       int                     error;
-
-       error = xfs_iget(mp, tp, ino, flags, lock_flags, ipp);
-       if (!error && tp) {
-               xfs_trans_ijoin(tp, *ipp);
-               (*ipp)->i_itemp->ili_lock_flags = lock_flags;
-       }
-       return error;
-}
-
 /*
  * Add a locked inode to the transaction.
  *

diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index d8e6f8c..258d4f9 100644 (file)
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -1310,7 +1310,7 @@ xfs_create(
        error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid,
                        XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, &udqp, &gdqp);
        if (error)
-               goto std_return;
+               return error;
 
        if (is_dir) {
                rdev = 0;
@@ -1389,12 +1389,6 @@ xfs_create(
                goto out_trans_abort;
        }
 
-       /*
-        * At this point, we've gotten a newly allocated inode.
-        * It is locked (and joined to the transaction).
-        */
-       ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
-
        /*
         * Now we join the directory inode to the transaction.  We do not do it
         * earlier because xfs_dir_ialloc might commit the previous transaction
@@ -1440,22 +1434,13 @@ xfs_create(
         */
        xfs_qm_vop_create_dqattach(tp, ip, udqp, gdqp);
 
-       /*
-        * xfs_trans_commit normally decrements the vnode ref count
-        * when it unlocks the inode. Since we want to return the
-        * vnode to the caller, we bump the vnode ref count now.
-        */
-       IHOLD(ip);
-
        error = xfs_bmap_finish(&tp, &free_list, &committed);
        if (error)
-               goto out_abort_rele;
+               goto out_bmap_cancel;
 
        error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);
-       if (error) {
-               IRELE(ip);
-               goto out_dqrele;
-       }
+       if (error)
+               goto out_release_inode;
 
        xfs_qm_dqrele(udqp);
        xfs_qm_dqrele(gdqp);
@@ -1469,27 +1454,21 @@ xfs_create(
        cancel_flags |= XFS_TRANS_ABORT;
  out_trans_cancel:
        xfs_trans_cancel(tp, cancel_flags);
- out_dqrele:
+ out_release_inode:
+       /*
+        * Wait until after the current transaction is aborted to
+        * release the inode.  This prevents recursive transactions
+        * and deadlocks from xfs_inactive.
+        */
+       if (ip)
+               IRELE(ip);
+
        xfs_qm_dqrele(udqp);
        xfs_qm_dqrele(gdqp);
 
        if (unlock_dp_on_error)
                xfs_iunlock(dp, XFS_ILOCK_EXCL);
- std_return:
        return error;
-
- out_abort_rele:
-       /*
-        * Wait until after the current transaction is aborted to
-        * release the inode.  This prevents recursive transactions
-        * and deadlocks from xfs_inactive.
-        */
-       xfs_bmap_cancel(&free_list);
-       cancel_flags |= XFS_TRANS_ABORT;
-       xfs_trans_cancel(tp, cancel_flags);
-       IRELE(ip);
-       unlock_dp_on_error = B_FALSE;
-       goto out_dqrele;
 }
 
 #ifdef DEBUG
@@ -2114,9 +2093,8 @@ xfs_symlink(
                                  XFS_BMAPI_WRITE | XFS_BMAPI_METADATA,
                                  &first_block, resblks, mval, &nmaps,
                                  &free_list);
-               if (error) {
-                       goto error1;
-               }
+               if (error)
+                       goto error2;
 
                if (resblks)
                        resblks -= fs_blocks;
@@ -2148,7 +2126,7 @@ xfs_symlink(
        error = xfs_dir_createname(tp, dp, link_name, ip->i_ino,
                                        &first_block, &free_list, resblks);
        if (error)
-               goto error1;
+               goto error2;
        xfs_trans_ichgtime(tp, dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
        xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
 
@@ -2161,13 +2139,6 @@ xfs_symlink(
                xfs_trans_set_sync(tp);
        }
 
-       /*
-        * xfs_trans_commit normally decrements the vnode ref count
-        * when it unlocks the inode. Since we want to return the
-        * vnode to the caller, we bump the vnode ref count now.
-        */
-       IHOLD(ip);
-
        error = xfs_bmap_finish(&tp, &free_list, &committed);
        if (error) {
                goto error2;