Skip to content

Commit b5f17be

Browse files
Dave ChinnerDarrick J. Wong
authored andcommitted
xfs: log shutdown triggers should only shut down the log
We've got a mess on our hands. 1. xfs_trans_commit() cannot cancel transactions because the mount is shut down - that causes dirty, aborted, unlogged log items to sit unpinned in memory and potentially get written to disk before the log is shut down. Hence xfs_trans_commit() can only abort transactions when xlog_is_shutdown() is true. 2. xfs_force_shutdown() is used in places to cause the current modification to be aborted via xfs_trans_commit() because it may be impractical or impossible to cancel the transaction directly, and hence xfs_trans_commit() must cancel transactions when xfs_is_shutdown() is true in this situation. But we can't do that because of #1. 3. Log IO errors cause log shutdowns by calling xfs_force_shutdown() to shut down the mount and then the log from log IO completion. 4. xfs_force_shutdown() can result in a log force being issued, which has to wait for log IO completion before it will mark the log as shut down. If #3 races with some other shutdown trigger that runs a log force, we rely on xfs_force_shutdown() silently ignoring #3 and avoiding shutting down the log until the failed log force completes. 5. To ensure #2 always works, we have to ensure that xfs_force_shutdown() does not return until the the log is shut down. But in the case of #4, this will result in a deadlock because the log Io completion will block waiting for a log force to complete which is blocked waiting for log IO to complete.... So the very first thing we have to do here to untangle this mess is dissociate log shutdown triggers from mount shutdowns. We already have xlog_forced_shutdown, which will atomically transistion to the log a shutdown state. Due to internal asserts it cannot be called multiple times, but was done simply because the only place that could call it was xfs_do_force_shutdown() (i.e. the mount shutdown!) and that could only call it once and once only. So the first thing we do is remove the asserts. We then convert all the internal log shutdown triggers to call xlog_force_shutdown() directly instead of xfs_force_shutdown(). This allows the log shutdown triggers to shut down the log without needing to care about mount based shutdown constraints. This means we shut down the log independently of the mount and the mount may not notice this until it's next attempt to read or modify metadata. At that point (e.g. xfs_trans_commit()) it will see that the log is shutdown, error out and shutdown the mount. To ensure that all the unmount behaviours and asserts track correctly as a result of a log shutdown, propagate the shutdown up to the mount if it is not already set. This keeps the mount and log state in sync, and saves a huge amount of hassle where code fails because of a log shutdown but only checks for mount shutdowns and hence ends up doing the wrong thing. Cleaning up that mess is an exercise for another day. This enables us to address the other problems noted above in followup patches. Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Darrick J. Wong <djwong@kernel.org> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
1 parent cd6f79d commit b5f17be

File tree

5 files changed

+33
-18
lines changed

5 files changed

+33
-18
lines changed

