| From 5a61ae1402f15276ee4e003e198aab816958ca69 Mon Sep 17 00:00:00 2001 |
| From: Andreas Gruenbacher <agruenba@redhat.com> |
| Date: Fri, 28 Aug 2020 23:44:36 +0200 |
| Subject: gfs2: Make sure we don't miss any delayed withdraws |
| |
| From: Andreas Gruenbacher <agruenba@redhat.com> |
| |
| commit 5a61ae1402f15276ee4e003e198aab816958ca69 upstream. |
| |
| 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> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| fs/gfs2/log.c | 61 ++++++++++++++++++++++++++++---------------------------- |
| fs/gfs2/super.c | 2 + |
| fs/gfs2/util.h | 10 +++++++++ |
| 3 files changed, 43 insertions(+), 30 deletions(-) |
| |
| --- a/fs/gfs2/log.c |
| +++ b/fs/gfs2/log.c |
| @@ -954,10 +954,8 @@ void gfs2_log_flush(struct gfs2_sbd *sdp |
| 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 |
| 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 |
| 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 |
| 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 |
| 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; |
| } |
| |
| /** |
| --- 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 */ |
| --- a/fs/gfs2/util.h |
| +++ b/fs/gfs2/util.h |
| @@ -205,6 +205,16 @@ static inline bool gfs2_withdrawn(struct |
| 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) |
| |