Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 40095b64 authored by David Chinner's avatar David Chinner Committed by Tim Shimmin
Browse files

[XFS] Sleeping with the ilock waiting for I/O completion is Bad.



Recent fixes to the filesystem freezing code introduced a vn_iowait call
in the middle of the sync code. Unfortunately, at the point where this
call was added we are holding the ilock. The ilock is needed by I/O
completion for unwritten extent conversion and now updating the file size.
Hence I/o cannot complete if we hold the ilock while waiting for I/O
completion.

Fix up the bug and clean the code up around it.

SGI-PV: 963674
SGI-Modid: xfs-linux-melb:xfs-kern:28566a

Signed-off-by: default avatarDavid Chinner <dgc@sgi.com>
Signed-off-by: default avatarChristoph Hellwig <hch@infradead.org>
Signed-off-by: default avatarTim Shimmin <tes@sgi.com>
parent 4cc929ee
Loading
Loading
Loading
Loading
+28 −45
Original line number Diff line number Diff line
@@ -1128,51 +1128,31 @@ xfs_sync_inodes(
		 * in the inode list.
		 */

		if ((flags & SYNC_CLOSE)  && (vp != NULL)) {
		/*
			 * This is the shutdown case.  We just need to
			 * flush and invalidate all the pages associated
			 * with the inode.  Drop the inode lock since
			 * we can't hold it across calls to the buffer
			 * cache.
			 *
			 * We don't set the VREMAPPING bit in the vnode
			 * here, because we don't hold the vnode lock
			 * exclusively.  It doesn't really matter, though,
			 * because we only come here when we're shutting
			 * down anyway.
			 */
			xfs_iunlock(ip, XFS_ILOCK_SHARED);

			if (XFS_FORCED_SHUTDOWN(mp)) {
				bhv_vop_toss_pages(vp, 0, -1, FI_REMAPF);
			} else {
				error = bhv_vop_flushinval_pages(vp, 0, -1, FI_REMAPF);
			}

			xfs_ilock(ip, XFS_ILOCK_SHARED);

		} else if ((flags & SYNC_DELWRI) && (vp != NULL)) {
			if (VN_DIRTY(vp)) {
				/* We need to have dropped the lock here,
				 * so insert a marker if we have not already
				 * done so.
		 * If we have to flush data or wait for I/O completion
		 * we need to drop the ilock that we currently hold.
		 * If we need to drop the lock, insert a marker if we
		 * have not already done so.
		 */
		if ((flags & (SYNC_CLOSE|SYNC_IOWAIT)) ||
		    ((flags & SYNC_DELWRI) && VN_DIRTY(vp))) {
			if (mount_locked) {
				IPOINTER_INSERT(ip, mp);
			}

				/*
				 * Drop the inode lock since we can't hold it
				 * across calls to the buffer cache.
				 */
			xfs_iunlock(ip, XFS_ILOCK_SHARED);

			if (flags & SYNC_CLOSE) {
				/* Shutdown case. Flush and invalidate. */
				if (XFS_FORCED_SHUTDOWN(mp))
					bhv_vop_toss_pages(vp, 0, -1, FI_REMAPF);
				else
					error = bhv_vop_flushinval_pages(vp, 0,
								-1, FI_REMAPF);
			} else if ((flags & SYNC_DELWRI) && VN_DIRTY(vp)) {
				error = bhv_vop_flush_pages(vp, (xfs_off_t)0,
							-1, fflag, FI_NONE);
				xfs_ilock(ip, XFS_ILOCK_SHARED);
			}

		}
			/*
			 * When freezing, we need to wait ensure all I/O (including direct
			 * I/O) is complete to ensure no further data modification can take
@@ -1181,6 +1161,9 @@ xfs_sync_inodes(
			if (flags & SYNC_IOWAIT)
				vn_iowait(vp);

			xfs_ilock(ip, XFS_ILOCK_SHARED);
		}

		if (flags & SYNC_BDFLUSH) {
			if ((flags & SYNC_ATTR) &&
			    ((ip->i_update_core) ||