Skip to contentSkip to conversation

GFS2: Prevent recovery before the local journal is set

PATCH New Bob Peterson 2 participants 6 messages May 2, 2014 – Jun 2, 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
#1 lore raw

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

Hi,

This patch uses a completion to prevent dlm's recovery 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/incore.h +2 -0
index bdf70c1..04c062c 100644
@@ -730,6 +730,8 @@ struct gfs2_sbd {
 	struct gfs2_holder sd_sc_gh;
 	struct gfs2_holder sd_qc_gh;
 
+	struct completion sd_journal_ready;
+
 	/* Daemon stuff */
 
 	struct task_struct *sd_logd_process;
fs/gfs2/ops_fstype.c +5 -0
index 22f9540..4535156 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);
+	init_completion(&sdp->sd_journal_ready);
 
 	INIT_LIST_HEAD(&sdp->sd_quota_list);
 	mutex_init(&sdp->sd_quota_mutex);

@@ -676,6 +677,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);
+		complete_all(&sdp->sd_journal_ready);
 		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,10 +688,13 @@ 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);
+			complete_all(&sdp->sd_journal_ready);
 			goto fail_jindex;
 		}
 		sdp->sd_jdesc = gfs2_jdesc_find(sdp, sdp->sd_lockstruct.ls_jid);
 
+		complete_all(&sdp->sd_journal_ready);
+
 		error = gfs2_glock_nq_num(sdp, sdp->sd_lockstruct.ls_jid,
 					  &gfs2_journal_glops,
 					  LM_ST_EXCLUSIVE, LM_FLAG_NOEXP,
fs/gfs2/sys.c +3 -0
index de25d55..02afa82 100644
@@ -407,6 +407,9 @@ int gfs2_recover_set(struct gfs2_sbd *sdp, unsigned jid)
 	struct gfs2_jdesc *jd;
 	int rv;
 
+	/* Wait for our primary journal to be initialized */
+	wait_for_completion(&sdp->sd_journal_ready);
+
 	spin_lock(&sdp->sd_jindex_spin);
 	rv = -EBUSY;
 	if (sdp->sd_jdesc->jd_jid == jid)
#2 lore raw

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

Hi,

Quoted 10 lines · Bob Peterson

Hi,

This patch uses a completion to prevent dlm's recovery process from referencing and trying to recover a journal before a journal has been opened.

Regards,

Bob Peterson
Red Hat File Systems

There is one possible issue that I can see, which is the error case. If we have some error that occurs prior to getting to the complete_all() will we land up with a process that has requested recovery waiting forever? If we can resolve that, then I think this should be good to go,

Steve.

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

---

fs/gfs2/incore.h +2 -0
index bdf70c1..04c062c 100644
@@ -730,6 +730,8 @@ struct gfs2_sbd {
  	struct gfs2_holder sd_sc_gh;
  	struct gfs2_holder sd_qc_gh;
 
+	struct completion sd_journal_ready;
+
  	/* Daemon stuff */
 
  	struct task_struct *sd_logd_process;
fs/gfs2/ops_fstype.c +5 -0
index 22f9540..4535156 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);
+	init_completion(&sdp->sd_journal_ready);
 
  	INIT_LIST_HEAD(&sdp->sd_quota_list);
  	mutex_init(&sdp->sd_quota_mutex);

@@ -676,6 +677,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);
+		complete_all(&sdp->sd_journal_ready);
  		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,10 +688,13 @@ 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);
+			complete_all(&sdp->sd_journal_ready);
  			goto fail_jindex;
  		}
  		sdp->sd_jdesc = gfs2_jdesc_find(sdp, sdp->sd_lockstruct.ls_jid);
 
