[Cluster-devel] [GFS2 PATCH] [TRY #2] GFS2: Prevent recovery before the local journal is set

[Cluster-devel] [GFS2 PATCH] [TRY #2] GFS2: Prevent recovery before the local journal is set

[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

Signed-off-by: Bob Peterson <rpeterso@redhat.com> 
---
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index bdf70c1..04c062c 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -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;
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 22f9540..4535156 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -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,
diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
index de25d55..02afa82 100644
--- a/fs/gfs2/sys.c
+++ b/fs/gfs2/sys.c
@@ -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)

[Cluster-devel] [GFS2 PATCH] [TRY #2] GFS2: Prevent recovery before the local journal is set

[Steven Whitehouse]

Hi,

On 02/05/14 15:05, Bob Peterson wrote:
> 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.

> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
> diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> index bdf70c1..04c062c 100644
> --- a/fs/gfs2/incore.h
> +++ b/fs/gfs2/incore.h
> @@ -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;
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index 22f9540..4535156 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -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,
> diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
> index de25d55..02afa82 100644
> --- a/fs/gfs2/sys.c
> +++ b/fs/gfs2/sys.c
> @@ -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)
>

[Cluster-devel] [GFS2 PATCH] [TRY #3] GFS2: Prevent recovery before the local journal is set

[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 <rpeterso@redhat.com> 
---
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 2434a96..67d310c 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -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;
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index be45c79..75310a4 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -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:
diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
index 7bc17ed..0e049f9 100644
--- a/fs/gfs2/sys.c
+++ b/fs/gfs2/sys.c
@@ -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)

[Cluster-devel] [GFS2 PATCH] [TRY #3] GFS2: Prevent recovery before the local journal is set

[Steven Whitehouse]

Hi,

On 22/05/14 14:17, Bob Peterson wrote:
> 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 <rpeterso@redhat.com>
> ---
> diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> index 2434a96..67d310c 100644
> --- a/fs/gfs2/incore.h
> +++ b/fs/gfs2/incore.h
> @@ -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;
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index be45c79..75310a4 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -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. */
> +
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.

>   	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:
> diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
> index 7bc17ed..0e049f9 100644
> --- a/fs/gfs2/sys.c
> +++ b/fs/gfs2/sys.c
> @@ -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)
>

[Cluster-devel] [GFS2 PATCH] [TRY #4] GFS2: Prevent recovery before the local journal is set

[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 <rpeterso@redhat.com>
---
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 2434a96..67d310c 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -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;
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index be45c79..bc564c0 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -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:
diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
index 7bc17ed..0e049f9 100644
--- a/fs/gfs2/sys.c
+++ b/fs/gfs2/sys.c
@@ -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)

[Cluster-devel] [GFS2 PATCH] [TRY #4] GFS2: Prevent recovery before the local journal is set

[Steven Whitehouse]

Hi,

Now in the -nmw tree. Thanks,

Steve.

On 02/06/14 14:40, Bob Peterson wrote:
> 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 <rpeterso@redhat.com>
> ---
> diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> index 2434a96..67d310c 100644
> --- a/fs/gfs2/incore.h
> +++ b/fs/gfs2/incore.h
> @@ -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;
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index be45c79..bc564c0 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -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:
> diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
> index 7bc17ed..0e049f9 100644
> --- a/fs/gfs2/sys.c
> +++ b/fs/gfs2/sys.c
> @@ -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)
>