cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
From: Bob Peterson <rpeterso@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH] gfs2: prevent gfs2_logd spinning
Date: Wed, 26 Jul 2023 12:00:33 -0500	[thread overview]
Message-ID: <20230726170033.389773-1-rpeterso@redhat.com> (raw)

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


                 reply	other threads:[~2023-07-26 17:00 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230726170033.389773-1-rpeterso@redhat.com \
    --to=rpeterso@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).