fs/xfs/xfs_log.c

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1374,7 +1374,7 @@ xlog_ioend_work(
13741374
*/
13751375
if (XFS_TEST_ERROR(error, log->l_mp, XFS_ERRTAG_IODONE_IOERR)) {
13761376
xfs_alert(log->l_mp, "log I/O error %d", error);
1377-
xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
1377+
xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
13781378
}
13791379

13801380
xlog_state_done_syncing(iclog);
@@ -1913,7 +1913,7 @@ xlog_write_iclog(
19131913
iclog->ic_flags &= ~(XLOG_ICL_NEED_FLUSH | XLOG_ICL_NEED_FUA);
19141914

19151915
if (xlog_map_iclog_data(&iclog->ic_bio, iclog->ic_data, count)) {
1916-
xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
1916+
xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
19171917
return;
19181918
}
19191919
if (is_vmalloc_addr(iclog->ic_data))
@@ -2488,7 +2488,7 @@ xlog_write(
24882488
xfs_alert_tag(log->l_mp, XFS_PTAG_LOGRES,
24892489
"ctx ticket reservation ran out. Need to up reservation");
24902490
xlog_print_tic_res(log->l_mp, ticket);
2491-
xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
2491+
xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
24922492
}
24932493

24942494
len = xlog_write_calc_vec_length(ticket, log_vector, optype);
@@ -3822,9 +3822,10 @@ xlog_verify_iclog(
38223822
#endif
38233823

38243824
/*
3825-
* Perform a forced shutdown on the log. This should be called once and once
3826-
* only by the high level filesystem shutdown code to shut the log subsystem
3827-
* down cleanly.
3825+
* Perform a forced shutdown on the log.
3826+
*
3827+
* This can be called from low level log code to trigger a shutdown, or from the
3828+
* high level mount shutdown code when the mount shuts down.
38283829
*
38293830
* Our main objectives here are to make sure that:
38303831
* a. if the shutdown was not due to a log IO error, flush the logs to
@@ -3833,6 +3834,8 @@ xlog_verify_iclog(
38333834
* parties to find out. Nothing new gets queued after this is done.
38343835
* c. Tasks sleeping on log reservations, pinned objects and
38353836
* other resources get woken up.
3837+
* d. The mount is also marked as shut down so that log triggered shutdowns
3838+
* still behave the same as if they called xfs_forced_shutdown().
38363839
*
38373840
* Return true if the shutdown cause was a log IO error and we actually shut the
38383841
* log down.
@@ -3851,8 +3854,6 @@ xlog_force_shutdown(
38513854
if (!log || xlog_in_recovery(log))
38523855
return false;
38533856

3854-
ASSERT(!xlog_is_shutdown(log));
3855-
38563857
/*
38573858
* Flush all the completed transactions to disk before marking the log
38583859
* being shut down. We need to do this first as shutting down the log
@@ -3879,11 +3880,24 @@ xlog_force_shutdown(
38793880
spin_lock(&log->l_icloglock);
38803881
if (test_and_set_bit(XLOG_IO_ERROR, &log->l_opstate)) {
38813882
spin_unlock(&log->l_icloglock);
3882-
ASSERT(0);
38833883
return false;
38843884
}
38853885
spin_unlock(&log->l_icloglock);
38863886

3887+
/*
3888+
* If this log shutdown also sets the mount shutdown state, issue a
3889+
* shutdown warning message.
3890+
*/
3891+
if (!test_and_set_bit(XFS_OPSTATE_SHUTDOWN, &log->l_mp->m_opstate)) {
3892+
xfs_alert_tag(log->l_mp, XFS_PTAG_SHUTDOWN_LOGERROR,
3893+
"Filesystem has been shut down due to log error (0x%x).",
3894+
shutdown_flags);
3895+
xfs_alert(log->l_mp,
3896+
"Please unmount the filesystem and rectify the problem(s).");
3897+
if (xfs_error_level >= XFS_ERRLEVEL_HIGH)
3898+
xfs_stack_trace();
3899+
}
3900+
38873901
/*
38883902
* We don't want anybody waiting for log reservations after this. That
38893903
* means we have to wake up everybody queued up on reserveq as well as

fs/xfs/xfs_log_cil.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ xlog_cil_insert_items(
540540
spin_unlock(&cil->xc_cil_lock);
541541

542542
if (tp->t_ticket->t_curr_res < 0)
543-
xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
543+
xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
544544
}
545545

546546
static void
@@ -854,7 +854,7 @@ xlog_cil_write_commit_record(
854854

855855
error = xlog_write(log, ctx, &vec, ctx->ticket, XLOG_COMMIT_TRANS);
856856
if (error)
857-
xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
857+
xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
858858
return error;
859859
}
860860

fs/xfs/xfs_log_recover.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2485,7 +2485,7 @@ xlog_finish_defer_ops(
24852485
error = xfs_trans_alloc(mp, &resv, dfc->dfc_blkres,
24862486
dfc->dfc_rtxres, XFS_TRANS_RESERVE, &tp);
24872487
if (error) {
2488-
xfs_force_shutdown(mp, SHUTDOWN_LOG_IO_ERROR);
2488+
xlog_force_shutdown(mp->m_log, SHUTDOWN_LOG_IO_ERROR);
24892489
return error;
24902490
}
24912491

@@ -3454,7 +3454,7 @@ xlog_recover_finish(
34543454
*/
34553455
xlog_recover_cancel_intents(log);
34563456
xfs_alert(log->l_mp, "Failed to recover intents");
3457-
xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
3457+
xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
34583458
return error;
34593459
}
34603460

@@ -3501,7 +3501,7 @@ xlog_recover_finish(
35013501
* end of intents processing can be pushed through the CIL
35023502
* and AIL.
35033503
*/
3504-
xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
3504+
xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
35053505
}
35063506

35073507
return 0;

fs/xfs/xfs_mount.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "xfs_trans.h"
2222
#include "xfs_trans_priv.h"
2323
#include "xfs_log.h"
24+
#include "xfs_log_priv.h"
2425
#include "xfs_error.h"
2526
#include "xfs_quota.h"
2627
#include "xfs_fsops.h"

fs/xfs/xfs_trans_ail.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -873,17 +873,17 @@ xfs_trans_ail_delete(
873873
int shutdown_type)
874874
{
875875
struct xfs_ail *ailp = lip->li_ailp;
876-
struct xfs_mount *mp = ailp->ail_log->l_mp;
876+
struct xlog *log = ailp->ail_log;
877877
xfs_lsn_t tail_lsn;
878878

879879
spin_lock(&ailp->ail_lock);
880880
if (!test_bit(XFS_LI_IN_AIL, &lip->li_flags)) {
881881
spin_unlock(&ailp->ail_lock);
882-
if (shutdown_type && !xlog_is_shutdown(ailp->ail_log)) {
883-
xfs_alert_tag(mp, XFS_PTAG_AILDELETE,
882+
if (shutdown_type && !xlog_is_shutdown(log)) {
883+
xfs_alert_tag(log->l_mp, XFS_PTAG_AILDELETE,
884884
"%s: attempting to delete a log item that is not in the AIL",
885885
__func__);
886-
xfs_force_shutdown(mp, shutdown_type);
886+
xlog_force_shutdown(log, shutdown_type);
887887
}
888888
return;
889889
}

0 commit comments

Comments
 (0)