| Message ID | 20170206074738.13978-4-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Mon, Feb 06, 2017 at 08:47:37AM +0100, Christoph Hellwig wrote: > When we allocate COW fork blocks for direct I/O writes we currently first > create a delayed allocation, and then convert it to a real allocation > once we've got the delayed one. > > As there is no good reason for that this patch instead makes use call > xfs_bmapi_write from the COW allocation path. The only interesting bits > are a few tweaks the low-level allocator to allow for this, most notably > the need to remove the call to xfs_bmap_extsize_align for the cowextsize > in xfs_bmap_btalloc - for the existing convert case it's a no-op, but > for the direct allocation case it would blow up our block reservation > way beyond what we reserved for the transaction. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks good, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/libxfs/xfs_bmap.c | 3 +- > fs/xfs/xfs_reflink.c | 94 +++++++++++++++++++++++++++++++++--------------- > 2 files changed, 68 insertions(+), 29 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index 1566e9d9fd7d..2b4ad8442ec0 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -2909,13 +2909,14 @@ xfs_bmap_add_extent_hole_real( > ASSERT(!isnullstartblock(new->br_startblock)); > ASSERT(!bma->cur || > !(bma->cur->bc_private.b.flags & XFS_BTCUR_BPRV_WASDEL)); > - ASSERT(whichfork != XFS_COW_FORK); > > XFS_STATS_INC(mp, xs_add_exlist); > > state = 0; > if (whichfork == XFS_ATTR_FORK) > state |= BMAP_ATTRFORK; > + if (whichfork == XFS_COW_FORK) > + state |= BMAP_COWFORK; > > /* > * Check and set flags if this segment has a left neighbor. > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c > index e6b8af91e9a4..e577f54beb2f 100644 > --- a/fs/xfs/xfs_reflink.c > +++ b/fs/xfs/xfs_reflink.c > @@ -390,62 +390,100 @@ __xfs_reflink_allocate_cow( > xfs_fileoff_t end_fsb) > { > struct xfs_mount *mp = ip->i_mount; > - struct xfs_bmbt_irec imap; > + struct xfs_bmbt_irec imap, got; > struct xfs_defer_ops dfops; > struct xfs_trans *tp; > xfs_fsblock_t first_block; > - int nimaps = 1, error; > - bool shared; > + int nimaps, error, lockmode; > + bool shared, trimmed; > + xfs_filblks_t resaligned; > + xfs_extlen_t resblks; > + xfs_extnum_t idx; > > - xfs_defer_init(&dfops, &first_block); > + resaligned = xfs_aligned_fsb_count(*offset_fsb, end_fsb - *offset_fsb, > + xfs_get_cowextsz_hint(ip)); > + resblks = XFS_DIOSTRAT_SPACE_RES(mp, resaligned); > > - error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, 0, 0, > - XFS_TRANS_RESERVE, &tp); > + error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, 0, 0, &tp); > if (error) > return error; > > - xfs_ilock(ip, XFS_ILOCK_EXCL); > + lockmode = XFS_ILOCK_EXCL; > + xfs_ilock(ip, lockmode); > > - /* Read extent from the source file. */ > - nimaps = 1; > - error = xfs_bmapi_read(ip, *offset_fsb, end_fsb - *offset_fsb, > - &imap, &nimaps, 0); > - if (error) > - goto out_unlock; > - ASSERT(nimaps == 1); > + /* > + * Even if the extent is not shared we might have a preallocation for > + * it in the COW fork. If so use it. > + */ > + if (xfs_iext_lookup_extent(ip, ip->i_cowfp, *offset_fsb, &idx, &got) && > + got.br_startoff <= *offset_fsb) { > + /* If we have a real allocation in the COW fork we're done. */ > + if (!isnullstartblock(got.br_startblock)) { > + xfs_trim_extent(&got, *offset_fsb, > + end_fsb - *offset_fsb); > + *offset_fsb = got.br_startoff + got.br_blockcount; > + goto out_trans_cancel; > + } > + } else { > + nimaps = 1; > + error = xfs_bmapi_read(ip, *offset_fsb, end_fsb - *offset_fsb, > + &imap, &nimaps, 0); > + if (error) > + goto out_trans_cancel; > + ASSERT(nimaps == 1); > + > + /* Trim the mapping to the nearest shared extent boundary. */ > + error = xfs_reflink_trim_around_shared(ip, &imap, &shared, > + &trimmed); > + if (error) > + goto out_trans_cancel; > + > + if (!shared) { > + *offset_fsb = imap.br_startoff + imap.br_blockcount; > + goto out_trans_cancel; > + } > > - /* Make sure there's a CoW reservation for it. */ > - error = xfs_reflink_reserve_cow(ip, &imap, &shared); > + *offset_fsb = imap.br_startoff; > + end_fsb = imap.br_startoff + imap.br_blockcount; > + } > + > + error = xfs_trans_reserve_quota_nblks(tp, ip, resblks, 0, > + XFS_QMOPT_RES_REGBLKS); > if (error) > goto out_trans_cancel; > > - if (!shared) { > - *offset_fsb = imap.br_startoff + imap.br_blockcount; > - goto out_trans_cancel; > - } > + xfs_trans_ijoin(tp, ip, 0); > + > + xfs_defer_init(&dfops, &first_block); > + nimaps = 1; > > /* Allocate the entire reservation as unwritten blocks. */ > - xfs_trans_ijoin(tp, ip, 0); > - error = xfs_bmapi_write(tp, ip, imap.br_startoff, imap.br_blockcount, > + error = xfs_bmapi_write(tp, ip, *offset_fsb, end_fsb - *offset_fsb, > XFS_BMAPI_COWFORK | XFS_BMAPI_PREALLOC, &first_block, > - XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK), > - &imap, &nimaps, &dfops); > + resblks, &imap, &nimaps, &dfops); > if (error) > - goto out_trans_cancel; > + goto out_bmap_cancel; > > /* Finish up. */ > error = xfs_defer_finish(&tp, &dfops, NULL); > if (error) > - goto out_trans_cancel; > + goto out_bmap_cancel; > > error = xfs_trans_commit(tp); > + if (error) > + goto out_unlock; > > *offset_fsb = imap.br_startoff + imap.br_blockcount; > + > out_unlock: > - xfs_iunlock(ip, XFS_ILOCK_EXCL); > + xfs_iunlock(ip, lockmode); > return error; > -out_trans_cancel: > + > +out_bmap_cancel: > xfs_defer_cancel(&dfops); > + xfs_trans_unreserve_quota_nblks(tp, ip, (long)resblks, 0, > + XFS_QMOPT_RES_REGBLKS); > +out_trans_cancel: > xfs_trans_cancel(tp); > goto out_unlock; > } > -- > 2.11.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index 1566e9d9fd7d..2b4ad8442ec0 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -2909,13 +2909,14 @@ xfs_bmap_add_extent_hole_real( ASSERT(!isnullstartblock(new->br_startblock)); ASSERT(!bma->cur || !(bma->cur->bc_private.b.flags & XFS_BTCUR_BPRV_WASDEL)); - ASSERT(whichfork != XFS_COW_FORK); XFS_STATS_INC(mp, xs_add_exlist); state = 0; if (whichfork == XFS_ATTR_FORK) state |= BMAP_ATTRFORK; + if (whichfork == XFS_COW_FORK) + state |= BMAP_COWFORK; /* * Check and set flags if this segment has a left neighbor. diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c index e6b8af91e9a4..e577f54beb2f 100644 --- a/fs/xfs/xfs_reflink.c +++ b/fs/xfs/xfs_reflink.c @@ -390,62 +390,100 @@ __xfs_reflink_allocate_cow( xfs_fileoff_t end_fsb) { struct xfs_mount *mp = ip->i_mount; - struct xfs_bmbt_irec imap; + struct xfs_bmbt_irec imap, got; struct xfs_defer_ops dfops; struct xfs_trans *tp; xfs_fsblock_t first_block; - int nimaps = 1, error; - bool shared; + int nimaps, error, lockmode; + bool shared, trimmed; + xfs_filblks_t resaligned; + xfs_extlen_t resblks; + xfs_extnum_t idx; - xfs_defer_init(&dfops, &first_block); + resaligned = xfs_aligned_fsb_count(*offset_fsb, end_fsb - *offset_fsb, + xfs_get_cowextsz_hint(ip)); + resblks = XFS_DIOSTRAT_SPACE_RES(mp, resaligned); - error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, 0, 0, - XFS_TRANS_RESERVE, &tp); + error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, 0, 0, &tp); if (error) return error; - xfs_ilock(ip, XFS_ILOCK_EXCL); + lockmode = XFS_ILOCK_EXCL; + xfs_ilock(ip, lockmode); - /* Read extent from the source file. */ - nimaps = 1; - error = xfs_bmapi_read(ip, *offset_fsb, end_fsb - *offset_fsb, - &imap, &nimaps, 0); - if (error) - goto out_unlock; - ASSERT(nimaps == 1); + /* + * Even if the extent is not shared we might have a preallocation for + * it in the COW fork. If so use it. + */ + if (xfs_iext_lookup_extent(ip, ip->i_cowfp, *offset_fsb, &idx, &got) && + got.br_startoff <= *offset_fsb) { + /* If we have a real allocation in the COW fork we're done. */ + if (!isnullstartblock(got.br_startblock)) { + xfs_trim_extent(&got, *offset_fsb, + end_fsb - *offset_fsb); + *offset_fsb = got.br_startoff + got.br_blockcount; + goto out_trans_cancel; + } + } else { + nimaps = 1; + error = xfs_bmapi_read(ip, *offset_fsb, end_fsb - *offset_fsb, + &imap, &nimaps, 0); + if (error) + goto out_trans_cancel; + ASSERT(nimaps == 1); + + /* Trim the mapping to the nearest shared extent boundary. */ + error = xfs_reflink_trim_around_shared(ip, &imap, &shared, + &trimmed); + if (error) + goto out_trans_cancel; + + if (!shared) { + *offset_fsb = imap.br_startoff + imap.br_blockcount; + goto out_trans_cancel; + } - /* Make sure there's a CoW reservation for it. */ - error = xfs_reflink_reserve_cow(ip, &imap, &shared); + *offset_fsb = imap.br_startoff; + end_fsb = imap.br_startoff + imap.br_blockcount; + } + + error = xfs_trans_reserve_quota_nblks(tp, ip, resblks, 0, + XFS_QMOPT_RES_REGBLKS); if (error) goto out_trans_cancel; - if (!shared) { - *offset_fsb = imap.br_startoff + imap.br_blockcount; - goto out_trans_cancel; - } + xfs_trans_ijoin(tp, ip, 0); + + xfs_defer_init(&dfops, &first_block); + nimaps = 1; /* Allocate the entire reservation as unwritten blocks. */ - xfs_trans_ijoin(tp, ip, 0); - error = xfs_bmapi_write(tp, ip, imap.br_startoff, imap.br_blockcount, + error = xfs_bmapi_write(tp, ip, *offset_fsb, end_fsb - *offset_fsb, XFS_BMAPI_COWFORK | XFS_BMAPI_PREALLOC, &first_block, - XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK), - &imap, &nimaps, &dfops); + resblks, &imap, &nimaps, &dfops); if (error) - goto out_trans_cancel; + goto out_bmap_cancel; /* Finish up. */ error = xfs_defer_finish(&tp, &dfops, NULL); if (error) - goto out_trans_cancel; + goto out_bmap_cancel; error = xfs_trans_commit(tp); + if (error) + goto out_unlock; *offset_fsb = imap.br_startoff + imap.br_blockcount; + out_unlock: - xfs_iunlock(ip, XFS_ILOCK_EXCL); + xfs_iunlock(ip, lockmode); return error; -out_trans_cancel: + +out_bmap_cancel: xfs_defer_cancel(&dfops); + xfs_trans_unreserve_quota_nblks(tp, ip, (long)resblks, 0, + XFS_QMOPT_RES_REGBLKS); +out_trans_cancel: xfs_trans_cancel(tp); goto out_unlock; }
When we allocate COW fork blocks for direct I/O writes we currently first create a delayed allocation, and then convert it to a real allocation once we've got the delayed one. As there is no good reason for that this patch instead makes use call xfs_bmapi_write from the COW allocation path. The only interesting bits are a few tweaks the low-level allocator to allow for this, most notably the need to remove the call to xfs_bmap_extsize_align for the cowextsize in xfs_bmap_btalloc - for the existing convert case it's a no-op, but for the direct allocation case it would blow up our block reservation way beyond what we reserved for the transaction. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/libxfs/xfs_bmap.c | 3 +- fs/xfs/xfs_reflink.c | 94 +++++++++++++++++++++++++++++++++--------------- 2 files changed, 68 insertions(+), 29 deletions(-)