Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a1af7d15 authored by Mark Fasheh's avatar Mark Fasheh
Browse files

ocfs2: Fix sleep-with-spinlock recovery regression 



This fixes a bug introduced with 539d8264:
    [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: default avatarJoel Becker <joel.becker@oracle.com>
Signed-off-by: default avatarMark Fasheh <mfasheh@suse.com>
parent a57a874b

fs/ocfs2/journal.c

+14 −9
Original line number Original line Diff line number Diff line
@@ -1418,13 +1418,13 @@ int ocfs2_mark_dead_nodes(struct ocfs2_super *osb) 
{

{

	unsigned int node_num;

	unsigned int node_num;

	int status, i;

	int status, i;

	u32 gen;

	struct buffer_head *bh = NULL;

	struct buffer_head *bh = NULL;

	struct ocfs2_dinode *di;

	struct ocfs2_dinode *di;





	/* This is called with the super block cluster lock, so we

	/* This is called with the super block cluster lock, so we

	 * know that the slot map can't change underneath us. */

	 * know that the slot map can't change underneath us. */





	spin_lock(&osb->osb_lock);

	for (i = 0; i < osb->max_slots; i++) {

	for (i = 0; i < osb->max_slots; i++) {

		/* Read journal inode to get the recovery generation */

		/* Read journal inode to get the recovery generation */

		status = ocfs2_read_journal_inode(osb, i, &bh, NULL);

		status = ocfs2_read_journal_inode(osb, i, &bh, NULL);
@@ -1433,23 +1433,31 @@ int ocfs2_mark_dead_nodes(struct ocfs2_super *osb) 
			goto bail;

			goto bail;

		}

		}

		di = (struct ocfs2_dinode *)bh->b_data;

		di = (struct ocfs2_dinode *)bh->b_data;

		osb->slot_recovery_generations[i] =

		gen = ocfs2_get_recovery_generation(di);

					ocfs2_get_recovery_generation(di);

		brelse(bh);

		brelse(bh);

		bh = NULL;

		bh = NULL;





		spin_lock(&osb->osb_lock);

		osb->slot_recovery_generations[i] = gen;



		mlog(0, "Slot %u recovery generation is %u\n", i,

		mlog(0, "Slot %u recovery generation is %u\n", i,

		     osb->slot_recovery_generations[i]);

		     osb->slot_recovery_generations[i]);





		if (i == osb->slot_num)

		if (i == osb->slot_num) {

			spin_unlock(&osb->osb_lock);

			continue;

			continue;

		}





		status = ocfs2_slot_to_node_num_locked(osb, i, &node_num);

		status = ocfs2_slot_to_node_num_locked(osb, i, &node_num);

		if (status == -ENOENT)

		if (status == -ENOENT) {

			spin_unlock(&osb->osb_lock);

			continue;

			continue;

		}





		if (__ocfs2_recovery_map_test(osb, node_num))

		if (__ocfs2_recovery_map_test(osb, node_num)) {

			spin_unlock(&osb->osb_lock);

			continue;

			continue;

		}

		spin_unlock(&osb->osb_lock);

		spin_unlock(&osb->osb_lock);





		/* Ok, we have a slot occupied by another node which

		/* 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);

			mlog_errno(status);

			goto bail;

			goto bail;

		}

		}



		spin_lock(&osb->osb_lock);

	}

	}

	spin_unlock(&osb->osb_lock);





	status = 0;

	status = 0;

bail:

bail: