[Cluster-devel] [GFS2 PATCH] GFS2: Change recovery to use mutex instead of spin_lock

[Bob Peterson <rpeterso@redhat.com>]

Hi,

GFS2's recovery procedure relied heavily on spin_locks to protect
certain variables, but spin_locks consume CPU time that is better
spent allowing things like corosync to run. This patch switches it
to a mutex, which will sleep until it becomes available.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/incore.h   |  2 +-
 fs/gfs2/lock_dlm.c | 74 +++++++++++++++++++++++++++---------------------------
 2 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index b7cf65d..b8049be 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -661,7 +661,7 @@ struct lm_lockstruct {
 	struct completion ls_sync_wait; /* {control,mounted}_{lock,unlock} */
 	char *ls_lvb_bits;
 
-	spinlock_t ls_recover_spin; /* protects following fields */
+	struct mutex ls_recover_mutex; /* protects following fields */
 	unsigned long ls_recover_flags; /* DFL_ */
 	uint32_t ls_recover_mount; /* gen in first recover_done cb */
 	uint32_t ls_recover_start; /* gen in last recover_done cb */
diff --git a/fs/gfs2/lock_dlm.c b/fs/gfs2/lock_dlm.c
index 0515f0a..b2fd828 100644
--- a/fs/gfs2/lock_dlm.c
+++ b/fs/gfs2/lock_dlm.c
@@ -585,7 +585,7 @@ static void gfs2_control_func(struct work_struct *work)
 	int recover_size;
 	int i, error;
 
-	spin_lock(&ls->ls_recover_spin);
+	mutex_lock(&ls->ls_recover_mutex);
 	/*
 	 * No MOUNT_DONE means we're still mounting; control_mount()
 	 * will set this flag, after which this thread will take over
@@ -597,12 +597,12 @@ static void gfs2_control_func(struct work_struct *work)
 	 */
 	if (!test_bit(DFL_MOUNT_DONE, &ls->ls_recover_flags) ||
 	     test_bit(DFL_FIRST_MOUNT, &ls->ls_recover_flags)) {
-		spin_unlock(&ls->ls_recover_spin);
+		mutex_unlock(&ls->ls_recover_mutex);
 		return;
 	}
 	block_gen = ls->ls_recover_block;
 	start_gen = ls->ls_recover_start;
-	spin_unlock(&ls->ls_recover_spin);
+	mutex_unlock(&ls->ls_recover_mutex);
 
 	/*
 	 * Equal block_gen and start_gen implies we are between
@@ -634,12 +634,12 @@ static void gfs2_control_func(struct work_struct *work)
 
 	control_lvb_read(ls, &lvb_gen, ls->ls_lvb_bits);
 
-	spin_lock(&ls->ls_recover_spin);
+	mutex_lock(&ls->ls_recover_mutex);
 	if (block_gen != ls->ls_recover_block ||
 	    start_gen != ls->ls_recover_start) {
 		fs_info(sdp, "recover generation %u block1 %u %u\n",
 			start_gen, block_gen, ls->ls_recover_block);
-		spin_unlock(&ls->ls_recover_spin);
+		mutex_unlock(&ls->ls_recover_mutex);
 		control_lock(sdp, DLM_LOCK_NL, DLM_LKF_CONVERT);
 		return;
 	}
@@ -700,7 +700,7 @@ static void gfs2_control_func(struct work_struct *work)
 		 * we should be getting a recover_done() for lvb_gen soon
 		 */
 	}
-	spin_unlock(&ls->ls_recover_spin);
+	mutex_unlock(&ls->ls_recover_mutex);
 
 	if (write_lvb) {
 		control_lvb_write(ls, start_gen, ls->ls_lvb_bits);
@@ -739,17 +739,17 @@ static void gfs2_control_func(struct work_struct *work)
 	 * again while working above)
 	 */
 
