* [RFC] xfs: wait for the write of the superblock on unmount [not found] <20120717215957.855744999@tinguelysgi.com> @ 2012-07-18 17:33 ` tinguely 2012-07-19 1:58 ` Dave Chinner 2012-07-24 13:57 ` Christoph Hellwig 0 siblings, 2 replies; 6+ messages in thread From: tinguely @ 2012-07-18 17:33 UTC (permalink / raw) To: xfs [-- Attachment #1: xfs-lock_sbcount_wait.patch --] [-- Type: text/plain, Size: 1466 bytes --] Sorry, I have been distracted away from this regression. This was previously titled "xfs: synchronously write the superblock on unmount". xfs_wait_buftarg() does not wait for the completion of the write of the uncached superblock. This write can race with the shutdown of the log and causes a panic if the write does not win the race. The log write of the superblock is important for possible recovery, but a second syncronous write of the same superblock seems redundant. Would just waiting for the iodone() of the log write before tearing down the log be enough? Signed-off-by: Mark Tinguely <tinguely@sgi.com> --- fs/xfs/xfs_mount.c | 8 ++++++++ 1 file changed, 8 insertions(+) Index: b/fs/xfs/xfs_mount.c =================================================================== --- a/fs/xfs/xfs_mount.c +++ b/fs/xfs/xfs_mount.c @@ -1457,6 +1457,7 @@ void xfs_unmountfs( struct xfs_mount *mp) { + struct xfs_buf *bp = mp->m_sb_bp; __uint64_t resblks; int error; @@ -1529,6 +1530,13 @@ xfs_unmountfs( xfs_ail_push_all_sync(mp->m_ail); xfs_wait_buftarg(mp->m_ddev_targp); + /* + * Wait for the superblock to be written out. The superblock buffer + * will remain locked until the iodone(). + */ + xfs_buf_lock(bp); + xfs_buf_unlock(bp); + xfs_log_unmount_write(mp); xfs_log_unmount(mp); xfs_uuid_unmount(mp); _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] xfs: wait for the write of the superblock on unmount 2012-07-18 17:33 ` [RFC] xfs: wait for the write of the superblock on unmount tinguely @ 2012-07-19 1:58 ` Dave Chinner 2012-07-20 13:21 ` Mark Tinguely 2012-07-24 13:57 ` Christoph Hellwig 1 sibling, 1 reply; 6+ messages in thread From: Dave Chinner @ 2012-07-19 1:58 UTC (permalink / raw) To: tinguely; +Cc: xfs On Wed, Jul 18, 2012 at 12:33:58PM -0500, tinguely@sgi.com wrote: > Sorry, I have been distracted away from this regression. This was previously > titled "xfs: synchronously write the superblock on unmount". > > xfs_wait_buftarg() does not wait for the completion of the write of the > uncached superblock. This write can race with the shutdown of the log and > causes a panic if the write does not win the race. > > The log write of the superblock is important for possible recovery, but a > second syncronous write of the same superblock seems redundant. Would just > waiting for the iodone() of the log write before tearing down the log be > enough? Yes. i.e. something like: /* * The superblock buffer is uncached, so xfs_wait_buftarg() * will not wait for it. Hence we need to explicitly wait * for IO completion on the superblock to occur here. */ error = xfs_buf_iowait(mp->m_sb_bp); if (error) AAAAIEEEE! This fix is also needed in xfs_quiesce_attr() for the freeze and ro,remount cases as well. Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] xfs: wait for the write of the superblock on unmount 2012-07-19 1:58 ` Dave Chinner @ 2012-07-20 13:21 ` Mark Tinguely 2012-07-20 21:24 ` Mark Tinguely 0 siblings, 1 reply; 6+ messages in thread From: Mark Tinguely @ 2012-07-20 13:21 UTC (permalink / raw) To: Dave Chinner; +Cc: xfs On 07/18/12 20:58, Dave Chinner wrote: > On Wed, Jul 18, 2012 at 12:33:58PM -0500, tinguely@sgi.com wrote: >> Sorry, I have been distracted away from this regression. This was previously >> titled "xfs: synchronously write the superblock on unmount". >> >> xfs_wait_buftarg() does not wait for the completion of the write of the >> uncached superblock. This write can race with the shutdown of the log and >> causes a panic if the write does not win the race. >> >> The log write of the superblock is important for possible recovery, but a >> second syncronous write of the same superblock seems redundant. Would just >> waiting for the iodone() of the log write before tearing down the log be >> enough? > > Yes. i.e. something like: > > /* > * The superblock buffer is uncached, so xfs_wait_buftarg() > * will not wait for it. Hence we need to explicitly wait > * for IO completion on the superblock to occur here. > */ > error = xfs_buf_iowait(mp->m_sb_bp); > if (error) > AAAAIEEEE! > > This fix is also needed in xfs_quiesce_attr() for the freeze and > ro,remount cases as well. > > Cheers, > > Dave. > Thanks for the feedback. I chose the ugly lock to block on the superblock buffer because it has XBF_ASYNC set at that point and won't go through the complete() patch. I will look at turning the XBF_ASYNC flag off and wait on a xfs_buf_iowait(). --Mark. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] xfs: wait for the write of the superblock on unmount 2012-07-20 13:21 ` Mark Tinguely @ 2012-07-20 21:24 ` Mark Tinguely 0 siblings, 0 replies; 6+ messages in thread From: Mark Tinguely @ 2012-07-20 21:24 UTC (permalink / raw) To: Dave Chinner; +Cc: xfs On 07/20/12 08:21, Mark Tinguely wrote: > On 07/18/12 20:58, Dave Chinner wrote: >> On Wed, Jul 18, 2012 at 12:33:58PM -0500, tinguely@sgi.com wrote: >>> Sorry, I have been distracted away from this regression. This was >>> previously >>> titled "xfs: synchronously write the superblock on unmount". >>> >>> xfs_wait_buftarg() does not wait for the completion of the write of the >>> uncached superblock. This write can race with the shutdown of the log >>> and >>> causes a panic if the write does not win the race. >>> >>> The log write of the superblock is important for possible recovery, >>> but a >>> second syncronous write of the same superblock seems redundant. Would >>> just >>> waiting for the iodone() of the log write before tearing down the log be >>> enough? >> >> Yes. i.e. something like: >> >> /* >> * The superblock buffer is uncached, so xfs_wait_buftarg() >> * will not wait for it. Hence we need to explicitly wait >> * for IO completion on the superblock to occur here. >> */ >> error = xfs_buf_iowait(mp->m_sb_bp); >> if (error) >> AAAAIEEEE! >> >> This fix is also needed in xfs_quiesce_attr() for the freeze and >> ro,remount cases as well. >> >> Cheers, >> >> Dave. >> > > Thanks for the feedback. > > I chose the ugly lock to block on the superblock buffer because it has > XBF_ASYNC set at that point and won't go through the complete() patch. I > will look at turning the XBF_ASYNC flag off and wait on a xfs_buf_iowait(). > > --Mark. Looks like the xfsaild_push() sets the XBF_ASYNC flag on the superblock buffer. Waiting on the xfs_buf_iowait() won't work but have to use the less elegant lock. --Mark. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] xfs: wait for the write of the superblock on unmount 2012-07-18 17:33 ` [RFC] xfs: wait for the write of the superblock on unmount tinguely 2012-07-19 1:58 ` Dave Chinner @ 2012-07-24 13:57 ` Christoph Hellwig 2012-07-24 14:03 ` Mark Tinguely 1 sibling, 1 reply; 6+ messages in thread From: Christoph Hellwig @ 2012-07-24 13:57 UTC (permalink / raw) To: tinguely; +Cc: xfs On Wed, Jul 18, 2012 at 12:33:58PM -0500, tinguely@sgi.com wrote: > Sorry, I have been distracted away from this regression. This was previously > titled "xfs: synchronously write the superblock on unmount". > > xfs_wait_buftarg() does not wait for the completion of the write of the > uncached superblock. This write can race with the shutdown of the log and > causes a panic if the write does not win the race. > > The log write of the superblock is important for possible recovery, but a > second syncronous write of the same superblock seems redundant. Would just > waiting for the iodone() of the log write before tearing down the log be > enough? This doesn't look beautiful, but I suspect there's no good way around it. Can you add your explanation from the reply on why xfs_buf_iowait does not work here to the comment above the lock/unlock pair? With that: Reviewed-by: Christoph Hellwig <hch@lst.de> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] xfs: wait for the write of the superblock on unmount 2012-07-24 13:57 ` Christoph Hellwig @ 2012-07-24 14:03 ` Mark Tinguely 0 siblings, 0 replies; 6+ messages in thread From: Mark Tinguely @ 2012-07-24 14:03 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On 07/24/12 08:57, Christoph Hellwig wrote: > On Wed, Jul 18, 2012 at 12:33:58PM -0500, tinguely@sgi.com wrote: >> Sorry, I have been distracted away from this regression. This was previously >> titled "xfs: synchronously write the superblock on unmount". >> >> xfs_wait_buftarg() does not wait for the completion of the write of the >> uncached superblock. This write can race with the shutdown of the log and >> causes a panic if the write does not win the race. >> >> The log write of the superblock is important for possible recovery, but a >> second syncronous write of the same superblock seems redundant. Would just >> waiting for the iodone() of the log write before tearing down the log be >> enough? > > This doesn't look beautiful, but I suspect there's no good way around > it. Can you add your explanation from the reply on why xfs_buf_iowait > does not work here to the comment above the lock/unlock pair? > > With that: > > Reviewed-by: Christoph Hellwig<hch@lst.de> Thank-you. I will repost with the same xfs_buf_lock() entry to xfs_quiesce_attr(). --Mark. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-24 14:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20120717215957.855744999@tinguelysgi.com>
2012-07-18 17:33 ` [RFC] xfs: wait for the write of the superblock on unmount tinguely
2012-07-19 1:58 ` Dave Chinner
2012-07-20 13:21 ` Mark Tinguely
2012-07-20 21:24 ` Mark Tinguely
2012-07-24 13:57 ` Christoph Hellwig
2012-07-24 14:03 ` Mark Tinguely
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox