GFS2: Prevent recovery before the local journal is set

PATCH New Bob Peterson 2 participants 2 messages Mar 25, 2014 – Mar 26, 2014 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

2014-03-25 15:54 UTC #1 lore raw

PATCH 1/1 GFS2: Prevent recovery before the local journal is set

Hi,

This patch repurposes the (formerly useless) sd_jindex_mutex lock to prevent dlm's recover process from referencing and trying to recover a journal before a journal has been opened.

Regards,

Bob Peterson
Red Hat File Systems

Signed-off-by Bob Peterson <[email protected]>

---

fs/gfs2/ops_fstype.c +4 -4

index fba74a2..b5e844f 100644
@@ -94,6 +94,7 @@ static struct gfs2_sbd *init_sbd(struct super_block *sb)
 	INIT_LIST_HEAD(&sdp->sd_jindex_list);
 	spin_lock_init(&sdp->sd_jindex_spin);
 	mutex_init(&sdp->sd_jindex_mutex);
+	mutex_lock(&sdp->sd_jindex_mutex);
 
 	INIT_LIST_HEAD(&sdp->sd_quota_list);
 	mutex_init(&sdp->sd_quota_mutex);

@@ -548,8 +549,6 @@ static int gfs2_jindex_hold(struct gfs2_sbd *sdp, struct gfs2_holder *ji_gh)
 
 	name.name = buf;
 
-	mutex_lock(&sdp->sd_jindex_mutex);
-
 	for (;;) {
 		error = gfs2_glock_nq_init(dip->i_gl, LM_ST_SHARED, 0, ji_gh);
 		if (error)

@@ -594,8 +593,6 @@ static int gfs2_jindex_hold(struct gfs2_sbd *sdp, struct gfs2_holder *ji_gh)
 		spin_unlock(&sdp->sd_jindex_spin);
 	}
 
-	mutex_unlock(&sdp->sd_jindex_mutex);
-
 	return error;
 }

@@ -676,6 +673,7 @@ static int init_journal(struct gfs2_sbd *sdp, int undo)
 
 	if (sdp->sd_args.ar_spectator) {
 		sdp->sd_jdesc = gfs2_jdesc_find(sdp, 0);
+		mutex_unlock(&sdp->sd_jindex_mutex);
 		atomic_set(&sdp->sd_log_blks_free, sdp->sd_jdesc->jd_blocks);
 		atomic_set(&sdp->sd_log_thresh1, 2*sdp->sd_jdesc->jd_blocks/5);
 		atomic_set(&sdp->sd_log_thresh2, 4*sdp->sd_jdesc->jd_blocks/5);

@@ -686,9 +684,11 @@ static int init_journal(struct gfs2_sbd *sdp, int undo)
 			fs_err(sdp, "there are only %u journals (0 - %u)\n",
 			       gfs2_jindex_size(sdp),
 			       gfs2_jindex_size(sdp) - 1);
+			mutex_unlock(&sdp->sd_jindex_mutex);
 			goto fail_jindex;
 		}
 		sdp->sd_jdesc = gfs2_jdesc_find(sdp, sdp->sd_lockstruct.ls_jid);
+		mutex_unlock(&sdp->sd_jindex_mutex);
 
 		error = gfs2_glock_nq_num(sdp, sdp->sd_lockstruct.ls_jid,
 					  &gfs2_journal_glops,

fs/gfs2/sys.c +2 -0

index de25d55..73bd32e 100644
@@ -407,6 +407,7 @@ int gfs2_recover_set(struct gfs2_sbd *sdp, unsigned jid)
 	struct gfs2_jdesc *jd;
 	int rv;
 
+	mutex_lock(&sdp->sd_jindex_mutex);
 	spin_lock(&sdp->sd_jindex_spin);
 	rv = -EBUSY;
 	if (sdp->sd_jdesc->jd_jid == jid)

@@ -420,6 +421,7 @@ int gfs2_recover_set(struct gfs2_sbd *sdp, unsigned jid)
 	}
 out:
 	spin_unlock(&sdp->sd_jindex_spin);
+	mutex_unlock(&sdp->sd_jindex_mutex);
 	return rv;
 }

Steven Whitehouse

2014-03-26 09:08 UTC #2 lore raw