-	spin_lock(&ls->ls_recover_spin);
+	mutex_lock(&ls->ls_recover_mutex);
 	if (ls->ls_recover_block == block_gen &&
 	    ls->ls_recover_start == start_gen) {
 		clear_bit(DFL_BLOCK_LOCKS, &ls->ls_recover_flags);
-		spin_unlock(&ls->ls_recover_spin);
+		mutex_unlock(&ls->ls_recover_mutex);
 		fs_info(sdp, "recover generation %u done\n", start_gen);
 		gfs2_glock_thaw(sdp);
 	} else {
 		fs_info(sdp, "recover generation %u block2 %u %u\n",
 			start_gen, block_gen, ls->ls_recover_block);
-		spin_unlock(&ls->ls_recover_spin);
+		mutex_unlock(&ls->ls_recover_mutex);
 	}
 }
 
@@ -865,11 +865,11 @@ static int control_mount(struct gfs2_sbd *sdp)
 
 	if (mounted_mode == DLM_LOCK_EX) {
 		/* first mounter, keep both EX while doing first recovery */
-		spin_lock(&ls->ls_recover_spin);
+		mutex_lock(&ls->ls_recover_mutex);
 		clear_bit(DFL_BLOCK_LOCKS, &ls->ls_recover_flags);
 		set_bit(DFL_MOUNT_DONE, &ls->ls_recover_flags);
 		set_bit(DFL_FIRST_MOUNT, &ls->ls_recover_flags);
-		spin_unlock(&ls->ls_recover_spin);
+		mutex_unlock(&ls->ls_recover_mutex);
 		fs_info(sdp, "first mounter control generation %u\n", lvb_gen);
 		return 0;
 	}
@@ -890,7 +890,7 @@ static int control_mount(struct gfs2_sbd *sdp)
 		goto restart;
 	}
 
-	spin_lock(&ls->ls_recover_spin);
+	mutex_lock(&ls->ls_recover_mutex);
 	block_gen = ls->ls_recover_block;
 	start_gen = ls->ls_recover_start;
 	mount_gen = ls->ls_recover_mount;
@@ -901,7 +901,7 @@ static int control_mount(struct gfs2_sbd *sdp)
 		fs_info(sdp, "control_mount wait1 block %u start %u mount %u "
 			"lvb %u flags %lx\n", block_gen, start_gen, mount_gen,
 			lvb_gen, ls->ls_recover_flags);
-		spin_unlock(&ls->ls_recover_spin);
+		mutex_unlock(&ls->ls_recover_mutex);
 		goto restart;
 	}
 
@@ -911,7 +911,7 @@ static int control_mount(struct gfs2_sbd *sdp)
 		fs_info(sdp, "control_mount wait2 block %u start %u mount %u "
 			"lvb %u flags %lx\n", block_gen, start_gen, mount_gen,
 			lvb_gen, ls->ls_recover_flags);
-		spin_unlock(&ls->ls_recover_spin);
+		mutex_unlock(&ls->ls_recover_mutex);
 		goto restart;
 	}
 
@@ -920,7 +920,7 @@ static int control_mount(struct gfs2_sbd *sdp)
 		fs_info(sdp, "control_mount wait3 block %u start %u mount %u "
 			"lvb %u flags %lx\n", block_gen, start_gen, mount_gen,
 			lvb_gen, ls->ls_recover_flags);
-		spin_unlock(&ls->ls_recover_spin);
+		mutex_unlock(&ls->ls_recover_mutex);
 		goto restart;
 	}
 
@@ -928,7 +928,7 @@ static int control_mount(struct gfs2_sbd *sdp)
 	set_bit(DFL_MOUNT_DONE, &ls->ls_recover_flags);
 	memset(ls->ls_recover_submit, 0, ls->ls_recover_size*sizeof(uint32_t));
 	memset(ls->ls_recover_result, 0, ls->ls_recover_size*sizeof(uint32_t));
-	spin_unlock(&ls->ls_recover_spin);
+	mutex_unlock(&ls->ls_recover_mutex);
 	return 0;
 
 fail:
@@ -944,7 +944,7 @@ static int control_first_done(struct gfs2_sbd *sdp)
 	int error;
 
 restart:
-	spin_lock(&ls->ls_recover_spin);
+	mutex_lock(&ls->ls_recover_mutex);
 	start_gen = ls->ls_recover_start;
 	block_gen = ls->ls_recover_block;
 
@@ -954,7 +954,7 @@ static int control_first_done(struct gfs2_sbd *sdp)
 		/* sanity check, should not happen */
 		fs_err(sdp, "control_first_done start %u block %u flags %lx\n",
 		       start_gen, block_gen, ls->ls_recover_flags);
