Commit ac983517 authored by Dave Chinner's avatar Dave Chinner Committed by Dave Chinner

xfs: don't sleep in xlog_cil_force_lsn on shutdown

Reports of a shutdown hang when fsyncing a directory have surfaced,
such as this:

[ 3663.394472] Call Trace:
[ 3663.397199]  [<ffffffff815f1889>] schedule+0x29/0x70
[ 3663.402743]  [<ffffffffa01feda5>] xlog_cil_force_lsn+0x185/0x1a0 [xfs]
[ 3663.416249]  [<ffffffffa01fd3af>] _xfs_log_force_lsn+0x6f/0x2f0 [xfs]
[ 3663.429271]  [<ffffffffa01a339d>] xfs_dir_fsync+0x7d/0xe0 [xfs]
[ 3663.435873]  [<ffffffff811df8c5>] do_fsync+0x65/0xa0
[ 3663.441408]  [<ffffffff811dfbc0>] SyS_fsync+0x10/0x20
[ 3663.447043]  [<ffffffff815fc7d9>] system_call_fastpath+0x16/0x1b

If we trigger a shutdown in xlog_cil_push() from xlog_write(), we
will never wake waiters on the current push sequence number, so
anything waiting in xlog_cil_force_lsn() for that push sequence
number to come up will not get woken and hence stall the shutdown.

Fix this by ensuring we call wake_up_all(&cil->xc_commit_wait) in
the push abort handling, in the log shutdown code when waking all
waiters, and adding a shutdown check in the sequence completion wait
loops to ensure they abort when a wakeup due to a shutdown occurs.
Reported-by: default avatarBoris Ranto <branto@redhat.com>
Reported-by: default avatarEric Sandeen <esandeen@redhat.com>
Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
Signed-off-by: default avatarDave Chinner <david@fromorbit.com>
parent 49abc3a8
...@@ -3915,11 +3915,14 @@ xfs_log_force_umount( ...@@ -3915,11 +3915,14 @@ xfs_log_force_umount(
retval = xlog_state_ioerror(log); retval = xlog_state_ioerror(log);
spin_unlock(&log->l_icloglock); spin_unlock(&log->l_icloglock);
} }
/* /*
* Wake up everybody waiting on xfs_log_force. * Wake up everybody waiting on xfs_log_force. Wake the CIL push first
* Callback all log item committed functions as if the * as if the log writes were completed. The abort handling in the log
* log writes were completed. * item committed callback functions will do this again under lock to
* avoid races.
*/ */
wake_up_all(&log->l_cilp->xc_commit_wait);
xlog_state_do_callback(log, XFS_LI_ABORTED, NULL); xlog_state_do_callback(log, XFS_LI_ABORTED, NULL);
#ifdef XFSERRORDEBUG #ifdef XFSERRORDEBUG
......
...@@ -385,7 +385,15 @@ xlog_cil_committed( ...@@ -385,7 +385,15 @@ xlog_cil_committed(
xfs_extent_busy_clear(mp, &ctx->busy_extents, xfs_extent_busy_clear(mp, &ctx->busy_extents,
(mp->m_flags & XFS_MOUNT_DISCARD) && !abort); (mp->m_flags & XFS_MOUNT_DISCARD) && !abort);
/*
* If we are aborting the commit, wake up anyone waiting on the
* committing list. If we don't, then a shutdown we can leave processes
* waiting in xlog_cil_force_lsn() waiting on a sequence commit that
* will never happen because we aborted it.
*/
spin_lock(&ctx->cil->xc_push_lock); spin_lock(&ctx->cil->xc_push_lock);
if (abort)
wake_up_all(&ctx->cil->xc_commit_wait);
list_del(&ctx->committing); list_del(&ctx->committing);
spin_unlock(&ctx->cil->xc_push_lock); spin_unlock(&ctx->cil->xc_push_lock);
...@@ -563,9 +571,19 @@ xlog_cil_push( ...@@ -563,9 +571,19 @@ xlog_cil_push(
restart: restart:
spin_lock(&cil->xc_push_lock); spin_lock(&cil->xc_push_lock);
list_for_each_entry(new_ctx, &cil->xc_committing, committing) { list_for_each_entry(new_ctx, &cil->xc_committing, committing) {
/*
* Avoid getting stuck in this loop because we were woken by the
* shutdown, but then went back to sleep once already in the
* shutdown state.
*/
if (XLOG_FORCED_SHUTDOWN(log)) {
spin_unlock(&cil->xc_push_lock);
goto out_abort_free_ticket;
}
/* /*
* Higher sequences will wait for this one so skip them. * Higher sequences will wait for this one so skip them.
* Don't wait for own own sequence, either. * Don't wait for our own sequence, either.
*/ */
if (new_ctx->sequence >= ctx->sequence) if (new_ctx->sequence >= ctx->sequence)
continue; continue;
...@@ -810,6 +828,13 @@ xlog_cil_force_lsn( ...@@ -810,6 +828,13 @@ xlog_cil_force_lsn(
*/ */
spin_lock(&cil->xc_push_lock); spin_lock(&cil->xc_push_lock);
list_for_each_entry(ctx, &cil->xc_committing, committing) { list_for_each_entry(ctx, &cil->xc_committing, committing) {
/*
* Avoid getting stuck in this loop because we were woken by the
* shutdown, but then went back to sleep once already in the
* shutdown state.
*/
if (XLOG_FORCED_SHUTDOWN(log))
goto out_shutdown;
if (ctx->sequence > sequence) if (ctx->sequence > sequence)
continue; continue;
if (!ctx->commit_lsn) { if (!ctx->commit_lsn) {
...@@ -833,14 +858,12 @@ xlog_cil_force_lsn( ...@@ -833,14 +858,12 @@ xlog_cil_force_lsn(
* push sequence after the above wait loop and the CIL still contains * push sequence after the above wait loop and the CIL still contains
* dirty objects. * dirty objects.
* *
* When the push occurs, it will empty the CIL and * When the push occurs, it will empty the CIL and atomically increment
* atomically increment the currect sequence past the push sequence and * the currect sequence past the push sequence and move it into the
* move it into the committing list. Of course, if the CIL is clean at * committing list. Of course, if the CIL is clean at the time of the
* the time of the push, it won't have pushed the CIL at all, so in that * push, it won't have pushed the CIL at all, so in that case we should
* case we should try the push for this sequence again from the start * try the push for this sequence again from the start just in case.
* just in case.
*/ */
if (sequence == cil->xc_current_sequence && if (sequence == cil->xc_current_sequence &&
!list_empty(&cil->xc_cil)) { !list_empty(&cil->xc_cil)) {
spin_unlock(&cil->xc_push_lock); spin_unlock(&cil->xc_push_lock);
...@@ -849,6 +872,17 @@ xlog_cil_force_lsn( ...@@ -849,6 +872,17 @@ xlog_cil_force_lsn(
spin_unlock(&cil->xc_push_lock); spin_unlock(&cil->xc_push_lock);
return commit_lsn; return commit_lsn;
/*
* We detected a shutdown in progress. We need to trigger the log force
* to pass through it's iclog state machine error handling, even though
* we are already in a shutdown state. Hence we can't return
* NULLCOMMITLSN here as that has special meaning to log forces (i.e.
* LSN is already stable), so we return a zero LSN instead.
*/
out_shutdown:
spin_unlock(&cil->xc_push_lock);
return 0;
} }
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment