[PATCH RFC 20/28] gfs2: wait for a pending withdraw before remounting read-only

[Christian Brauner]

Since commit 9c4a3de6cd5b ("gfs2: Asynchronous withdraw") most of a
withdraw happens in do_withdraw() from a work item and SDF_JOURNAL_LIVE
stays set until it ran. A remount to read-only in that window goes
through gfs2_make_fs_ro(). Its log flushes bail out early on the
withdrawn filesystem before FLUSH_SHUTDOWN can clear SDF_JOURNAL_LIVE
and then gfs2_quota_cleanup() hits its BUG_ON():

  kernel BUG at fs/gfs2/quota.c:1532!
  RIP: 0010:gfs2_quota_cleanup+0x562/0x6b0
   gfs2_make_fs_ro+0xf3/0x2e0
   gfs2_reconfigure+0x42e/0xfd0
   reconfigure_super+0x437/0xb20

Just flushing the withdraw work before the remount isn't enough.
gfs2_withdraw() sets SDF_WITHDRAWN, calls dump_stack() and only then
queues the work. So a remount can see a withdrawn filesystem while
there's nothing to flush yet. And a withdraw can also start in the
middle of gfs2_make_fs_ro().

So let do_withdraw() wake up waiters when it clears SDF_JOURNAL_LIVE and
let gfs2_make_fs_ro() wait for the bit after its log flushes, right
before gfs2_quota_cleanup(). On a healthy filesystem the shutdown flush
already cleared it. umount doesn't need to wait. gfs2_put_super() sets
SDF_NORECOVERY first and gfs2_quota_cleanup() only BUGs without it. Once
the bit is clear the journal isn't live anymore and gfs2_make_fs_ro()
doesn't write anything.

gfs2_make_fs_rw() can leave the bit set on a withdrawn filesystem as
well. It checks for a withdraw and then sets SDF_JOURNAL_LIVE. If
do_withdraw() runs in between it finds the journal not live yet and
returns. Nothing clears the bit then and the next remount to read-only
waits for it forever. So check for the withdraw and set the bit under
sd_log_flush_lock, which do_withdraw() holds while it tests the bit.
Then either the remount sees the withdraw or do_withdraw() sees the live
journal.

Reproducer, lock_nolock: a SCHED_FIFO task writes 1 to the withdraw
sysfs knob from cpu 1 and keeps cpu 1 busy (RT throttling and cpu 1's
fair server disabled) so do_withdraw() can't run, then from cpu 2:

  mount -o remount,ro /mnt

On lock_dlm the window lasts as long as the withdraw helper runs. The
window before the work is queued can be widened with a kprobe on
dump_stack_lvl() that delays the task writing to the withdraw knob by a
few seconds. A remount to read-only or sysrq u in that window hits the
same BUG_ON().

Fixes: 9c4a3de6cd5b ("gfs2: Asynchronous withdraw")
Cc: stable at vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner at kernel.org>
---
 fs/gfs2/super.c | 22 ++++++++++++++++++----
 fs/gfs2/util.c  |  2 +-
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 06302c29340f..1d7b39354d65 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -147,12 +147,23 @@ int gfs2_make_fs_rw(struct gfs2_sbd *sdp)
 	}
 
 	error = gfs2_quota_init(sdp);
-	if (!error && gfs2_withdrawn(sdp)) {
-		gfs2_quota_cleanup(sdp);
+	if (error)
+		return error;
+
+	/*
+	 * do_withdraw() only takes a live journal down. Check for a withdraw
+	 * under the lock it holds for that so that either we see the withdraw
+	 * or it sees the journal live. Otherwise gfs2_make_fs_ro() waits for
+	 * the bit forever.
+	 */
+	down_write(&sdp->sd_log_flush_lock);
+	if (gfs2_withdrawn(sdp))
 		error = -EIO;
-	}
-	if (!error)
+	else
 		set_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
+	up_write(&sdp->sd_log_flush_lock);
+	if (error)
+		gfs2_quota_cleanup(sdp);
 	return error;
 }
 
@@ -565,6 +576,9 @@ void gfs2_make_fs_ro(struct gfs2_sbd *sdp)
 				   HZ * 5);
 		gfs2_assert_warn(sdp, gfs2_log_is_empty(sdp));
 	}
+	/* A withdraw that do_withdraw() didn't finish yet leaves it live. */
+	if (!test_bit(SDF_NORECOVERY, &sdp->sd_flags))
+		wait_on_bit(&sdp->sd_flags, SDF_JOURNAL_LIVE, TASK_UNINTERRUPTIBLE);
 	gfs2_quota_cleanup(sdp);
 }
 
diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
index 83b8bb6446e5..1535ce2b7aa8 100644
--- a/fs/gfs2/util.c
+++ b/fs/gfs2/util.c
@@ -122,7 +122,7 @@ static void do_withdraw(struct gfs2_sbd *sdp)
 		up_write(&sdp->sd_log_flush_lock);
 		return;
 	}
-	clear_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
+	clear_and_wake_up_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
 	gfs2_ail_drain(sdp); /* frees all transactions */
 	up_write(&sdp->sd_log_flush_lock);
 

-- 
2.53.0