-		spin_unlock(&ls->ls_recover_spin);
+		mutex_unlock(&ls->ls_recover_mutex);
 		control_unlock(sdp);
 		return -1;
 	}
@@ -967,7 +967,7 @@ static int control_first_done(struct gfs2_sbd *sdp)
 		 * because we are still the first mounter and any failed nodes
 		 * have not fully mounted, so they don't need recovery.
 		 */
-		spin_unlock(&ls->ls_recover_spin);
+		mutex_unlock(&ls->ls_recover_mutex);
 		fs_info(sdp, "control_first_done wait gen %u\n", start_gen);
 
 		wait_on_bit(&ls->ls_recover_flags, DFL_DLM_RECOVERY,
@@ -979,7 +979,7 @@ static int control_first_done(struct gfs2_sbd *sdp)
 	set_bit(DFL_FIRST_MOUNT_DONE, &ls->ls_recover_flags);
 	memset(ls->ls_recover_submit, 0, ls->ls_recover_size*sizeof(uint32_t));
 	memset(ls->ls_recover_result, 0, ls->ls_recover_size*sizeof(uint32_t));
-	spin_unlock(&ls->ls_recover_spin);
+	mutex_unlock(&ls->ls_recover_mutex);
 
 	memset(ls->ls_lvb_bits, 0, GDLM_LVB_SIZE);
 	control_lvb_write(ls, start_gen, ls->ls_lvb_bits);
@@ -1039,7 +1039,7 @@ static int set_recover_size(struct gfs2_sbd *sdp, struct dlm_slot *slots,
 		return -ENOMEM;
 	}
 
-	spin_lock(&ls->ls_recover_spin);
+	mutex_lock(&ls->ls_recover_mutex);
 	memcpy(submit, ls->ls_recover_submit, old_size * sizeof(uint32_t));
 	memcpy(result, ls->ls_recover_result, old_size * sizeof(uint32_t));
 	kfree(ls->ls_recover_submit);
@@ -1047,7 +1047,7 @@ static int set_recover_size(struct gfs2_sbd *sdp, struct dlm_slot *slots,
 	ls->ls_recover_submit = submit;
 	ls->ls_recover_result = result;
 	ls->ls_recover_size = new_size;
-	spin_unlock(&ls->ls_recover_spin);
+	mutex_unlock(&ls->ls_recover_mutex);
 	return 0;
 }
 
@@ -1068,17 +1068,17 @@ static void gdlm_recover_prep(void *arg)
 	struct gfs2_sbd *sdp = arg;
 	struct lm_lockstruct *ls = &sdp->sd_lockstruct;
 
-	spin_lock(&ls->ls_recover_spin);
+	mutex_lock(&ls->ls_recover_mutex);
 	ls->ls_recover_block = ls->ls_recover_start;
 	set_bit(DFL_DLM_RECOVERY, &ls->ls_recover_flags);
 
 	if (!test_bit(DFL_MOUNT_DONE, &ls->ls_recover_flags) ||
 	     test_bit(DFL_FIRST_MOUNT, &ls->ls_recover_flags)) {
-		spin_unlock(&ls->ls_recover_spin);
+		mutex_unlock(&ls->ls_recover_mutex);
 		return;
 	}
 	set_bit(DFL_BLOCK_LOCKS, &ls->ls_recover_flags);
-	spin_unlock(&ls->ls_recover_spin);
+	mutex_unlock(&ls->ls_recover_mutex);
 }
 
 /* dlm calls after recover_prep has been completed on all lockspace members;
@@ -1090,11 +1090,11 @@ static void gdlm_recover_slot(void *arg, struct dlm_slot *slot)
 	struct lm_lockstruct *ls = &sdp->sd_lockstruct;
 	int jid = slot->slot - 1;
 
-	spin_lock(&ls->ls_recover_spin);
+	mutex_lock(&ls->ls_recover_mutex);
 	if (ls->ls_recover_size < jid + 1) {
 		fs_err(sdp, "recover_slot jid %d gen %u short size %d",
 		       jid, ls->ls_recover_block, ls->ls_recover_size);
-		spin_unlock(&ls->ls_recover_spin);
+		mutex_unlock(&ls->ls_recover_mutex);
 		return;
 	}
 
@@ -1103,7 +1103,7 @@ static void gdlm_recover_slot(void *arg, struct dlm_slot *slot)
 			jid, ls->ls_recover_block, ls->ls_recover_submit[jid]);
 	}
 	ls->ls_recover_submit[jid] = ls->ls_recover_block;
-	spin_unlock(&ls->ls_recover_spin);
+	mutex_unlock(&ls->ls_recover_mutex);
 }
 
 /* dlm calls after recover_slot and after it completes lock recovery */
