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

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

[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.

Regards,

Bob Peterson
Red Hat File Systems

Signed-off-by: Bob Peterson <rpeterso@redhat.com> 
---
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index fba74a2..b5e844f 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);
+	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,
diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
index de25d55..73bd32e 100644
--- a/fs/gfs2/sys.c
+++ b/fs/gfs2/sys.c
@@ -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;
 }
 

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

[Steven Whitehouse]

Hi,

On Tue, 2014-03-25 at 11:54 -0400, Bob Peterson wrote:
> 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 <rpeterso@redhat.com> 

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.

> ---
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index fba74a2..b5e844f 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);
> +	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,
> diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
> index de25d55..73bd32e 100644
> --- a/fs/gfs2/sys.c
> +++ b/fs/gfs2/sys.c
> @@ -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;
>  }
>  
>