[PATCH] jfs: serialize jfs_flush_journal() logsync list checks

Cen Zhang posted 1 patch 1 month, 1 week ago
fs/jfs/jfs_logmgr.c | 57 +++++++++++++++++++++++++++++++++------------
1 file changed, 38 insertions(+), 19 deletions(-)
[PATCH] jfs: serialize jfs_flush_journal() logsync list checks
Posted by Cen Zhang 1 month, 1 week ago
The logsync list is protected by log->synclock.  Entries are added,
moved, removed, and have their metapage logsync state updated while
holding that lock.

jfs_flush_journal() checks log->synclist without taking log->synclock
while waiting for the lazycommit thread to catch up.  In debug builds it
can also traverse the list and dump entries without serialization.  A
lazy commit can wake a flush waiter before txUnlock() updates metapage
logsync state and removes the transaction block from log->synclist, so
jfs_flush_journal() can sample or traverse the list concurrently with a
protected updater.

Use log->synclock for the flush-side empty checks and for the debug list
walk.  This keeps jfs_flush_journal() serialized with the existing
logsync mutators without changing commit ordering.

Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 fs/jfs/jfs_logmgr.c | 57 +++++++++++++++++++++++++++++++++------------
 1 file changed, 38 insertions(+), 19 deletions(-)

diff --git a/fs/jfs/jfs_logmgr.c b/fs/jfs/jfs_logmgr.c
index ada00d5..ea607ce 100644
--- a/fs/jfs/jfs_logmgr.c
+++ b/fs/jfs/jfs_logmgr.c
@@ -1495,6 +1495,18 @@ int lmLogClose(struct super_block *sb)
 }
 
 
+static bool jfs_log_synclist_empty(struct jfs_log *log)
+{
+	unsigned long flags;
+	bool empty;
+
+	LOGSYNC_LOCK(log, flags);
+	empty = list_empty(&log->synclist);
+	LOGSYNC_UNLOCK(log, flags);
+
+	return empty;
+}
+
 /*
  * NAME:	jfs_flush_journal()
  *
@@ -1575,40 +1587,47 @@ void jfs_flush_journal(struct jfs_log *log, int wait)
 	 * If there was recent activity, we may need to wait
 	 * for the lazycommit thread to catch up
 	 */
-	if ((!list_empty(&log->cqueue)) || !list_empty(&log->synclist)) {
+	if ((!list_empty(&log->cqueue)) || !jfs_log_synclist_empty(log)) {
 		for (i = 0; i < 200; i++) {	/* Too much? */
 			msleep(250);
 			write_special_inodes(log, filemap_fdatawrite);
 			if (list_empty(&log->cqueue) &&
-			    list_empty(&log->synclist))
+			    jfs_log_synclist_empty(log))
 				break;
 		}
 	}
 	assert(list_empty(&log->cqueue));
 
 #ifdef CONFIG_JFS_DEBUG
-	if (!list_empty(&log->synclist)) {
+	{
+		unsigned long flags;
 		struct logsyncblk *lp;
 
-		printk(KERN_ERR "jfs_flush_journal: synclist not empty\n");
-		list_for_each_entry(lp, &log->synclist, synclist) {
-			if (lp->xflag & COMMIT_PAGE) {
-				struct metapage *mp = (struct metapage *)lp;
-				print_hex_dump(KERN_ERR, "metapage: ",
-					       DUMP_PREFIX_ADDRESS, 16, 4,
-					       mp, sizeof(struct metapage), 0);
-				print_hex_dump(KERN_ERR, "page: ",
-					       DUMP_PREFIX_ADDRESS, 16,
-					       sizeof(long), mp->folio,
-					       sizeof(struct page), 0);
-			} else
-				print_hex_dump(KERN_ERR, "tblock:",
-					       DUMP_PREFIX_ADDRESS, 16, 4,
-					       lp, sizeof(struct tblock), 0);
+		LOGSYNC_LOCK(log, flags);
+		if (!list_empty(&log->synclist)) {
+			pr_err("%s: synclist not empty\n", __func__);
+			list_for_each_entry(lp, &log->synclist, synclist) {
+				if (lp->xflag & COMMIT_PAGE) {
+					struct metapage *mp = (struct metapage *)lp;
+
+					print_hex_dump(KERN_ERR, "metapage: ",
+						       DUMP_PREFIX_ADDRESS, 16, 4,
+						       mp, sizeof(struct metapage), 0);
+					print_hex_dump(KERN_ERR, "page: ",
+						       DUMP_PREFIX_ADDRESS, 16,
+						       sizeof(long), mp->folio,
+						       sizeof(struct page), 0);
+				} else {
+					print_hex_dump(KERN_ERR, "tblock:",
+						       DUMP_PREFIX_ADDRESS, 16, 4,
+						       lp, sizeof(struct tblock), 0);
+				}
+			}
 		}
+		LOGSYNC_UNLOCK(log, flags);
 	}
 #else
-	WARN_ON(!list_empty(&log->synclist));
+	WARN_ON(!jfs_log_synclist_empty(log));
 #endif
 	clear_bit(log_FLUSH, &log->flag);
 }
-- 
2.51.0