[Cluster-devel] [GFS2 PATCH 4/5] gfs2: special log flush sequence to protect jdata writes

[Bob Peterson <rpeterso@redhat.com>]

This patch implements Ben Marzinkski's idea of using two locks
as different layers of protection inside the gfs2_log_flush.
To quote Ben:

  You would need two locks, the regular log lock [sd_log_flush_lock],
  and the updating log lock [sd_log_updating_lock]. Calling
  gfs2_log_flush would grab and later release the updating log lock,
  just like it currently grabs and releases sd_log_flush_lock.

  Everything else that currently grabs sd_log_flush_lock, like
  gfs2_log_reserve(), would grab the regular log lock instead.
  (No changes if we define regular log lock == log_flush_lock.

  In a normal log flush, you would grab the regular log lock
  [log_flush_lock], call gfs2_log_flush, which would grab and
  later release the updating log lock, and after it returned,
  drop the regular log lock. In this case, the two locks would
  both lock basically the same code.

  For syncs [that] don't care if new data is written after the
  call to sync, you could do the same as rhel7 gfs2_meta_syncfs()
  [did in RHEL7], grabbing the regular log lock [gfs2_log_flush_lock]
  before each of the calls to gfs_log_flush(), and dropping after
  each return. Again, the two locks would basically lock the
  same code.

  For syncs where you need to be sure that no new data is coming
  in, like freezes and shutdowns, you need to grab the regular
  log lock, call gfs2_log_flush(), clean out the active items list
  like [rhel7's] gfs2_meta_syncfs() does, and then call
  gfs2_log_flush again, this time with the freeze or shutdown type.

  Only after both gfs2_log_flush() calls can you drop the regular
  log lock. This would mean that when you are writing out the
  active items in these syncing flushes, only the regular log lock
  would be held. This is the only time you will be holding one
  lock and not the other.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/incore.h     |  1 +
 fs/gfs2/log.c        | 77 ++++++++++++++++++++++++++++----------------
 fs/gfs2/ops_fstype.c |  1 +
 3 files changed, 52 insertions(+), 27 deletions(-)

diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 84a824293a78..129cf8582a0a 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -834,6 +834,7 @@ struct gfs2_sbd {
 	int sd_log_idle;
 
 	struct rw_semaphore sd_log_flush_lock;
+	struct rw_semaphore sd_log_updating_lock;
 	atomic_t sd_log_in_flight;
 	struct bio *sd_log_bio;
 	wait_queue_head_t sd_log_flush_wait;
diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 2abec43ae898..5371af3cd96c 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -876,6 +876,30 @@ static void ail_drain(struct gfs2_sbd *sdp)
 	spin_unlock(&sdp->sd_ail_lock);
 }
 
+static void gfs2_meta_syncfs(struct gfs2_sbd *sdp, u32 flags)
+{
+	if (!sdp->sd_log_idle) {
+		for (;;) {
+			gfs2_ail1_start(sdp);
+			gfs2_ail1_wait(sdp);
+			if (gfs2_ail1_empty(sdp, 0))
+				break;
+		}
+		if (gfs2_withdrawn(sdp))
+			return;
+		atomic_dec(&sdp->sd_log_blks_free); /* Adjust for unreserved
+						       buffer */
+		trace_gfs2_log_blocks(sdp, -1);
+		log_write_header(sdp, flags);
+		sdp->sd_log_head = sdp->sd_log_flush_head;
+	}
+	if (flags & (GFS2_LOG_HEAD_FLUSH_SHUTDOWN |
+		     GFS2_LOG_HEAD_FLUSH_FREEZE))
+		gfs2_log_shutdown(sdp);
+	if (flags & GFS2_LOG_HEAD_FLUSH_FREEZE)
+		atomic_set(&sdp->sd_freeze_state, SFS_FROZEN);
+}
+
 /**
  * gfs2_log_flush - flush incore transaction(s)
  * @sdp: the filesystem
@@ -884,13 +908,13 @@ static void ail_drain(struct gfs2_sbd *sdp)
  *
  */
 
-void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, u32 flags)
+static void __gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl,
+			     u32 flags)
 {
 	struct gfs2_trans *tr = NULL;
 	enum gfs2_freeze_state state = atomic_read(&sdp->sd_freeze_state);
 
-	down_write(&sdp->sd_log_flush_lock);
-
+	down_write(&sdp->sd_log_updating_lock);
 	/*
 	 * Do this check while holding the log_flush_lock to prevent new
 	 * buffers from being added to the ail via gfs2_pin()
@@ -900,7 +924,7 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, u32 flags)
 
 	/* Log might have been flushed while we waited for the flush lock */
 	if (gl && !test_bit(GLF_LFLUSH, &gl->gl_flags)) {
-		up_write(&sdp->sd_log_flush_lock);
+		up_write(&sdp->sd_log_updating_lock);
 		return;
 	}
 	trace_gfs2_log_flush(sdp, 1, flags);
@@ -963,28 +987,6 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, u32 flags)
 	spin_unlock(&sdp->sd_ail_lock);
 	gfs2_log_unlock(sdp);
 
