[PATCH] ocfs2: fix orphan recovery deadlock

Orphan dir recovery can deadlock with another process in
ocfs2_delete_inode() in some corner cases. Fix this by tracking recovery
state more closely and allowing it to handle inode wipes which might
deadlock.

Signed-off-by: Mark Fasheh <mark.fasheh@oracle.com>

1 files changed, 93 insertions, 31 deletions

diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index d329c9df90ae..4be801f4559b 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -1408,21 +1408,17 @@ bail:
         return status;
 }
 
-static int ocfs2_recover_orphans(struct ocfs2_super *osb,
-                                 int slot)
+static int ocfs2_queue_orphans(struct ocfs2_super *osb,
+                               int slot,
+                               struct inode **head)
 {
-        int status = 0;
-        int have_disk_lock = 0;
-        struct inode *inode = NULL;
-        struct inode *iter;
+        int status;
         struct inode *orphan_dir_inode = NULL;
+        struct inode *iter;
         unsigned long offset, blk, local;
         struct buffer_head *bh = NULL;
         struct ocfs2_dir_entry *de;
         struct super_block *sb = osb->sb;
-        struct ocfs2_inode_info *oi;
-
-        mlog(0, "Recover inodes from orphan dir in slot %d\n", slot);
 
         orphan_dir_inode = ocfs2_get_system_file_inode(osb,
                                                        ORPHAN_DIR_SYSTEM_INODE,
@@ -1430,17 +1426,15 @@ static int ocfs2_recover_orphans(struct ocfs2_super *osb,
         if  (!orphan_dir_inode) {
                 status = -ENOENT;
                 mlog_errno(status);
-                goto out;
-        }
+                return status;
+        }       
 
         mutex_lock(&orphan_dir_inode->i_mutex);
         status = ocfs2_meta_lock(orphan_dir_inode, NULL, NULL, 0);
         if (status < 0) {
-                mutex_unlock(&orphan_dir_inode->i_mutex);
                 mlog_errno(status);
                 goto out;
         }
-        have_disk_lock = 1;
 
         offset = 0;
         iter = NULL;
@@ -1451,11 +1445,10 @@ static int ocfs2_recover_orphans(struct ocfs2_super *osb,
                 if (!bh)
                         status = -EINVAL;
                 if (status < 0) {
-                        mutex_unlock(&orphan_dir_inode->i_mutex);
                         if (bh)
                                 brelse(bh);
                         mlog_errno(status);
-                        goto out;
+                        goto out_unlock;
                 }
 
                 local = 0;
@@ -1465,11 +1458,10 @@ static int ocfs2_recover_orphans(struct ocfs2_super *osb,
 
                         if (!ocfs2_check_dir_entry(orphan_dir_inode,
                                                   de, bh, local)) {
-                                mutex_unlock(&orphan_dir_inode->i_mutex);
                                 status = -EINVAL;
                                 mlog_errno(status);
                                 brelse(bh);
-                                goto out;
+                                goto out_unlock;
                         }
 
                         local += le16_to_cpu(de->rec_len);
@@ -1504,18 +1496,95 @@ static int ocfs2_recover_orphans(struct ocfs2_super *osb,
 
                         mlog(0, "queue orphan %"MLFu64"\n",
                              OCFS2_I(iter)->ip_blkno);
-                        OCFS2_I(iter)->ip_next_orphan = inode;
-                        inode = iter;
+                        /* No locking is required for the next_orphan
+                         * queue as there is only ever a single
+                         * process doing orphan recovery. */
+                        OCFS2_I(iter)->ip_next_orphan = *head;
+                        *head = iter;
                 }
                 brelse(bh);
         }
-        mutex_unlock(&orphan_dir_inode->i_mutex);
 
+out_unlock:
         ocfs2_meta_unlock(orphan_dir_inode, 0);
-        have_disk_lock = 0;
-
+out:
+        mutex_unlock(&orphan_dir_inode->i_mutex);
         iput(orphan_dir_inode);
-        orphan_dir_inode = NULL;
+        return status;
+}
+
+static int ocfs2_orphan_recovery_can_continue(struct ocfs2_super *osb,
+                                              int slot)
+{
+        int ret;
+
+        spin_lock(&osb->osb_lock);
+        ret = !osb->osb_orphan_wipes[slot];
+        spin_unlock(&osb->osb_lock);
+        return ret;
+}
+
+static void ocfs2_mark_recovering_orphan_dir(struct ocfs2_super *osb,
+                                             int slot)
+{
+        spin_lock(&osb->osb_lock);
+        /* Mark ourselves such that new processes in delete_inode()
+         * know to quit early. */
+        ocfs2_node_map_set_bit(osb, &osb->osb_recovering_orphan_dirs, slot);
+        while (osb->osb_orphan_wipes[slot]) {
+                /* If any processes are already in the middle of an
+                 * orphan wipe on this dir, then we need to wait for
+                 * them. */
+                spin_unlock(&osb->osb_lock);
+                wait_event_interruptible(osb->osb_wipe_event,
+                                         ocfs2_orphan_recovery_can_continue(osb, slot));
+                spin_lock(&osb->osb_lock);
+        }
+        spin_unlock(&osb->osb_lock);
+}
+
+static void ocfs2_clear_recovering_orphan_dir(struct ocfs2_super *osb,
+                                              int slot)
+{
+        ocfs2_node_map_clear_bit(osb, &osb->osb_recovering_orphan_dirs, slot);
+}
+
+/*
+ * Orphan recovery. Each mounted node has it's own orphan dir which we
+ * must run during recovery. Our strategy here is to build a list of
+ * the inodes in the orphan dir and iget/iput them. The VFS does
+ * (most) of the rest of the work.
+ *
+ * Orphan recovery can happen at any time, not just mount so we have a
+ * couple of extra considerations.
+ *
+ * - We grab as many inodes as we can under the orphan dir lock -
+ *   doing iget() outside the orphan dir risks getting a reference on
+ *   an invalid inode.
+ * - We must be sure not to deadlock with other processes on the
+ *   system wanting to run delete_inode(). This can happen when they go
+ *   to lock the orphan dir and the orphan recovery process attempts to
+ *   iget() inside the orphan dir lock. This can be avoided by
+ *   advertising our state to ocfs2_delete_inode().
+ */
+static int ocfs2_recover_orphans(struct ocfs2_super *osb,
+                                 int slot)
+{
+        int ret = 0;
+        struct inode *inode = NULL;
+        struct inode *iter;
+        struct ocfs2_inode_info *oi;
+
+        mlog(0, "Recover inodes from orphan dir in slot %d\n", slot);
+
+        ocfs2_mark_recovering_orphan_dir(osb, slot);
+        ret = ocfs2_queue_orphans(osb, slot, &inode);
+        ocfs2_clear_recovering_orphan_dir(osb, slot);
+
+        /* Error here should be noted, but we want to continue with as
+         * many queued inodes as we've got. */
+        if (ret)
+                mlog_errno(ret);
 
         while (inode) {
                 oi = OCFS2_I(inode);
@@ -1541,14 +1610,7 @@ static int ocfs2_recover_orphans(struct ocfs2_super *osb,
                 inode = iter;
         }
 
-out:
-        if (have_disk_lock)
-                ocfs2_meta_unlock(orphan_dir_inode, 0);
-
-        if (orphan_dir_inode)
-                iput(orphan_dir_inode);
-
-        return status;
+        return ret;
 }
 
 static int ocfs2_wait_on_mount(struct ocfs2_super *osb)