+		complete_all(&sdp->sd_journal_ready);
+
  		error = gfs2_glock_nq_num(sdp, sdp->sd_lockstruct.ls_jid,
  					  &gfs2_journal_glops,
  					  LM_ST_EXCLUSIVE, LM_FLAG_NOEXP,
fs/gfs2/sys.c +3 -0
index de25d55..02afa82 100644
@@ -407,6 +407,9 @@ int gfs2_recover_set(struct gfs2_sbd *sdp, unsigned jid)
  	struct gfs2_jdesc *jd;
  	int rv;
 
+	/* Wait for our primary journal to be initialized */
+	wait_for_completion(&sdp->sd_journal_ready);
+
  	spin_lock(&sdp->sd_jindex_spin);
  	rv = -EBUSY;
  	if (sdp->sd_jdesc->jd_jid == jid)
author
#3 lore raw

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

In reply to Bob Peterson

Hi,

This is my third attempt at a recovery patch that prevents recovery before the journal is set by GFS2. Steve Whitehouse pointed out that there were error paths in the code that could leave recovery permanently hung, waiting for the completion. It turns out there were lots of error paths with this problem, which prompted me to completely rewrite the patch. This version keeps track of when there might possibly be waiters for the completion, and if so, the error path does a complete_all.

Patch description:

This patch uses a completion to prevent dlm's recovery 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/incore.h +2 -0
index 2434a96..67d310c 100644
@@ -728,6 +728,8 @@ struct gfs2_sbd {
 	struct gfs2_holder sd_sc_gh;
 	struct gfs2_holder sd_qc_gh;
 
+	struct completion sd_journal_ready;
+
 	/* Daemon stuff */
 
 	struct task_struct *sd_logd_process;
fs/gfs2/ops_fstype.c +12 -0
index be45c79..75310a4 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);
+	init_completion(&sdp->sd_journal_ready);
 
 	INIT_LIST_HEAD(&sdp->sd_quota_list);
 	mutex_init(&sdp->sd_quota_mutex);

@@ -796,6 +797,7 @@ static int init_inodes(struct gfs2_sbd *sdp, int undo)
 		goto fail_qinode;
 
 	error = init_journal(sdp, undo);
+	complete_all(&sdp->sd_journal_ready);
 	if (error)
 		goto fail;
 

@@ -1063,6 +1065,7 @@ static int fill_super(struct super_block *sb, struct gfs2_args *args, int silent
 	struct gfs2_sbd *sdp;
 	struct gfs2_holder mount_gh;
 	int error;
+	int poss_j_waiters = 0; /* Possible journal completion waiters */
 
 	sdp = init_sbd(sb);
 	if (!sdp) {

@@ -1138,6 +1141,9 @@ static int fill_super(struct super_block *sb, struct gfs2_args *args, int silent
 	if (error)
 		goto fail_debug;
 
+	poss_j_waiters = 1; /* After this point, we could have lock_dlm
+			       waiting for our sd_journal_ready completion. */
+
 	error = init_locking(sdp, &mount_gh, DO);
 	if (error)
 		goto fail_lm;

@@ -1171,6 +1177,10 @@ static int fill_super(struct super_block *sb, struct gfs2_args *args, int silent
 		snprintf(sdp->sd_fsname, GFS2_FSNAME_LEN, "%s.%u",
 			 sdp->sd_table_name, sdp->sd_lockstruct.ls_jid);
 
+	poss_j_waiters = 0; /* init_inodes calls init_journal which satisfies
+			       the sd_journal_ready completion, regardless
+			       of whether it's successful. */
+
 	error = init_inodes(sdp, DO);
 	if (error)
 		goto fail_sb;

@@ -1212,6 +1222,8 @@ fail_sb:
 fail_locking:
 	init_locking(sdp, &mount_gh, UNDO);
 fail_lm:
+	if (poss_j_waiters)
+		complete_all(&sdp->sd_journal_ready);
 	gfs2_gl_hash_clear(sdp);
 	gfs2_lm_unmount(sdp);
 fail_debug:
fs/gfs2/sys.c +3 -0
index 7bc17ed..0e049f9 100644
@@ -407,6 +407,9 @@ int gfs2_recover_set(struct gfs2_sbd *sdp, unsigned jid)
 	struct gfs2_jdesc *jd;
 	int rv;
 
+	/* Wait for our primary journal to be initialized */
+	wait_for_completion(&sdp->sd_journal_ready);
+
 	spin_lock(&sdp->sd_jindex_spin);
 	rv = -EBUSY;
 	if (sdp->sd_jdesc->jd_jid == jid)
#4 lore raw

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

Hi,

Quoted 65 lines · Bob Peterson

Hi,

This is my third attempt at a recovery patch that prevents recovery before the journal is set by GFS2. Steve Whitehouse pointed out that there were error paths in the code that could leave recovery permanently hung, waiting for the completion. It turns out there were lots of error paths with this problem, which prompted me to completely rewrite the patch. This version keeps track of when there might possibly be waiters for the completion, and if so, the error path does a complete_all.

Patch description:

This patch uses a completion to prevent dlm's recovery 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/incore.h +2 -0
index 2434a96..67d310c 100644
@@ -728,6 +728,8 @@ struct gfs2_sbd {
  	struct gfs2_holder sd_sc_gh;
  	struct gfs2_holder sd_qc_gh;
 
+	struct completion sd_journal_ready;
+
  	/* Daemon stuff */
 
  	struct task_struct *sd_logd_process;
fs/gfs2/ops_fstype.c +3 -0
index be45c79..75310a4 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);
+	init_completion(&sdp->sd_journal_ready);
 
  	INIT_LIST_HEAD(&sdp->sd_quota_list);
  	mutex_init(&sdp->sd_quota_mutex);

@@ -796,6 +797,7 @@ static int init_inodes(struct gfs2_sbd *sdp, int undo)
  		goto fail_qinode;
 
  	error = init_journal(sdp, undo);
+	complete_all(&sdp->sd_journal_ready);
  	if (error)
  		goto fail;
 

@@ -1063,6 +1065,7 @@ static int fill_super(struct super_block *sb, struct gfs2_args *args, int silent
  	struct gfs2_sbd *sdp;
  	struct gfs2_holder mount_gh;
  	int error;
+	int poss_j_waiters = 0; /* Possible journal completion waiters */
 
  	sdp = init_sbd(sb);
  	if (!sdp) {

fs/gfs2/ops_fstype.c static int fill_super(struct super_block *sb, struct gfs2_args *args, int silent · line 1146

Quoted 6 lines
fs/gfs2/ops_fstype.c +3 -0
@@ -1138,6 +1141,9 @@ static int fill_super(struct super_block *sb, struct gfs2_args *args, int silent
  	if (error)
  		goto fail_debug;
 
+	poss_j_waiters = 1; /* After this point, we could have lock_dlm
+			       waiting for our sd_journal_ready completion. */
+

I don't think this is true... as soon as sysfs is set up (before this point) there can be processes waiting on the completion. Also, I think you can just call the complete_all() unconditionally at fail_lm time, so that the poss_j_waiters thing is not really needed I think. Otherwise it looks good though,

Steve.

Quoted 38 lines
fs/gfs2/ops_fstype.c +6 -0
  	error = init_locking(sdp, &mount_gh, DO);
  	if (error)
  		goto fail_lm;

@@ -1171,6 +1177,10 @@ static int fill_super(struct super_block *sb, struct gfs2_args *args, int silent
  		snprintf(sdp->sd_fsname, GFS2_FSNAME_LEN, "%s.%u",
  			 sdp->sd_table_name, sdp->sd_lockstruct.ls_jid);
 
+	poss_j_waiters = 0; /* init_inodes calls init_journal which satisfies
+			       the sd_journal_ready completion, regardless
+			       of whether it's successful. */
+
  	error = init_inodes(sdp, DO);
  	if (error)
  		goto fail_sb;

@@ -1212,6 +1222,8 @@ fail_sb:
  fail_locking:
  	init_locking(sdp, &mount_gh, UNDO);
  fail_lm:
+	if (poss_j_waiters)
+		complete_all(&sdp->sd_journal_ready);
  	gfs2_gl_hash_clear(sdp);
  	gfs2_lm_unmount(sdp);
  fail_debug:
fs/gfs2/sys.c +3 -0
index 7bc17ed..0e049f9 100644
@@ -407,6 +407,9 @@ int gfs2_recover_set(struct gfs2_sbd *sdp, unsigned jid)
  	struct gfs2_jdesc *jd;
  	int rv;
 
+	/* Wait for our primary journal to be initialized */
+	wait_for_completion(&sdp->sd_journal_ready);
+
  	spin_lock(&sdp->sd_jindex_spin);
  	rv = -EBUSY;
  	if (sdp->sd_jdesc->jd_jid == jid)
author
#5 lore raw

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

In reply to Bob Peterson

Hi,

This is my fourth attempt at a recovery patch that prevents recovery before the journal is set by GFS2.

Patch description:

This patch uses a completion to prevent dlm's recovery 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/incore.h +2 -0
index 2434a96..67d310c 100644
@@ -728,6 +728,8 @@ struct gfs2_sbd {
 	struct gfs2_holder sd_sc_gh;
 	struct gfs2_holder sd_qc_gh;
 
+	struct completion sd_journal_ready;
+
 	/* Daemon stuff */
 
 	struct task_struct *sd_logd_process;
fs/gfs2/ops_fstype.c +3 -0
index be45c79..bc564c0 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);
+	init_completion(&sdp->sd_journal_ready);
 
 	INIT_LIST_HEAD(&sdp->sd_quota_list);
 	mutex_init(&sdp->sd_quota_mutex);

@@ -796,6 +797,7 @@ static int init_inodes(struct gfs2_sbd *sdp, int undo)
 		goto fail_qinode;
 
 	error = init_journal(sdp, undo);
+	complete_all(&sdp->sd_journal_ready);
 	if (error)
 		goto fail;
 

@@ -1212,6 +1214,7 @@ fail_sb:
 fail_locking:
 	init_locking(sdp, &mount_gh, UNDO);
 fail_lm:
+	complete_all(&sdp->sd_journal_ready);
 	gfs2_gl_hash_clear(sdp);
 	gfs2_lm_unmount(sdp);
 fail_debug:
fs/gfs2/sys.c +3 -0
index 7bc17ed..0e049f9 100644
@@ -407,6 +407,9 @@ int gfs2_recover_set(struct gfs2_sbd *sdp, unsigned jid)
 	struct gfs2_jdesc *jd;
 	int rv;
 
+	/* Wait for our primary journal to be initialized */
+	wait_for_completion(&sdp->sd_journal_ready);
+
 	spin_lock(&sdp->sd_jindex_spin);
 	rv = -EBUSY;
 	if (sdp->sd_jdesc->jd_jid == jid)
#6 lore raw

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

Hi,

Now in the -nmw tree. Thanks,

Steve.

Quoted 74 lines · Bob Peterson

Hi,

This is my fourth attempt at a recovery patch that prevents recovery before the journal is set by GFS2.

Patch description:

This patch uses a completion to prevent dlm's recovery 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/incore.h +2 -0
index 2434a96..67d310c 100644
@@ -728,6 +728,8 @@ struct gfs2_sbd {
  	struct gfs2_holder sd_sc_gh;
  	struct gfs2_holder sd_qc_gh;
 
+	struct completion sd_journal_ready;
+
  	/* Daemon stuff */
 
  	struct task_struct *sd_logd_process;
fs/gfs2/ops_fstype.c +3 -0
index be45c79..bc564c0 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);
+	init_completion(&sdp->sd_journal_ready);
 
  	INIT_LIST_HEAD(&sdp->sd_quota_list);
  	mutex_init(&sdp->sd_quota_mutex);

@@ -796,6 +797,7 @@ static int init_inodes(struct gfs2_sbd *sdp, int undo)
  		goto fail_qinode;
 
  	error = init_journal(sdp, undo);
+	complete_all(&sdp->sd_journal_ready);
  	if (error)
  		goto fail;
 

@@ -1212,6 +1214,7 @@ fail_sb:
  fail_locking:
  	init_locking(sdp, &mount_gh, UNDO);
  fail_lm:
+	complete_all(&sdp->sd_journal_ready);
  	gfs2_gl_hash_clear(sdp);
  	gfs2_lm_unmount(sdp);
  fail_debug:
fs/gfs2/sys.c +3 -0
index 7bc17ed..0e049f9 100644
@@ -407,6 +407,9 @@ int gfs2_recover_set(struct gfs2_sbd *sdp, unsigned jid)
  	struct gfs2_jdesc *jd;
  	int rv;
 
+	/* Wait for our primary journal to be initialized */
+	wait_for_completion(&sdp->sd_journal_ready);
+
  	spin_lock(&sdp->sd_jindex_spin);
  	rv = -EBUSY;
  	if (sdp->sd_jdesc->jd_jid == jid)