PATCH 1/1 GFS2: Prevent recovery before the local journal is set

Hi,

Quoted 6 lines · Bob Peterson

Hi,
This patch repurposes the (formerly useless) sd_jindex_mutex lock to prevent dlm's recover process from referencing and trying to recover a journal before a journal has been opened.

Quoted 6 lines

Regards,
Bob Peterson
Red Hat File Systems
Signed-off-by Bob Peterson <[email protected]>

I'm not quite sure that this is the right fix... what appears to be needed is waiting for all of the journals to be initialised rather than just blocking out recovery requests while we read them in. So I wonder whether using a completion (which would be completed at "others may mount" time) would not be better here?

Steve.

Quoted 72 lines

---

fs/gfs2/ops_fstype.c +4 -4

index fba74a2..b5e844f 100644
@@ -94,6 +94,7 @@ static struct gfs2_sbd *init_sbd(struct super_block *sb)
 	INIT_LIST_HEAD(&sdp->sd_jindex_list);
 	spin_lock_init(&sdp->sd_jindex_spin);
 	mutex_init(&sdp->sd_jindex_mutex);
+	mutex_lock(&sdp->sd_jindex_mutex);
 
 	INIT_LIST_HEAD(&sdp->sd_quota_list);
 	mutex_init(&sdp->sd_quota_mutex);

@@ -548,8 +549,6 @@ static int gfs2_jindex_hold(struct gfs2_sbd *sdp, struct gfs2_holder *ji_gh)
 
 	name.name = buf;
 
-	mutex_lock(&sdp->sd_jindex_mutex);
-
 	for (;;) {
 		error = gfs2_glock_nq_init(dip->i_gl, LM_ST_SHARED, 0, ji_gh);
 		if (error)

@@ -594,8 +593,6 @@ static int gfs2_jindex_hold(struct gfs2_sbd *sdp, struct gfs2_holder *ji_gh)
 		spin_unlock(&sdp->sd_jindex_spin);
 	}
 
-	mutex_unlock(&sdp->sd_jindex_mutex);
-
 	return error;
 }

@@ -676,6 +673,7 @@ static int init_journal(struct gfs2_sbd *sdp, int undo)
 
 	if (sdp->sd_args.ar_spectator) {
 		sdp->sd_jdesc = gfs2_jdesc_find(sdp, 0);
+		mutex_unlock(&sdp->sd_jindex_mutex);
 		atomic_set(&sdp->sd_log_blks_free, sdp->sd_jdesc->jd_blocks);
 		atomic_set(&sdp->sd_log_thresh1, 2*sdp->sd_jdesc->jd_blocks/5);
 		atomic_set(&sdp->sd_log_thresh2, 4*sdp->sd_jdesc->jd_blocks/5);

@@ -686,9 +684,11 @@ static int init_journal(struct gfs2_sbd *sdp, int undo)
 			fs_err(sdp, "there are only %u journals (0 - %u)\n",
 			       gfs2_jindex_size(sdp),
 			       gfs2_jindex_size(sdp) - 1);
+			mutex_unlock(&sdp->sd_jindex_mutex);
 			goto fail_jindex;
 		}
 		sdp->sd_jdesc = gfs2_jdesc_find(sdp, sdp->sd_lockstruct.ls_jid);
+		mutex_unlock(&sdp->sd_jindex_mutex);
 
 		error = gfs2_glock_nq_num(sdp, sdp->sd_lockstruct.ls_jid,
 					  &gfs2_journal_glops,

fs/gfs2/sys.c +2 -0

index de25d55..73bd32e 100644
@@ -407,6 +407,7 @@ int gfs2_recover_set(struct gfs2_sbd *sdp, unsigned jid)
 	struct gfs2_jdesc *jd;
 	int rv;
 
+	mutex_lock(&sdp->sd_jindex_mutex);
 	spin_lock(&sdp->sd_jindex_spin);
 	rv = -EBUSY;
 	if (sdp->sd_jdesc->jd_jid == jid)

@@ -420,6 +421,7 @@ int gfs2_recover_set(struct gfs2_sbd *sdp, unsigned jid)
 	}
 out:
 	spin_unlock(&sdp->sd_jindex_spin);
+	mutex_unlock(&sdp->sd_jindex_mutex);
 	return rv;
 }