[Cluster-devel] [PATCH] gfs2: prevent gfs2_logd spinning

[Cluster-devel] [PATCH] gfs2: prevent gfs2_logd spinning

[Bob Peterson]

Before this patch if the ail list was busy due to slow hardware, and the
number of free blocks was insufficient for a transaction the logd daemon
could loop hundreds of thousands of times, each making no progress.
This robbed the system of cpu time and did massive amounts of locking
and unlocking of the ail list spinlock (sd_ail_lock) which made it
harder to make progress flushing the log to free up the necessary space
in the journal.

This patch checks to see if any progress has been made on processing the
ail list. If all the ail items are busy (being written) and no progress
has been made, the logd daemon sleeps for one second (rather than the
normal 30 seconds). As before, log flushes override this behavior.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/log.c | 42 ++++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index aa568796207c..df626cdc7093 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -298,12 +298,13 @@ static void gfs2_ail_empty_tr(struct gfs2_sbd *sdp, struct gfs2_trans *tr,
  * @sdp: the filesystem
  * @tr: the transaction
  * @max_revokes: If nonzero, issue revokes for the bd items for written buffers
+ * @inactive: Returned count of ail elements that are no longer active
  *
  * returns: the transaction's count of remaining active items
  */
 
 static int gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr,
-				int *max_revokes)
+				int *max_revokes, u32 *inactive)
 {
 	struct gfs2_bufdata *bd, *s;
 	struct buffer_head *bh;
@@ -326,6 +327,7 @@ static int gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr,
 			active_count++;
 			continue;
 		}
+		(*inactive)++;
 		if (!buffer_uptodate(bh) &&
 		    !cmpxchg(&sdp->sd_log_error, 0, -EIO)) {
 			gfs2_io_error_bh(sdp, bh);
@@ -351,19 +353,23 @@ static int gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr,
  * gfs2_ail1_empty - Try to empty the ail1 lists
  * @sdp: The superblock
  * @max_revokes: If non-zero, add revokes where appropriate
+ * @progress: if non-null, count of ail items for which we made progress
  *
  * Tries to empty the ail1 lists, starting with the oldest first
  */
 
-static int gfs2_ail1_empty(struct gfs2_sbd *sdp, int max_revokes)
+static int gfs2_ail1_empty(struct gfs2_sbd *sdp, int max_revokes, u32 *progress)
 {
 	struct gfs2_trans *tr, *s;
 	int oldest_tr = 1;
+	u32 tot_inactive = 0, inactive = 0;
 	int ret;
 
 	spin_lock(&sdp->sd_ail_lock);
 	list_for_each_entry_safe_reverse(tr, s, &sdp->sd_ail1_list, tr_list) {
-		if (!gfs2_ail1_empty_one(sdp, tr, &max_revokes) && oldest_tr)
+		ret = gfs2_ail1_empty_one(sdp, tr, &max_revokes, &inactive);
+		tot_inactive += inactive;
+;		if (!ret && oldest_tr)
 			list_move(&tr->tr_list, &sdp->sd_ail2_list);
 		else
 			oldest_tr = 0;
@@ -377,6 +383,8 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp, int max_revokes)
 		gfs2_withdraw(sdp);
 	}
 
+	if (progress)
+		*progress += tot_inactive;
 	return ret;
 }
 
@@ -812,7 +820,7 @@ void gfs2_flush_revokes(struct gfs2_sbd *sdp)
 	unsigned int max_revokes = atomic_read(&sdp->sd_log_revokes_available);
 
 	gfs2_log_lock(sdp);
-	gfs2_ail1_empty(sdp, max_revokes);
+	gfs2_ail1_empty(sdp, max_revokes, NULL);
 	gfs2_log_unlock(sdp);
 }
 
@@ -984,7 +992,7 @@ static void empty_ail1_list(struct gfs2_sbd *sdp)
 		}
 		gfs2_ail1_start(sdp);
 		gfs2_ail1_wait(sdp);
-		if (gfs2_ail1_empty(sdp, 0))
+		if (gfs2_ail1_empty(sdp, 0, NULL))
 			return;
 	}
 }
@@ -1301,6 +1309,7 @@ int gfs2_logd(void *data)
 {
 	struct gfs2_sbd *sdp = data;
 	unsigned long t = 1;
+	u32 progress;
 	DEFINE_WAIT(wait);
 
 	while (!kthread_should_stop()) {
@@ -1320,28 +1329,41 @@ int gfs2_logd(void *data)
 			continue;
 		}
 
+		progress = 0;
 		if (gfs2_jrnl_flush_reqd(sdp) || t == 0) {
-			gfs2_ail1_empty(sdp, 0);
+			gfs2_ail1_empty(sdp, 0, &progress);
 			gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_NORMAL |
 						  GFS2_LFC_LOGD_JFLUSH_REQD);
 		}
 
+		t = gfs2_tune_get(sdp, gt_logd_secs) * HZ;
+
 		if (gfs2_ail_flush_reqd(sdp)) {
 			gfs2_ail1_start(sdp);
 			gfs2_ail1_wait(sdp);
-			gfs2_ail1_empty(sdp, 0);
+			/*
+			 * If we already made progress don't count again:
+			 * the second call may make no progress and zero our
+			 * counter, and we really only need to know if progress
+			 * was made.
+			 */
+			gfs2_ail1_empty(sdp, 0, progress ? NULL : &progress);
 			gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_NORMAL |
 						  GFS2_LFC_LOGD_AIL_FLUSH_REQD);
+			/*
+			 * If we made no progress on our ail list, wait only
+			 * 1 second.
+			 */
+			if (!progress)
+				t = HZ;
 		}
 
-		t = gfs2_tune_get(sdp, gt_logd_secs) * HZ;
-
 		try_to_freeze();
 
 		do {
 			prepare_to_wait(&sdp->sd_logd_waitq, &wait,
 					TASK_INTERRUPTIBLE);
-			if (!gfs2_ail_flush_reqd(sdp) &&
+			if ((!gfs2_ail_flush_reqd(sdp) || !progress) &&
 			    !gfs2_jrnl_flush_reqd(sdp) &&
 			    !kthread_should_stop())
 				t = schedule_timeout(t);
-- 
2.41.0