From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Thu, 22 May 2014 14:32:11 +0100 Subject: [Cluster-devel] [GFS2 PATCH] [TRY #3] GFS2: Prevent recovery before the local journal is set In-Reply-To: <1282622671.10155260.1400764638562.JavaMail.zimbra@redhat.com> References: <256959012.14056298.1399039517312.JavaMail.zimbra@redhat.com> <1282622671.10155260.1400764638562.JavaMail.zimbra@redhat.com> Message-ID: <537DFC5B.0@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 > --- > 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) >