[Cluster-devel] [GFS2 PATCH V2] gfs2: clean_journal was setting sd_log_flush_head replaying other journals

[Cluster-devel] [GFS2 PATCH V2] gfs2: clean_journal was setting sd_log_flush_head replaying other journals

[Bob Peterson]

Hi,

Yesterday I posted a patch for this problem, but it was grossly inadequate.
This patch, version 2, is another attempt to fix it. Many thanks to Ross
Lagerwall for helping us identify, fix, and test the problem.

Regards,

Bob Peterson
---
gfs2: clean_journal was setting sd_log_flush_head replaying other journals

Function clean_journal was setting the value of sd_log_flush_head,
but that's only a valid thing to do if it is replaying its own journal.
If it's replaying another node's journal, that's completely wrong and
will lead to multiple problems. This patch tries to clean up the
mess by passing the value of head to gfs2_write_log_header so it can
treat non-owned journals fairly.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/log.c      | 12 ++++++++----
 fs/gfs2/log.h      |  3 ++-
 fs/gfs2/recovery.c |  6 +++---
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 64e435cedb32..39be1160f056 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -660,7 +660,8 @@ void gfs2_write_revokes(struct gfs2_sbd *sdp)
  */
 
 void gfs2_write_log_header(struct gfs2_sbd *sdp, struct gfs2_jdesc *jd,
-			   u64 seq, u32 tail, u32 flags, int op_flags)
+			   u64 seq, u32 tail, u32 flags, unsigned int head,
+			   int op_flags)
 {
 	struct gfs2_log_header *lh;
 	u32 hash, crc;
@@ -685,14 +686,17 @@ void gfs2_write_log_header(struct gfs2_sbd *sdp, struct gfs2_jdesc *jd,
 	lh->lh_sequence = cpu_to_be64(seq);
 	lh->lh_flags = cpu_to_be32(flags);
 	lh->lh_tail = cpu_to_be32(tail);
-	lh->lh_blkno = cpu_to_be32(sdp->sd_log_flush_head);
+	lh->lh_blkno = cpu_to_be32(head);
 	hash = ~crc32(~0, lh, LH_V1_SIZE);
 	lh->lh_hash = cpu_to_be32(hash);
 
 	ktime_get_coarse_real_ts64(&tv);
 	lh->lh_nsec = cpu_to_be32(tv.tv_nsec);
 	lh->lh_sec = cpu_to_be64(tv.tv_sec);
-	addr = gfs2_log_bmap(sdp);
+	if (jd->jd_jid == sdp->sd_lockstruct.ls_jid)
+		addr = gfs2_log_bmap(sdp);
+	else
+		addr = 0;
 	lh->lh_addr = cpu_to_be64(addr);
 	lh->lh_jinode = cpu_to_be64(GFS2_I(jd->jd_inode)->i_no_addr);
 
@@ -748,7 +752,7 @@ static void log_write_header(struct gfs2_sbd *sdp, u32 flags)
 	}
 	sdp->sd_log_idle = (tail == sdp->sd_log_flush_head);
 	gfs2_write_log_header(sdp, sdp->sd_jdesc, sdp->sd_log_sequence++, tail,
-			      flags, op_flags);
+			      flags, sdp->sd_log_flush_head, op_flags);
 
 	if (sdp->sd_log_tail != tail)
 		log_pull_tail(sdp, tail);
diff --git a/fs/gfs2/log.h b/fs/gfs2/log.h
index bd2d08d0f21c..bb5d934197dd 100644
--- a/fs/gfs2/log.h
+++ b/fs/gfs2/log.h
@@ -70,7 +70,8 @@ extern unsigned int gfs2_struct2blk(struct gfs2_sbd *sdp, unsigned int nstruct,
 extern void gfs2_log_release(struct gfs2_sbd *sdp, unsigned int blks);
 extern int gfs2_log_reserve(struct gfs2_sbd *sdp, unsigned int blks);
 extern void gfs2_write_log_header(struct gfs2_sbd *sdp, struct gfs2_jdesc *jd,
-				  u64 seq, u32 tail, u32 flags, int op_flags);
+				  u64 seq, u32 tail, u32 flags,
+				  unsigned int head, int op_flags);
 extern void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl,
 			   u32 type);
 extern void gfs2_log_commit(struct gfs2_sbd *sdp, struct gfs2_trans *trans);
diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
index 020cac0e1002..170e662e1076 100644
--- a/fs/gfs2/recovery.c
+++ b/fs/gfs2/recovery.c
@@ -386,10 +386,10 @@ static void clean_journal(struct gfs2_jdesc *jd,
 			  struct gfs2_log_header_host *head)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
+	unsigned int lblock = head->lh_blkno;
 
-	sdp->sd_log_flush_head = head->lh_blkno;
-	gfs2_replay_incr_blk(jd, &sdp->sd_log_flush_head);
-	gfs2_write_log_header(sdp, jd, head->lh_sequence + 1, 0,
+	gfs2_replay_incr_blk(jd, &lblock);
+	gfs2_write_log_header(sdp, jd, head->lh_sequence + 1, 0, lblock,
 			      GFS2_LOG_HEAD_UNMOUNT | GFS2_LOG_HEAD_RECOVERY,
 			      REQ_PREFLUSH | REQ_FUA | REQ_META | REQ_SYNC);
 }

[Cluster-devel] [GFS2 PATCH V2] gfs2: clean_journal was setting sd_log_flush_head replaying other journals

[Ross Lagerwall]

On 3/27/19 5:35 PM, Bob Peterson wrote:
> Hi,
> 
> Yesterday I posted a patch for this problem, but it was grossly inadequate.
> This patch, version 2, is another attempt to fix it. Many thanks to Ross
> Lagerwall for helping us identify, fix, and test the problem.
> 
> Regards,
> 
> Bob Peterson
> ---
> gfs2: clean_journal was setting sd_log_flush_head replaying other journals
> 
> Function clean_journal was setting the value of sd_log_flush_head,
> but that's only a valid thing to do if it is replaying its own journal.
> If it's replaying another node's journal, that's completely wrong and
> will lead to multiple problems. This patch tries to clean up the
> mess by passing the value of head to gfs2_write_log_header so it can
> treat non-owned journals fairly.
> 
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
This does seem to fix the issue. Thanks!

Tested-by: Ross Lagerwall <ross.lagerwall@citrix.com>