-	if (!(flags & GFS2_LOG_HEAD_FLUSH_NORMAL)) {
-		if (!sdp->sd_log_idle) {
-			for (;;) {
-				gfs2_ail1_start(sdp);
-				gfs2_ail1_wait(sdp);
-				if (gfs2_ail1_empty(sdp, 0))
-					break;
-			}
-			if (gfs2_withdrawn(sdp))
-				goto out;
-			atomic_dec(&sdp->sd_log_blks_free); /* Adjust for unreserved buffer */
-			trace_gfs2_log_blocks(sdp, -1);
-			log_write_header(sdp, flags);
-			sdp->sd_log_head = sdp->sd_log_flush_head;
-		}
-		if (flags & (GFS2_LOG_HEAD_FLUSH_SHUTDOWN |
-			     GFS2_LOG_HEAD_FLUSH_FREEZE))
-			gfs2_log_shutdown(sdp);
-		if (flags & GFS2_LOG_HEAD_FLUSH_FREEZE)
-			atomic_set(&sdp->sd_freeze_state, SFS_FROZEN);
-	}
-
 out:
 	if (gfs2_withdrawn(sdp)) {
 		ail_drain(sdp); /* frees all transactions */
@@ -992,11 +994,32 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, u32 flags)
 	}
 
 	trace_gfs2_log_flush(sdp, 0, flags);
-	up_write(&sdp->sd_log_flush_lock);
+	up_write(&sdp->sd_log_updating_lock);
 
 	kfree(tr);
 }
 
+void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, u32 flags)
+{
+	u32 normal = 0;
+
+	down_write(&sdp->sd_log_flush_lock);
+	if (flags & GFS2_LOG_HEAD_FLUSH_NORMAL)
+		goto normal_flush;
+
+	normal = flags & ~(GFS2_LOG_HEAD_FLUSH_SHUTDOWN |
+			   GFS2_LOG_HEAD_FLUSH_FREEZE |
+			   GFS2_LOG_HEAD_FLUSH_SYNC);
+	normal |= GFS2_LOG_HEAD_FLUSH_NORMAL;
+	__gfs2_log_flush(sdp, NULL, normal);
+
+	gfs2_meta_syncfs(sdp, flags);
+
+normal_flush:
+	__gfs2_log_flush(sdp, gl, flags);
+	up_write(&sdp->sd_log_flush_lock);
+}
+
 /**
  * gfs2_merge_trans - Merge a new transaction into a cached transaction
  * @old: Original transaction to be expanded
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 1aeb591bfd28..11a3a471124c 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -133,6 +133,7 @@ static struct gfs2_sbd *init_sbd(struct super_block *sb)
 	INIT_LIST_HEAD(&sdp->sd_ail2_list);
 
 	init_rwsem(&sdp->sd_log_flush_lock);
+	init_rwsem(&sdp->sd_log_updating_lock);
 	atomic_set(&sdp->sd_log_in_flight, 0);
 	atomic_set(&sdp->sd_reserving_log, 0);
 	init_waitqueue_head(&sdp->sd_reserving_log_wait);
-- 
2.25.1