gfs2: Perform second log flush in gfs2_make_fs_ro
authorBob Peterson <rpeterso@redhat.com>
Fri, 21 Apr 2023 19:07:08 +0000 (15:07 -0400)
committerAndreas Gruenbacher <agruenba@redhat.com>
Tue, 25 Apr 2023 09:01:41 +0000 (11:01 +0200)
Before this patch, function gfs2_make_fs_ro called gfs2_log_flush once to
finalize the log. However, if there's dirty metadata, log flushes tend
to sync the metadata and formulate revokes. Before this patch, those
revokes may not be written out to the journal immediately, which meant
unresolved glocks could still have revokes in their ail lists. When the
glock worker runs, it tries to transition the glock, but the unresolved
revokes in the ail still need to be written, so it tries to start a
transaction. It's impossible to start a transaction because at that
point, the SDF_JOURNAL_LIVE flag has been cleared by gfs2_make_fs_ro.
That causes the glock worker to fail, unable to write the revokes. The
calling sequence looked something like this:

gfs2_make_fs_ro
   gfs2_log_flush - with GFS2_LOG_HEAD_FLUSH_SHUTDOWN flag set
if (flags & GFS2_LOG_HEAD_FLUSH_SHUTDOWN)
clear_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
...meanwhile...
glock_work_func
   do_xmote
      rgrp_go_sync (or possibly inode_go_sync)
         ...
         gfs2_ail_empty_gl
            __gfs2_trans_begin
               if (unlikely(!test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags))) {
               ...
                  return -EROFS;

The previous patch in the series ("gfs2: return errors from
gfs2_ail_empty_gl") now causes the transaction error to no longer be
ignored, so it causes a warning from MOST of the xfstests:

WARNING: CPU: 11 PID: X at fs/gfs2/super.c:603 gfs2_put_super [gfs2]

which corresponds to:

WARN_ON(gfs2_withdrawing(sdp));

The withdraw was triggered silently from do_xmote by:

if (unlikely(sdp->sd_log_error && !gfs2_withdrawn(sdp)))
gfs2_withdraw_delayed(sdp);

This patch adds a second log_flush to gfs2_make_fs_ro: one to sync the
data and one to sync any outstanding revokes and finalize the journal.
Note that both of these log flushes need to be "special," in other
words, not GFS2_LOG_HEAD_FLUSH_NORMAL.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
fs/gfs2/super.c

index a83fa62..5eed8c2 100644 (file)
@@ -552,6 +552,15 @@ void gfs2_make_fs_ro(struct gfs2_sbd *sdp)
                gfs2_quota_sync(sdp->sd_vfs, 0);
                gfs2_statfs_sync(sdp->sd_vfs, 0);
 
+               /* We do two log flushes here. The first one commits dirty inodes
+                * and rgrps to the journal, but queues up revokes to the ail list.
+                * The second flush writes out and removes the revokes.
+                *
+                * The first must be done before the FLUSH_SHUTDOWN code
+                * clears the LIVE flag, otherwise it will not be able to start
+                * a transaction to write its revokes, and the error will cause
+                * a withdraw of the file system. */
+               gfs2_log_flush(sdp, NULL, GFS2_LFC_MAKE_FS_RO);
                gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_SHUTDOWN |
                               GFS2_LFC_MAKE_FS_RO);
                wait_event_timeout(sdp->sd_log_waitq,