@@ -1117,7 +1117,7 @@ static void gdlm_recover_done(void *arg, struct dlm_slot *slots, int num_slots,
 	/* ensure the ls jid arrays are large enough */
 	set_recover_size(sdp, slots, num_slots);
 
-	spin_lock(&ls->ls_recover_spin);
+	mutex_lock(&ls->ls_recover_mutex);
 	ls->ls_recover_start = generation;
 
 	if (!ls->ls_recover_mount) {
@@ -1131,7 +1131,7 @@ static void gdlm_recover_done(void *arg, struct dlm_slot *slots, int num_slots,
 	clear_bit(DFL_DLM_RECOVERY, &ls->ls_recover_flags);
 	smp_mb__after_atomic();
 	wake_up_bit(&ls->ls_recover_flags, DFL_DLM_RECOVERY);
-	spin_unlock(&ls->ls_recover_spin);
+	mutex_unlock(&ls->ls_recover_mutex);
 }
 
 /* gfs2_recover thread has a journal recovery result */
@@ -1148,15 +1148,15 @@ static void gdlm_recovery_result(struct gfs2_sbd *sdp, unsigned int jid,
 	if (jid == ls->ls_jid)
 		return;
 
-	spin_lock(&ls->ls_recover_spin);
+	mutex_lock(&ls->ls_recover_mutex);
 	if (test_bit(DFL_FIRST_MOUNT, &ls->ls_recover_flags)) {
-		spin_unlock(&ls->ls_recover_spin);
+		mutex_unlock(&ls->ls_recover_mutex);
 		return;
 	}
 	if (ls->ls_recover_size < jid + 1) {
 		fs_err(sdp, "recovery_result jid %d short size %d",
 		       jid, ls->ls_recover_size);
-		spin_unlock(&ls->ls_recover_spin);
+		mutex_unlock(&ls->ls_recover_mutex);
 		return;
 	}
 
@@ -1172,7 +1172,7 @@ static void gdlm_recovery_result(struct gfs2_sbd *sdp, unsigned int jid,
 	if (!test_bit(DFL_UNMOUNT, &ls->ls_recover_flags))
 		queue_delayed_work(gfs2_control_wq, &sdp->sd_control_work,
 				   result == LM_RD_GAVEUP ? HZ : 0);
-	spin_unlock(&ls->ls_recover_spin);
+	mutex_unlock(&ls->ls_recover_mutex);
 }
 
 const struct dlm_lockspace_ops gdlm_lockspace_ops = {
@@ -1194,7 +1194,7 @@ static int gdlm_mount(struct gfs2_sbd *sdp, const char *table)
 	 */
 
 	INIT_DELAYED_WORK(&sdp->sd_control_work, gfs2_control_func);
-	spin_lock_init(&ls->ls_recover_spin);
+	mutex_init(&ls->ls_recover_mutex);
 	ls->ls_recover_flags = 0;
 	ls->ls_recover_mount = 0;
 	ls->ls_recover_start = 0;
@@ -1300,9 +1300,9 @@ static void gdlm_unmount(struct gfs2_sbd *sdp)
 
 	/* wait for gfs2_control_wq to be done with this mount */
 
-	spin_lock(&ls->ls_recover_spin);
+	mutex_lock(&ls->ls_recover_mutex);
 	set_bit(DFL_UNMOUNT, &ls->ls_recover_flags);
-	spin_unlock(&ls->ls_recover_spin);
+	mutex_unlock(&ls->ls_recover_mutex);
 	flush_delayed_work(&sdp->sd_control_work);
 
 	/* mounted_lock and control_lock will be purged in dlm recovery */