ocfs2: Fix sleep-with-spinlock recovery regression

This fixes a bug introduced with 539d8264093560b917ee3afe4c7f74e5da09d6a5:
    [PATCH 2/2] ocfs2: Fix race between mount and recovery

ocfs2_mark_dead_nodes() was reading journal inodes while holding the
spinlock protecting our in-memory recovery state. The fix is very simple -
the disk state is protected by a cluster lock that's already held, so we
just move the spinlock down past the read.

Reviewed-by: Joel Becker <joel.becker@oracle.com>
Signed-off-by: Mark Fasheh <mfasheh@suse.com>

diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index 7a37240f7a3117dce66ef2e7a67be76fd2156850..c47bc2a809c2493703563f7f88e2b4f8a5bf1f9c 100644 (file)
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -1418,13 +1418,13 @@ int ocfs2_mark_dead_nodes(struct ocfs2_super *osb)
 {
        unsigned int node_num;
        int status, i;
+       u32 gen;
        struct buffer_head *bh = NULL;
        struct ocfs2_dinode *di;
 
        /* This is called with the super block cluster lock, so we
         * know that the slot map can't change underneath us. */
 
-       spin_lock(&osb->osb_lock);
        for (i = 0; i < osb->max_slots; i++) {
                /* Read journal inode to get the recovery generation */
                status = ocfs2_read_journal_inode(osb, i, &bh, NULL);
@@ -1433,23 +1433,31 @@ int ocfs2_mark_dead_nodes(struct ocfs2_super *osb)
                        goto bail;
                }
                di = (struct ocfs2_dinode *)bh->b_data;
-               osb->slot_recovery_generations[i] =
-                                       ocfs2_get_recovery_generation(di);
+               gen = ocfs2_get_recovery_generation(di);
                brelse(bh);
                bh = NULL;
 
+               spin_lock(&osb->osb_lock);
+               osb->slot_recovery_generations[i] = gen;
+
                mlog(0, "Slot %u recovery generation is %u\n", i,
                     osb->slot_recovery_generations[i]);
 
-               if (i == osb->slot_num)
+               if (i == osb->slot_num) {
+                       spin_unlock(&osb->osb_lock);
                        continue;
+               }
 
                status = ocfs2_slot_to_node_num_locked(osb, i, &node_num);
-               if (status == -ENOENT)
+               if (status == -ENOENT) {
+                       spin_unlock(&osb->osb_lock);
                        continue;
+               }
 
-               if (__ocfs2_recovery_map_test(osb, node_num))
+               if (__ocfs2_recovery_map_test(osb, node_num)) {
+                       spin_unlock(&osb->osb_lock);
                        continue;
+               }
                spin_unlock(&osb->osb_lock);
 
                /* Ok, we have a slot occupied by another node which
@@ -1465,10 +1473,7 @@ int ocfs2_mark_dead_nodes(struct ocfs2_super *osb)
                        mlog_errno(status);
                        goto bail;
                }
-
-               spin_lock(&osb->osb_lock);
        }
-       spin_unlock(&osb->osb_lock);
 
        status = 0;
 bail: