Skip to contentSkip to conversation

gfs2: don't hold the sd_jindex_spin during recovery

PATCH New Bob Peterson 2 participants 2 messages Aug 17, 2018 – Aug 20, 2018 1 message not in the archive GFS2 file system

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Bob Peterson

author
#1 lore raw

PATCH 1/1 gfs2: don't hold the sd_jindex_spin during recovery

Hi,

The sd_jindex_spin is used to serialize access to the sd_jindex_list. Before this patch function gfs2_recover_set would hold the spin_lock while recovery is running. Since recovery may take a very long time, other processes needing to use the list would monopolize a CPU for a very long time, spinning. This patch allows it to unlock the spin_lock before calling gfs2_recover_journal. The test_and_set_bit there should prevent multiple processes from trying to recover the same journal.

This is only a problem when multiple processes attempt recovery, which is possible via (1) a uevent kicking a 1 into the sysfs file /sys/fs/gfs2/<table>/lock_module/recover, while the gfs2_control_func in lock_dlm also calls gfs2_recover_set().

Signed-off-by Bob Peterson <[email protected]>
---
 fs/gfs2/sys.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
fs/gfs2/sys.c +2 -2
index 0c2a60fa66d7f..9fcb66d882b45 100644
@@ -424,8 +424,8 @@ int gfs2_recover_set(struct gfs2_sbd *sdp, unsigned jid)
 	list_for_each_entry(jd, &sdp->sd_jindex_list, jd_list) {
 		if (jd->jd_jid != jid && !sdp->sd_args.ar_spectator)
 			continue;
-		rv = gfs2_recover_journal(jd, false);
-		break;
+		spin_unlock(&sdp->sd_jindex_spin);
+		return gfs2_recover_journal(jd, false);
 	}
 out:
 	spin_unlock(&sdp->sd_jindex_spin);
#2 lore raw

PATCH 1/1 gfs2: don't hold the sd_jindex_spin during recovery

Hi,

Quoted 26 lines · Bob Peterson

Hi,

The sd_jindex_spin is used to serialize access to the sd_jindex_list. Before this patch function gfs2_recover_set would hold the spin_lock while recovery is running. Since recovery may take a very long time, other processes needing to use the list would monopolize a CPU for a very long time, spinning. This patch allows it to unlock the spin_lock before calling gfs2_recover_journal. The test_and_set_bit there should prevent multiple processes from trying to recover the same journal.

This is only a problem when multiple processes attempt recovery, which is possible via (1) a uevent kicking a 1 into the sysfs file /sys/fs/gfs2/<table>/lock_module/recover, while the gfs2_control_func in lock_dlm also calls gfs2_recover_set().

Signed-off-by Bob Peterson <[email protected]>
---
  fs/gfs2/sys.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
fs/gfs2/sys.c +0 -0
index 0c2a60fa66d7f..9fcb66d882b45 100644

fs/gfs2/sys.c int gfs2_recover_set(struct gfs2_sbd *sdp, unsigned jid) · line 428

Quoted 7 lines
fs/gfs2/sys.c +2 -2
@@ -424,8 +424,8 @@ int gfs2_recover_set(struct gfs2_sbd *sdp, unsigned jid)
  	list_for_each_entry(jd, &sdp->sd_jindex_list, jd_list) {
  		if (jd->jd_jid != jid && !sdp->sd_args.ar_spectator)
  			continue;
-		rv = gfs2_recover_journal(jd, false);
-		break;
+		spin_unlock(&sdp->sd_jindex_spin);
+		return gfs2_recover_journal(jd, false);

Since the wait parameter is false here, all gfs2_recover_journal does is queue some work, and that should not block. Also it breaks the locking between the JDF_RECOVERY flag and the queuing of the work too,

Steve.

Quoted 4 lines
fs/gfs2/sys.c +0 -0
  	}
  out:
  	spin_unlock(&sdp->sd_jindex_spin);