[REVIEW 2 of 4] Remove igrab from xfs_iunpin

[David Chinner <dgc@xxxxxxx>]

-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group


Remove the need for grabbing the linux inode in xfs_iunpin(). This
causes deadlocks when the xfslogd drops the final reference to an
inode and needs to issue a transaction when the log is full. We can
do this by providing a guarantee external to xfs_iunpin() that when
either of the XFS_IRECLAIM or XFS_IRECLAIMABLE flags are set on the
xfs inode there is no linux inode to look up.

---
 fs/xfs/xfs_inode.c    |   44 +++++++++++++++++++++-----------------------
 fs/xfs/xfs_vnodeops.c |   21 ++++++++++++++-------
 2 files changed, 35 insertions(+), 30 deletions(-)

Index: 2.6.x-xfs-new/fs/xfs/xfs_inode.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_inode.c       2006-10-19 10:25:07.334515530 
+1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_inode.c    2006-10-19 10:26:01.983441994 +1000
@@ -2742,41 +2742,39 @@ xfs_iunpin(
        ASSERT(atomic_read(&ip->i_pincount) > 0);
 
        if (atomic_dec_and_test(&ip->i_pincount)) {
+
                /*
-                * If the inode is currently being reclaimed, the
-                * linux inode _and_ the xfs vnode may have been
-                * freed so we cannot reference either of them safely.
-                * Hence we should not try to do anything to them
-                * if the xfs inode is currently in the reclaim
-                * path.
+                * If the inode is currently being reclaimed, the link between
+                * the bhv_vnode and the xfs_inode will be broken after the
+                * XFS_IRECLAIM* flag is set. Hence, if these flags are not
+                * set, then we can move forward and mark the linux inode dirty
+                * knowing that it is still valid as it won't freed until after
+                * the bhv_vnode<->xfs_inode link is broken in xfs_reclaim. The
+                * i_flags_lock is used to synchronise the setting of the
+                * XFS_IRECLAIM* flags and the breaking of the link, and so we
+                * can execute atomically w.r.t to reclaim by holding this lock
+                * here.
                 *
-                * However, we still need to issue the unpin wakeup
-                * call as the inode reclaim may be blocked waiting for
-                * the inode to become unpinned.
+                * However, we still need to issue the unpin wakeup call as the
+                * inode reclaim may be blocked waiting for the inode to become
+                * unpinned.
                 */
-               struct inode *inode = NULL;
 
                spin_lock(&ip->i_flags_lock);
                if (!__xfs_iflags_test(ip, XFS_IRECLAIM|XFS_IRECLAIMABLE)) {
                        bhv_vnode_t     *vp = XFS_ITOV_NULL(ip);
+                       struct inode *inode = NULL;
 
-                       /* make sync come back and flush this inode */
-                       if (vp) {
-                               inode = vn_to_inode(vp);
+                       BUG_ON(vp == NULL);
+                       inode = vn_to_inode(vp);
+                       BUG_ON(inode->i_state & I_CLEAR);
 
-                               if (!(inode->i_state &
-                                               (I_NEW|I_FREEING|I_CLEAR))) {
-                                       inode = igrab(inode);
-                                       if (inode)
-                                               mark_inode_dirty_sync(inode);
-                               } else
-                                       inode = NULL;
-                       }
+                       /* make sync come back and flush this inode */
+                       if (!(inode->i_state & (I_NEW|I_FREEING)))
+                               mark_inode_dirty_sync(inode);
                }
                spin_unlock(&ip->i_flags_lock);
                wake_up(&ip->i_ipin_wait);
-               if (inode)
-                       iput(inode);
        }
 }
 
Index: 2.6.x-xfs-new/fs/xfs/xfs_vnodeops.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_vnodeops.c    2006-10-19 10:25:07.338515013 
+1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_vnodeops.c 2006-10-19 10:25:17.957140602 +1000
@@ -3827,11 +3827,16 @@ xfs_reclaim(
         */
        xfs_synchronize_atime(ip);
 
-       /* If we have nothing to flush with this inode then complete the
-        * teardown now, otherwise break the link between the xfs inode
-        * and the linux inode and clean up the xfs inode later. This
-        * avoids flushing the inode to disk during the delete operation
-        * itself.
+       /*
+        * If we have nothing to flush with this inode then complete the
+        * teardown now, otherwise break the link between the xfs inode and the
+        * linux inode and clean up the xfs inode later. This avoids flushing
+        * the inode to disk during the delete operation itself.
+        *
+        * When breaking the link, we need to set the XFS_IRECLAIMABLE flag
+        * first to ensure that xfs_iunpin() will never see an xfs inode
+        * that has a linux inode being reclaimed. Synchronisation is provided
+        * by the i_flags_lock.
         */
        if (!ip->i_update_core && (ip->i_itemp == NULL)) {
                xfs_ilock(ip, XFS_ILOCK_EXCL);
@@ -3840,11 +3845,13 @@ xfs_reclaim(
        } else {
                xfs_mount_t     *mp = ip->i_mount;
 
-               /* Protect sync from us */
+               /* Protect sync and unpin from us */
                XFS_MOUNT_ILOCK(mp);
+               spin_lock(&ip->i_flags_lock);
+               __xfs_iflags_set(ip, XFS_IRECLAIMABLE);
                vn_bhv_remove(VN_BHV_HEAD(vp), XFS_ITOBHV(ip));
+               spin_unlock(&ip->i_flags_lock);
                list_add_tail(&ip->i_reclaim, &mp->m_del_inodes);
-               xfs_iflags_set(ip, XFS_IRECLAIMABLE);
                XFS_MOUNT_IUNLOCK(mp);
        }
        return 0;