Hi, * On Tue, May 08, 2012 at 09:53:21AM +1000, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
On Mon, May 07, 2012 at 02:44:07PM +0530, raghu.prabhu13@xxxxxxxxx wrote:From: Raghavendra D Prabhu <rprabhu@xxxxxxxxxxx> This is to prevent syncing from running ad-infinitum till umount if the disk has been forcefully unplugged. This is to prevent messages like these from being displayed......--- fs/xfs/xfs_sync.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/fs/xfs/xfs_sync.c b/fs/xfs/xfs_sync.c index 205ebcb..7ec412c 100644 --- a/fs/xfs/xfs_sync.c +++ b/fs/xfs/xfs_sync.c @@ -460,6 +460,12 @@ xfs_sync_worker( struct xfs_mount, m_sync_work); int error; + if (!xfs_fs_writable(mp)) { + xfs_err(mp, "Filesystem not writable / already shutdown."); + xfs_syncd_stop(mp); + return; + } +That is going to kill the xfssyncd on read only and frozen filesystems as well as shutdowns, so this is certainly not correct. The xfs_sync_worker should continue to run until the filesystem is unmounted, even if it does nothing when it runs.
Yes, in the next patch I had sent -- http://article.gmane.org/gmane.comp.file-systems.xfs.general/45568 -- I removed xfs_syncd_stop from there and added it under xfs_bwrite. (PS: Adding xfs_syncd_stop in xfs_sync_worker was a very bad decision - it sent few of my kworkers to 'D' state and nearly corrupted that fs)
Indeed, all that is needed in xfs_sync_worker() is this:
- if (!(mp->m_flags & XFS_MOUNT_RDONLY)) {
+ if (!xfs_fs_writable(mp)) {
and the error message won't appear. It fixes the problem for the
shutdown case, as well as handles frozen and read-only filesystems
correctly.
which should take care of xfs_sync_worker, xfs_flush_worker, xfs_iflush and xfs_unmountfs
Does this sound good?
Yeah, I realized that (sent a patch after this http://article.gmane.org/gmane.comp.file-systems.xfs.general/45568)./* dgc: errors ignored here */ if (mp->m_super->s_frozen == SB_UNFROZEN && @@ -551,6 +557,12 @@ xfs_flush_worker( struct xfs_mount *mp = container_of(work, struct xfs_mount, m_flush_work); + if (!xfs_fs_writable(mp)) { + xfs_err(mp, "Filesystem not writable / already shutdown."); + xfs_syncd_stop(mp); + return; + } + xfs_sync_data(mp, SYNC_TRYLOCK); xfs_sync_data(mp, SYNC_TRYLOCK | SYNC_WAIT);This is not necessary, either, because xfs_sync_data() has shutdown checks and xfs_flush_worker() should never be called on a shutdown filesystem....
Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
Regards, -- Raghavendra Prabhu GPG Id : 0xD72BE977 Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 www: wnohang.net
Attachment:
pgpHOmxWq7dFB.pgp
Description: PGP signature
_______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
