[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: David Chinner <dgc@sgi.com>
Signed-off-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Tim Shimmin <tes@sgi.com>

1 files changed, 28 insertions, 45 deletions

diff --git a/fs/xfs/xfs_vfsops.c b/fs/xfs/xfs_vfsops.c
index 65c561201cb8..92c1425d06ce 100644
--- a/fs/xfs/xfs_vfsops.c
+++ b/fs/xfs/xfs_vfsops.c
@@ -1128,58 +1128,41 @@ 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);
+                /*
+                 * 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);
                         }
+                        xfs_iunlock(ip, XFS_ILOCK_SHARED);
 
-                        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 (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
+                         * place after this point
+                         */
+                        if (flags & SYNC_IOWAIT)
+                                vn_iowait(vp);
+
+                        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
-                 * place after this point
-                 */
-                if (flags & SYNC_IOWAIT)
-                        vn_iowait(vp);
 
                 if (flags & SYNC_BDFLUSH) {
                         if ((flags & SYNC_ATTR) &&