From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bob Peterson Date: Wed, 19 Apr 2017 14:52:39 -0400 (EDT) Subject: [Cluster-devel] [GFS2 PATCH] GFS2: Change recovery to use mutex instead of spin_lock In-Reply-To: <2144052809.17222072.1492627907851.JavaMail.zimbra@redhat.com> Message-ID: <1556412740.17222239.1492627959810.JavaMail.zimbra@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 --- 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 */