Re: [PATCH 31/63] xfs: create delalloc extents in CoW fork

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Tue, Oct 04, 2016 at 12:38:23PM -0400, Brian Foster wrote:
> On Thu, Sep 29, 2016 at 08:09:01PM -0700, Darrick J. Wong wrote:
> > Wire up iomap_begin to detect shared extents and create delayed allocation
> > extents in the CoW fork:
> > 
> >  1) Check if we already have an extent in the COW fork for the area.
> >     If so nothing to do, we can move along.
> >  2) Look up block number for the current extent, and if there is none
> >     it's not shared move along.
> >  3) Unshare the current extent as far as we are going to write into it.
> >     For this we avoid an additional COW fork lookup and use the
> >     information we set aside in step 1) above.
> >  4) Goto 1) unless we've covered the whole range.
> > 
> > Last but not least, this updates the xfs_reflink_reserve_cow_range calling
> > convention to pass a byte offset and length, as that is what both callers
> > expect anyway.  This patch has been refactored considerably as part of the
> > iomap transition.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  fs/xfs/xfs_iomap.c   |   12 ++-
> >  fs/xfs/xfs_reflink.c |  202 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  fs/xfs/xfs_reflink.h |    9 ++
> >  3 files changed, 221 insertions(+), 2 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> > index 59c7beb..e8312b0 100644
> > --- a/fs/xfs/xfs_iomap.c
> > +++ b/fs/xfs/xfs_iomap.c
> > @@ -39,6 +39,7 @@
> >  #include "xfs_quota.h"
> >  #include "xfs_dquot_item.h"
> >  #include "xfs_dquot.h"
> > +#include "xfs_reflink.h"
> >  
> >  
> >  #define XFS_WRITEIO_ALIGN(mp,off)	(((off) >> mp->m_writeio_log) \
> > @@ -961,8 +962,15 @@ xfs_file_iomap_begin(
> >  	if (XFS_FORCED_SHUTDOWN(mp))
> >  		return -EIO;
> >  
> > -	if ((flags & IOMAP_WRITE) &&
> > -	    !IS_DAX(inode) && !xfs_get_extsz_hint(ip)) {
> > +	if ((flags & (IOMAP_WRITE | IOMAP_ZERO)) && xfs_is_reflink_inode(ip)) {
> > +		error = xfs_reflink_reserve_cow_range(ip, offset, length);
> > +		if (error < 0)
> > +			return error;
> > +	}
> > +
> > +	if ((flags & IOMAP_WRITE) && !IS_DAX(inode) &&
> > +		   !xfs_get_extsz_hint(ip)) {
> > +		/* Reserve delalloc blocks for regular writeback. */
> >  		return xfs_file_iomap_begin_delay(inode, offset, length, flags,
> >  				iomap);
> >  	}
> 
> What about the short write case? E.g., do we have to clear out delalloc
> blocks from the cow fork in iomap_end() if we don't end up using them?

Nope, unused blocks sit around in the CoW fork (with the cowextsize hint
set, this happens all the time) so that a subsequent write to an
adjacent file offset lands in the same place as the successful write.
The unused extents get cleaned out when the inode is evicted, we run out
of disk space, or the garbage collector triggers.

> > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> > index 7adbb83..05a7fe6 100644
> > --- a/fs/xfs/xfs_reflink.c
> > +++ b/fs/xfs/xfs_reflink.c
> > @@ -51,6 +51,7 @@
> >  #include "xfs_btree.h"
> >  #include "xfs_bmap_btree.h"
> >  #include "xfs_reflink.h"
> > +#include "xfs_iomap.h"
> >  
> >  /*
> >   * Copy on Write of Shared Blocks
> > @@ -112,3 +113,204 @@
> >   * ioend structure.  Better yet, the more ground we can cover with one
> >   * ioend, the better.
> >   */
> > +
> > +/*
> > + * Given an AG extent, find the lowest-numbered run of shared blocks within
> > + * that range and return the range in fbno/flen.
> > + */
> > +int
> > +xfs_reflink_find_shared(
> > +	struct xfs_mount	*mp,
> > +	xfs_agnumber_t		agno,
> > +	xfs_agblock_t		agbno,
> > +	xfs_extlen_t		aglen,
> > +	xfs_agblock_t		*fbno,
> > +	xfs_extlen_t		*flen,
> > +	bool			find_maximal)
> > +{
> > +	struct xfs_buf		*agbp;
> > +	struct xfs_btree_cur	*cur;
> > +	int			error;
> > +
> > +	error = xfs_alloc_read_agf(mp, NULL, agno, 0, &agbp);
> > +	if (error)
> > +		return error;
> > +
> > +	cur = xfs_refcountbt_init_cursor(mp, NULL, agbp, agno, NULL);
> > +
> > +	error = xfs_refcount_find_shared(cur, agbno, aglen, fbno, flen,
> > +			find_maximal);
> > +
> > +	xfs_btree_del_cursor(cur, error ? XFS_BTREE_ERROR : XFS_BTREE_NOERROR);
> > +
> > +	xfs_buf_relse(agbp);
> > +	return error;
> > +}
> > +
> > +/*
> > + * Trim the mapping to the next block where there's a change in the
> > + * shared/unshared status.  More specifically, this means that we
> > + * find the lowest-numbered extent of shared blocks that coincides with
> > + * the given block mapping.  If the shared extent overlaps the start of
> > + * the mapping, trim the mapping to the end of the shared extent.  If
> > + * the shared region intersects the mapping, trim the mapping to the
> > + * start of the shared extent.  If there are no shared regions that
> > + * overlap, just return the original extent.
> > + */
> > +int
> > +xfs_reflink_trim_around_shared(
> > +	struct xfs_inode	*ip,
> > +	struct xfs_bmbt_irec	*irec,
> > +	bool			*shared,
> > +	bool			*trimmed)
> > +{
> > +	xfs_agnumber_t		agno;
> > +	xfs_agblock_t		agbno;
> > +	xfs_extlen_t		aglen;
> > +	xfs_agblock_t		fbno;
> > +	xfs_extlen_t		flen;
> > +	int			error = 0;
> > +
> > +	/* Holes, unwritten, and delalloc extents cannot be shared */
> > +	if (!xfs_is_reflink_inode(ip) ||
> > +	    ISUNWRITTEN(irec) ||
> > +	    irec->br_startblock == HOLESTARTBLOCK ||
> > +	    irec->br_startblock == DELAYSTARTBLOCK) {
> > +		*shared = false;
> > +		return 0;
> > +	}
> > +
> > +	trace_xfs_reflink_trim_around_shared(ip, irec);
> > +
> > +	agno = XFS_FSB_TO_AGNO(ip->i_mount, irec->br_startblock);
> > +	agbno = XFS_FSB_TO_AGBNO(ip->i_mount, irec->br_startblock);
> > +	aglen = irec->br_blockcount;
> > +
> > +	error = xfs_reflink_find_shared(ip->i_mount, agno, agbno,
> > +			aglen, &fbno, &flen, true);
> > +	if (error)
> > +		return error;
> > +
> > +	*shared = *trimmed = false;
> > +	if (flen == 0) {
> 
> Preferable to use NULLAGBLOCK for this, imo.

Yeah, I will look into changing this.

> > +		/* No shared blocks at all. */
> > +		return 0;
> > +	} else if (fbno == agbno) {
> > +		/* The start of this extent is shared. */
> > +		irec->br_blockcount = flen;
> > +		*shared = true;
> > +		*trimmed = true;
> 
> Why do we set trimmed based solely on fbno == agbno? Is that valid if
> the bmapbt extent exactly matches the refcntbt extent and we thus don't
> actually modify the extent (e.g., br_blockcount == flen)? It's hard to
> tell because trimmed looks unused (to this point?), so I could just
> misunderstand the meaning.

You're right, we don't have to set trimmed if flen == aglen.

> > +		return 0;
> > +	} else {
> > +		/* There's a shared extent midway through this extent. */
> > +		irec->br_blockcount = fbno - agbno;
> 
> Don't we have to push the startblock forward in this case?
> 
> Oh, I see. We trim the unshared length to push the fileoffset fsb to the
> start of the shared region for the next iteration.

Yep.  I'll clarify the comment.

--D

> 
> Brian
> 
> > +		*trimmed = true;
> > +		return 0;
> > +	}
> > +}
> > +
> > +/* Create a CoW reservation for a range of blocks within a file. */
> > +static int
> > +__xfs_reflink_reserve_cow(
> > +	struct xfs_inode	*ip,
> > +	xfs_fileoff_t		*offset_fsb,
> > +	xfs_fileoff_t		end_fsb)
> > +{
> > +	struct xfs_bmbt_irec	got, prev, imap;
> > +	xfs_fileoff_t		orig_end_fsb;
> > +	int			nimaps, eof = 0, error = 0;
> > +	bool			shared = false, trimmed = false;
> > +	xfs_extnum_t		idx;
> > +
> > +	/* Already reserved?  Skip the refcount btree access. */
> > +	xfs_bmap_search_extents(ip, *offset_fsb, XFS_COW_FORK, &eof, &idx,
> > +			&got, &prev);
> > +	if (!eof && got.br_startoff <= *offset_fsb) {
> > +		end_fsb = orig_end_fsb = got.br_startoff + got.br_blockcount;
> > +		trace_xfs_reflink_cow_found(ip, &got);
> > +		goto done;
> > +	}
> > +
> > +	/* 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);
> > +
> > +	/* Trim the mapping to the nearest shared extent boundary. */
> > +	error = xfs_reflink_trim_around_shared(ip, &imap, &shared, &trimmed);
> > +	if (error)
> > +		goto out_unlock;
> > +
> > +	end_fsb = orig_end_fsb = imap.br_startoff + imap.br_blockcount;
> > +
> > +	/* Not shared?  Just report the (potentially capped) extent. */
> > +	if (!shared)
> > +		goto done;
> > +
> > +	/*
> > +	 * Fork all the shared blocks from our write offset until the end of
> > +	 * the extent.
> > +	 */
> > +	error = xfs_qm_dqattach_locked(ip, 0);
> > +	if (error)
> > +		goto out_unlock;
> > +
> > +retry:
> > +	error = xfs_bmapi_reserve_delalloc(ip, XFS_COW_FORK, *offset_fsb,
> > +			end_fsb - *offset_fsb, &got,
> > +			&prev, &idx, eof);
> > +	switch (error) {
> > +	case 0:
> > +		break;
> > +	case -ENOSPC:
> > +	case -EDQUOT:
> > +		/* retry without any preallocation */
> > +		trace_xfs_reflink_cow_enospc(ip, &imap);
> > +		if (end_fsb != orig_end_fsb) {
> > +			end_fsb = orig_end_fsb;
> > +			goto retry;
> > +		}
> > +		/*FALLTHRU*/
> > +	default:
> > +		goto out_unlock;
> > +	}
> > +
> > +	trace_xfs_reflink_cow_alloc(ip, &got);
> > +done:
> > +	*offset_fsb = end_fsb;
> > +out_unlock:
> > +	return error;
> > +}
> > +
> > +/* Create a CoW reservation for part of a file. */
> > +int
> > +xfs_reflink_reserve_cow_range(
> > +	struct xfs_inode	*ip,
> > +	xfs_off_t		offset,
> > +	xfs_off_t		count)
> > +{
> > +	struct xfs_mount	*mp = ip->i_mount;
> > +	xfs_fileoff_t		offset_fsb, end_fsb;
> > +	int			error;
> > +
> > +	trace_xfs_reflink_reserve_cow_range(ip, offset, count);
> > +
> > +	offset_fsb = XFS_B_TO_FSBT(mp, offset);
> > +	end_fsb = XFS_B_TO_FSB(mp, offset + count);
> > +
> > +	xfs_ilock(ip, XFS_ILOCK_EXCL);
> > +	while (offset_fsb < end_fsb) {
> > +		error = __xfs_reflink_reserve_cow(ip, &offset_fsb, end_fsb);
> > +		if (error) {
> > +			trace_xfs_reflink_reserve_cow_range_error(ip, error,
> > +				_RET_IP_);
> > +			break;
> > +		}
> > +	}
> > +	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > +
> > +	return error;
> > +}
> > diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
> > index 820b151..f824f87 100644
> > --- a/fs/xfs/xfs_reflink.h
> > +++ b/fs/xfs/xfs_reflink.h
> > @@ -20,4 +20,13 @@
> >  #ifndef __XFS_REFLINK_H
> >  #define __XFS_REFLINK_H 1
> >  
> > +extern int xfs_reflink_find_shared(struct xfs_mount *mp, xfs_agnumber_t agno,
> > +		xfs_agblock_t agbno, xfs_extlen_t aglen, xfs_agblock_t *fbno,
> > +		xfs_extlen_t *flen, bool find_maximal);
> > +extern int xfs_reflink_trim_around_shared(struct xfs_inode *ip,
> > +		struct xfs_bmbt_irec *irec, bool *shared, bool *trimmed);
> > +
> > +extern int xfs_reflink_reserve_cow_range(struct xfs_inode *ip,
> > +		xfs_off_t offset, xfs_off_t count);
> > +
> >  #endif /* __XFS_REFLINK_H */
> > 
> > --
> > 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