Re: [PATCH 4/4] xfs: rewrite and optimize the delalloc write path

[Brian Foster <bfoster@xxxxxxxxxx>]

On Fri, Aug 26, 2016 at 12:03:39PM -0400, Brian Foster wrote:
> On Fri, Aug 26, 2016 at 04:33:44PM +0200, Christoph Hellwig wrote:
> > On Thu, Aug 25, 2016 at 10:37:09AM -0400, Brian Foster wrote:
...
> > (we could potentially re-derive offset_fsb from offset if we don't
> > mind the inefficieny and recalculate maxbytes_fsb.  This already
> > assumes mp is trivially derived from ip)
> > 
> > and return
> > 
> > alloc_blocks, end_fsb
> > 
> > so the function would be quite a monster in terms of its calling
> > convention.  Additionally we'd have the related by not qute the
> > same if blocks around XFS_MOUNT_DFLT_IOSIZE and the isize split
> > over two functions, which doesn't exactly help understanding
> > the flow.
> > 
> 
> Not quite sure I follow the last bit, but I don't necessarily think the
> whole thing has to be boxed into a helper to clean it up. E.g., I'd do
> something like the appended diff (compile tested only).
> 

... and if the function signature is really an issue, trade off idx &
prev for a conditional base preallocation size (applies on top of the
previous diff):

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 45e5268..c748429 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -381,7 +381,7 @@ STATIC xfs_fsblock_t
 xfs_iomap_prealloc_size(
        struct xfs_inode        *ip,
        xfs_off_t               offset,
-       struct xfs_bmbt_irec    *prev)
+       xfs_extlen_t            base)
 {
        struct xfs_mount        *mp = ip->i_mount;
        int                     shift = 0;
@@ -406,8 +406,8 @@ xfs_iomap_prealloc_size(
         * always extends to MAXEXTLEN rather than falling short due to things
         * like stripe unit/width alignment of real extents.
         */
-       if (prev->br_blockcount <= (MAXEXTLEN >> 1))
-               alloc_blocks = prev->br_blockcount << 1;
+       if (base <= (MAXEXTLEN >> 1))
+               alloc_blocks = base << 1;
        else
                alloc_blocks = XFS_B_TO_FSB(mp, offset);
        if (!alloc_blocks)
@@ -506,12 +506,10 @@ xfs_iomap_prealloc(
        struct xfs_inode        *ip,
        loff_t                  offset,
        loff_t                  count,
-       xfs_extnum_t            idx,
-       struct xfs_bmbt_irec    *prev)
+       xfs_extlen_t            base)
 {
        struct xfs_mount        *mp = ip->i_mount;
        xfs_fsblock_t           alloc_blocks;
-       xfs_fileoff_t           offset_fsb = XFS_B_TO_FSBT(mp, offset);
 
        if (offset + count <= XFS_ISIZE(ip))
                return 0;
@@ -526,12 +524,11 @@ xfs_iomap_prealloc(
         */
        if ((mp->m_flags & XFS_MOUNT_DFLT_IOSIZE) ||
            XFS_ISIZE(ip) < XFS_FSB_TO_B(mp, mp->m_dalign) ||
-           idx == 0 ||
-           prev->br_startoff + prev->br_blockcount < offset_fsb)
+           !base)
                alloc_blocks = mp->m_writeio_blocks;
        else
                alloc_blocks =
-                       xfs_iomap_prealloc_size(ip, offset, prev);
+                       xfs_iomap_prealloc_size(ip, offset, base);
 
        return alloc_blocks;
 }
@@ -603,8 +600,15 @@ xfs_file_iomap_begin_delay(
        end_fsb = orig_end_fsb =
                min(XFS_B_TO_FSB(mp, offset + count), maxbytes_fsb);
 
-       if (eof)
-               prealloc = xfs_iomap_prealloc(ip, offset, count, idx, &prev);
+       if (eof) {
+               xfs_extlen_t    base = 0;
+
+               /* use prev extent as base if contiguous */
+               if (idx > 0 &&
+                   prev.br_startoff + prev.br_blockcount < offset_fsb)
+                       base = prev.br_blockcount;
+               prealloc = xfs_iomap_prealloc(ip, offset, count, base);
+       }
        if (prealloc) {
                xfs_off_t       aligned_offset;
                xfs_extlen_t    align;