xfs: simplify xfs_file_iomap_begin() logic
authorDave Chinner <dchinner@redhat.com>
Wed, 2 May 2018 19:54:53 +0000 (12:54 -0700)
committerDarrick J. Wong <darrick.wong@oracle.com>
Wed, 9 May 2018 17:04:01 +0000 (10:04 -0700)
The current logic that determines whether allocation should be done
has grown somewhat spaghetti like with the addition of IOMAP_NOWAIT
functionality. Separate out each of the different cases into single,
obvious checks to get rid most of the nested IOMAP_NOWAIT checks
in the allocation logic.

Signed-Off-By: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
fs/xfs/xfs_iomap.c

index 046469f..16565da 100644 (file)
@@ -1040,6 +1040,10 @@ xfs_file_iomap_begin(
                        goto out_unlock;
        }
 
+       /* Non-modifying mapping requested, so we are done */
+       if (!(flags & (IOMAP_WRITE | IOMAP_ZERO)))
+               goto out_found;
+
        if (xfs_is_reflink_inode(ip) &&
            ((flags & IOMAP_WRITE) ||
             ((flags & IOMAP_ZERO) && needs_cow_for_zeroing(&imap, nimaps)))) {
@@ -1068,46 +1072,45 @@ xfs_file_iomap_begin(
                length = XFS_FSB_TO_B(mp, end_fsb) - offset;
        }
 
-       if ((flags & IOMAP_WRITE) && imap_needs_alloc(inode, &imap, nimaps)) {
-               /*
-                * If nowait is set bail since we are going to make
-                * allocations.
-                */
-               if (flags & IOMAP_NOWAIT) {
-                       error = -EAGAIN;
-                       goto out_unlock;
-               }
-               /*
-                * We cap the maximum length we map here to MAX_WRITEBACK_PAGES
-                * pages to keep the chunks of work done where somewhat symmetric
-                * with the work writeback does. This is a completely arbitrary
-                * number pulled out of thin air as a best guess for initial
-                * testing.
-                *
-                * Note that the values needs to be less than 32-bits wide until
-                * the lower level functions are updated.
-                */
-               length = min_t(loff_t, length, 1024 * PAGE_SIZE);
-               /*
-                * xfs_iomap_write_direct() expects the shared lock. It
-                * is unlocked on return.
-                */
-               if (lockmode == XFS_ILOCK_EXCL)
-                       xfs_ilock_demote(ip, lockmode);
-               error = xfs_iomap_write_direct(ip, offset, length, &imap,
-                               nimaps);
-               if (error)
-                       return error;
+       /* Don't need to allocate over holes when doing zeroing operations. */
+       if (flags & IOMAP_ZERO)
+               goto out_found;
 
-               iomap->flags = IOMAP_F_NEW;
-               trace_xfs_iomap_alloc(ip, offset, length, 0, &imap);
-       } else {
-               ASSERT(nimaps);
+       if (!imap_needs_alloc(inode, &imap, nimaps))
+               goto out_found;
 
-               xfs_iunlock(ip, lockmode);
-               trace_xfs_iomap_found(ip, offset, length, 0, &imap);
+       /* If nowait is set bail since we are going to make allocations. */
+       if (flags & IOMAP_NOWAIT) {
+               error = -EAGAIN;
+               goto out_unlock;
        }
 
+       /*
+        * We cap the maximum length we map to a sane size  to keep the chunks
+        * of work done where somewhat symmetric with the work writeback does.
+        * This is a completely arbitrary number pulled out of thin air as a
+        * best guess for initial testing.
+        *
+        * Note that the values needs to be less than 32-bits wide until the
+        * lower level functions are updated.
+        */
+       length = min_t(loff_t, length, 1024 * PAGE_SIZE);
+
+       /*
+        * xfs_iomap_write_direct() expects the shared lock. It is unlocked on
+        * return.
+        */
+       if (lockmode == XFS_ILOCK_EXCL)
+               xfs_ilock_demote(ip, lockmode);
+       error = xfs_iomap_write_direct(ip, offset, length, &imap,
+                       nimaps);
+       if (error)
+               return error;
+
+       iomap->flags = IOMAP_F_NEW;
+       trace_xfs_iomap_alloc(ip, offset, length, 0, &imap);
+
+out_finish:
        if (xfs_ipincount(ip) && (ip->i_itemp->ili_fsync_fields
                                & ~XFS_ILOG_TIMESTAMP))
                iomap->flags |= IOMAP_F_DIRTY;
@@ -1117,6 +1120,13 @@ xfs_file_iomap_begin(
        if (shared)
                iomap->flags |= IOMAP_F_SHARED;
        return 0;
+
+out_found:
+       ASSERT(nimaps);
+       xfs_iunlock(ip, lockmode);
+       trace_xfs_iomap_found(ip, offset, length, 0, &imap);
+       goto out_finish;
+
 out_unlock:
        xfs_iunlock(ip, lockmode);
        return error;