gfs2: Make sure we don't miss any delayed withdraws

Commit ca399c96e96e changes gfs2_log_flush to not withdraw the
filesystem while holding the log flush lock, but it fails to check if
the filesystem needs to be withdrawn once the log flush lock has been
released.  Likewise, commit f05b86db314d depends on gfs2_log_flush to
trigger for delayed withdraws.  Add that and clean up the code flow
somewhat.

In gfs2_put_super, add a check for delayed withdraws that have been
missed to prevent these kinds of bugs in the future.

Fixes: ca399c96e96e ("gfs2: flesh out delayed withdraw for gfs2_log_flush")
Fixes: f05b86db314d ("gfs2: Prepare to withdraw as soon as an IO error occurs in log write")
Cc: stable@vger.kernel.org # v5.7+: 462582b99b607: gfs2: add some much needed cleanup for log flushes that fail
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>

diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 3763c9ff1406bdbe102db85e2f5a68bf166e5c09..93032feb515995548e7706477b630af82a73b925 100644 (file)
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -954,10 +954,8 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, u32 flags)
                goto out;
 
        /* Log might have been flushed while we waited for the flush lock */
-       if (gl && !test_bit(GLF_LFLUSH, &gl->gl_flags)) {
-               up_write(&sdp->sd_log_flush_lock);
-               return;
-       }
+       if (gl && !test_bit(GLF_LFLUSH, &gl->gl_flags))
+               goto out;
        trace_gfs2_log_flush(sdp, 1, flags);
 
        if (flags & GFS2_LOG_HEAD_FLUSH_SHUTDOWN)
@@ -971,25 +969,25 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, u32 flags)
                if (unlikely (state == SFS_FROZEN))
                        if (gfs2_assert_withdraw_delayed(sdp,
                               !tr->tr_num_buf_new && !tr->tr_num_databuf_new))
-                               goto out;
+                               goto out_withdraw;
        }
 
        if (unlikely(state == SFS_FROZEN))
                if (gfs2_assert_withdraw_delayed(sdp, !sdp->sd_log_num_revoke))
-                       goto out;
+                       goto out_withdraw;
        if (gfs2_assert_withdraw_delayed(sdp,
                        sdp->sd_log_num_revoke == sdp->sd_log_committed_revoke))
-               goto out;
+               goto out_withdraw;
 
        gfs2_ordered_write(sdp);
        if (gfs2_withdrawn(sdp))
-               goto out;
+               goto out_withdraw;
        lops_before_commit(sdp, tr);
        if (gfs2_withdrawn(sdp))
-               goto out;
+               goto out_withdraw;
        gfs2_log_submit_bio(&sdp->sd_log_bio, REQ_OP_WRITE);
        if (gfs2_withdrawn(sdp))
-               goto out;
+               goto out_withdraw;
 
        if (sdp->sd_log_head != sdp->sd_log_flush_head) {
                log_flush_wait(sdp);
@@ -1000,7 +998,7 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, u32 flags)
                log_write_header(sdp, flags);
        }
        if (gfs2_withdrawn(sdp))
-               goto out;
+               goto out_withdraw;
        lops_after_commit(sdp, tr);
 
        gfs2_log_lock(sdp);
@@ -1020,7 +1018,7 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, u32 flags)
                if (!sdp->sd_log_idle) {
                        empty_ail1_list(sdp);
                        if (gfs2_withdrawn(sdp))
-                               goto out;
+                               goto out_withdraw;
                        atomic_dec(&sdp->sd_log_blks_free); /* Adjust for unreserved buffer */
                        trace_gfs2_log_blocks(sdp, -1);
                        log_write_header(sdp, flags);
@@ -1033,27 +1031,30 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, u32 flags)
                        atomic_set(&sdp->sd_freeze_state, SFS_FROZEN);
        }
 
-out:
-       if (gfs2_withdrawn(sdp)) {
-               trans_drain(tr);
-               /**
-                * If the tr_list is empty, we're withdrawing during a log
-                * flush that targets a transaction, but the transaction was
-                * never queued onto any of the ail lists. Here we add it to
-                * ail1 just so that ail_drain() will find and free it.
-                */
-               spin_lock(&sdp->sd_ail_lock);
-               if (tr && list_empty(&tr->tr_list))
-                       list_add(&tr->tr_list, &sdp->sd_ail1_list);
-               spin_unlock(&sdp->sd_ail_lock);
-               ail_drain(sdp); /* frees all transactions */
-               tr = NULL;
-       }
-
+out_end:
        trace_gfs2_log_flush(sdp, 0, flags);
+out:
        up_write(&sdp->sd_log_flush_lock);
-
        gfs2_trans_free(sdp, tr);
+       if (gfs2_withdrawing(sdp))
+               gfs2_withdraw(sdp);
+       return;
+
+out_withdraw:
+       trans_drain(tr);
+       /**
+        * If the tr_list is empty, we're withdrawing during a log
+        * flush that targets a transaction, but the transaction was
+        * never queued onto any of the ail lists. Here we add it to
+        * ail1 just so that ail_drain() will find and free it.
+        */
+       spin_lock(&sdp->sd_ail_lock);
+       if (tr && list_empty(&tr->tr_list))
+               list_add(&tr->tr_list, &sdp->sd_ail1_list);
+       spin_unlock(&sdp->sd_ail_lock);
+       ail_drain(sdp); /* frees all transactions */
+       tr = NULL;
+       goto out_end;
 }
 
 /**

diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 9f4d9e7be83971618a3275e933e85d5ae9a82af3..19add2da101344e3ac8a6896fd55cf602023c804 100644 (file)
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -702,6 +702,8 @@ restart:
                if (error)
                        gfs2_io_error(sdp);
        }
+       WARN_ON(gfs2_withdrawing(sdp));
+
        /*  At this point, we're through modifying the disk  */
 
        /*  Release stuff  */

diff --git a/fs/gfs2/util.h b/fs/gfs2/util.h
index 6d9157efe16c3b38ddcacd48d35eae674867f807..d7562981b3a09fcf206f385a09984571a8d68165 100644 (file)
--- a/fs/gfs2/util.h
+++ b/fs/gfs2/util.h
@@ -205,6 +205,16 @@ static inline bool gfs2_withdrawn(struct gfs2_sbd *sdp)
                test_bit(SDF_WITHDRAWING, &sdp->sd_flags);
 }
 
+/**
+ * gfs2_withdrawing - check if a withdraw is pending
+ * @sdp: the superblock
+ */
+static inline bool gfs2_withdrawing(struct gfs2_sbd *sdp)
+{
+       return test_bit(SDF_WITHDRAWING, &sdp->sd_flags) &&
+              !test_bit(SDF_WITHDRAWN, &sdp->sd_flags);
+}
+
 #define gfs2_tune_get(sdp, field) \
 gfs2_tune_get_i(&(sdp)->sd_tune, &(sdp)->sd_tune.field)