On Tue, Apr 29, 2014 at 08:56:49AM -0400, Brian Foster wrote: > On Tue, Apr 29, 2014 at 08:18:49AM +1000, Dave Chinner wrote: > > On Mon, Apr 28, 2014 at 04:39:50PM -0500, Mark Tinguely wrote: > > > On 04/28/14 15:54, Dave Chinner wrote: > > > >On Mon, Apr 28, 2014 at 11:35:16AM -0500, Eric Sandeen wrote: > ... > > > > xfs: don't sleep in xlog_cil_force_lsn on shutdown > > > > From: Dave Chinner <dchinner@xxxxxxxxxx> > > > > 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. > > > > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> > > --- > > fs/xfs/xfs_log.c | 7 +++++-- > > fs/xfs/xfs_log_cil.c | 50 ++++++++++++++++++++++++++++++++++++++++++-------- > > 2 files changed, 47 insertions(+), 10 deletions(-) > > > > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c > > index a5f8bd9..dbba2d7 100644 > > --- a/fs/xfs/xfs_log.c > > +++ b/fs/xfs/xfs_log.c > > @@ -3952,11 +3952,14 @@ xfs_log_force_umount( > > retval = xlog_state_ioerror(log); > > spin_unlock(&log->l_icloglock); > > } > > + > > /* > > - * Wake up everybody waiting on xfs_log_force. > > - * Callback all log item committed functions as if the > > + * Wake up everybody waiting on xfs_log_force. This needs to wake anyone > > + * waiting on a CIL push that is issued as part of a log force first > > + * before running the log item committed callback functions as if the > > * log writes were completed. > > */ > > + wake_up_all(&log->l_cilp->xc_commit_wait); > > xlog_state_do_callback(log, XFS_LI_ABORTED, NULL); > > > > Why is this necessary? It looks like xlog_state_do_callback() will hit > the xlog_cil_committed() callback with the aborted flag... I added it before adding the code into xlog_cil_committed(). Regardless, I think it's a good idea to have a specific wakeup to for waiters catch the shutdown purely from a defensive POV - if we screw up something else, we will still get a wakeup after the log is marked shutdown and not hang... > > #ifdef XFSERRORDEBUG > > diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c > > index 7e54553..3a68ddf 100644 > > --- a/fs/xfs/xfs_log_cil.c > > +++ b/fs/xfs/xfs_log_cil.c > > @@ -385,7 +385,15 @@ xlog_cil_committed( > > xfs_extent_busy_clear(mp, &ctx->busy_extents, > > (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); > > + if (abort) > > + wake_up_all(&cil->xc_commit_wait); > > There's a compile error here. The parameter should be '&ctx->cil->...' Forgot to refresh the patch - the tested version has this fix. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
