[PATCH v2 0/5] xfs: basic cow fork speculative preallocation

[PATCH v2 0/5] xfs: basic cow fork speculative preallocation

[Brian Foster]

Hi all,

Here's v2 of the cow fork speculative prealloc patches. No major changes
here, as outlined below. Thoughts on any of this?

Brian

v2:
- Rebased to latest for-next.
- Dropped fiemap COW fork patch.
- Added Reviewed-by tags from previous series.
v1: http://www.spinics.net/lists/linux-xfs/msg02466.html
- Rebase onto for-next (new iext lookup helpers).
- Split off speculative prealloc refactor patches into separate series.
- Prepend the COW fork fiemap patch.
- Create xfs_iomap_search_extents() helper for data/COW fork extent
  lookup.
- Reuse xfs_iomap_prealloc_size() to incorporate prealloc throttling.
- Added patch to reclaim cowblocks on write failure due to ENOSPC.
rfc: http://www.spinics.net/lists/linux-xfs/msg02152.html

Brian Foster (5):
  xfs: refactor iomap delalloc existing extent search into helper
  xfs: logically separate iomap range from allocation range
  xfs: reuse iomap delalloc code for COW fork reservation
  xfs: free cowblocks and retry on buffered write ENOSPC
  xfs: implement basic COW fork speculative preallocation

 fs/xfs/xfs_file.c  |   1 +
 fs/xfs/xfs_iomap.c | 270 +++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 193 insertions(+), 78 deletions(-)

-- 
2.7.4

[PATCH v2 1/5] xfs: refactor iomap delalloc existing extent search into helper

[Brian Foster]

In preparation for reuse of the existing delayed allocation code for COW
reservation, factor out the part of xfs_file_iomap_begin_delay()
responsible for existing extent lookup into a new helper. This patch
does not change behavior.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_iomap.c | 72 ++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 57 insertions(+), 15 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 0d14742..ec86262 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -522,6 +522,48 @@ xfs_iomap_prealloc_size(
 	return alloc_blocks;
 }
 
+/*
+ * Prepare to handle delayed allocation based on whether extents exist in the
+ * inode data and COW forks. If an existing data extent is shared, determine
+ * whether COW fork reservation is necessary.
+ *
+ * The 'found' parameter indicates whether a writable mapping was found. If the
+ * mapping is shared, a COW reservation is performed for the corresponding
+ * range.
+ */
+static int
+xfs_iomap_search_extents(
+	struct xfs_inode	*ip,
+	xfs_fileoff_t		offset_fsb,
+	xfs_fileoff_t		end_fsb,
+	int			*eof,
+	int			*idx,
+	struct xfs_bmbt_irec	*got,
+	bool			*found)		/* found usable extent */
+{
+	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
+	int			error = 0;
+
+	*found = false;
+
+	*eof = !xfs_iext_lookup_extent(ip, ifp, offset_fsb, idx, got);
+	if (*eof || got->br_startoff > offset_fsb)
+		return 0;
+
+	if (xfs_is_reflink_inode(ip)) {
+		bool		shared;
+
+		xfs_trim_extent(got, offset_fsb, end_fsb - offset_fsb);
+		error = xfs_reflink_reserve_cow(ip, got, &shared);
+		if (error)
+			return error;
+	}
+
+	*found = true;
+
+	return error;
+}
+
 static int
 xfs_file_iomap_begin_delay(
 	struct inode		*inode,
@@ -533,18 +575,22 @@ xfs_file_iomap_begin_delay(
 	struct xfs_inode	*ip = XFS_I(inode);
 	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
-	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
+	xfs_fileoff_t		offset_fsb;
+	xfs_fileoff_t		end_fsb;
 	xfs_fileoff_t		maxbytes_fsb =
 		XFS_B_TO_FSB(mp, mp->m_super->s_maxbytes);
-	xfs_fileoff_t		end_fsb;
 	int			error = 0, eof = 0;
 	struct xfs_bmbt_irec	got;
 	xfs_extnum_t		idx;
 	xfs_fsblock_t		prealloc_blocks = 0;
+	bool			found;
 
 	ASSERT(!XFS_IS_REALTIME_INODE(ip));
 	ASSERT(!xfs_get_extsz_hint(ip));
 
+	offset_fsb = XFS_B_TO_FSBT(mp, offset);
+	end_fsb = min(XFS_B_TO_FSB(mp, offset + count), maxbytes_fsb);
+
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 
 	if (unlikely(XFS_TEST_ERROR(
@@ -564,19 +610,15 @@ xfs_file_iomap_begin_delay(
 			goto out_unlock;
 	}
 
-	eof = !xfs_iext_lookup_extent(ip, ifp, offset_fsb, &idx, &got);
-	if (!eof && got.br_startoff <= offset_fsb) {
-		if (xfs_is_reflink_inode(ip)) {
-			bool		shared;
-
-			end_fsb = min(XFS_B_TO_FSB(mp, offset + count),
-					maxbytes_fsb);
-			xfs_trim_extent(&got, offset_fsb, end_fsb - offset_fsb);
-			error = xfs_reflink_reserve_cow(ip, &got, &shared);
-			if (error)
-				goto out_unlock;
-		}
-
+	/*
+	 * Search for preexisting extents. If an existing data extent is shared,
+	 * this will perform COW fork reservation.
+	 */
+	error = xfs_iomap_search_extents(ip, offset_fsb, end_fsb, &eof, &idx,
+					 &got, &found);
+	if (error)
+		goto out_unlock;
+	if (found) {
 		trace_xfs_iomap_found(ip, offset, count, 0, &got);
 		goto done;
 	}
-- 
2.7.4

[PATCH v2 2/5] xfs: logically separate iomap range from allocation range

[Brian Foster]

xfs_file_iomap_begin_delay() uses the got variable for the extent
returned by the in-core extent list lookup, the allocated extent
returned by xfs_bmapi_reserve_delalloc() and the extent mapped to the
iomap data structure for the caller.

This is fine for current usage, but doesn't work for COW fork
reservation. In that case, the extent returned by
xfs_bmapi_reserve_delalloc() is the COW reservation and not the extent
to be mapped to the iomap. Instead, the shared data fork extent should
be trimmed appropriately and mapped to the iomap.

To prepare *_begin_delay() to support COW fork reservation, add a new
imap variable to separately track the data fork extent (for iomap) from
the extent returned from the lookup. The latter extent may refer to
either the data or COW fork in the future.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_iomap.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index ec86262..8791ed5 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -536,6 +536,7 @@ xfs_iomap_search_extents(
 	struct xfs_inode	*ip,
 	xfs_fileoff_t		offset_fsb,
 	xfs_fileoff_t		end_fsb,
+	struct xfs_bmbt_irec	*imap,		/* for iomap mapping */
 	int			*eof,
 	int			*idx,
 	struct xfs_bmbt_irec	*got,
@@ -546,15 +547,21 @@ xfs_iomap_search_extents(
 
 	*found = false;
 
-	*eof = !xfs_iext_lookup_extent(ip, ifp, offset_fsb, idx, got);
-	if (*eof || got->br_startoff > offset_fsb)
+	/*
+	 * Look up a preexisting extent directly into imap. Set got for the
+	 * bmapi delalloc call if nothing is found.
+	 */
+	*eof = !xfs_iext_lookup_extent(ip, ifp, offset_fsb, idx, imap);
+	if (*eof || imap->br_startoff > offset_fsb) {
+		*got = *imap;
 		return 0;
+	}
 
 	if (xfs_is_reflink_inode(ip)) {
 		bool		shared;
 
-		xfs_trim_extent(got, offset_fsb, end_fsb - offset_fsb);
-		error = xfs_reflink_reserve_cow(ip, got, &shared);
+		xfs_trim_extent(imap, offset_fsb, end_fsb - offset_fsb);
+		error = xfs_reflink_reserve_cow(ip, imap, &shared);
 		if (error)
 			return error;
 	}
@@ -580,6 +587,7 @@ xfs_file_iomap_begin_delay(
 	xfs_fileoff_t		maxbytes_fsb =
 		XFS_B_TO_FSB(mp, mp->m_super->s_maxbytes);
 	int			error = 0, eof = 0;
+	struct xfs_bmbt_irec	imap;
 	struct xfs_bmbt_irec	got;
 	xfs_extnum_t		idx;
 	xfs_fsblock_t		prealloc_blocks = 0;
@@ -614,12 +622,12 @@ xfs_file_iomap_begin_delay(
 	 * Search for preexisting extents. If an existing data extent is shared,
 	 * this will perform COW fork reservation.
 	 */
-	error = xfs_iomap_search_extents(ip, offset_fsb, end_fsb, &eof, &idx,
-					 &got, &found);
+	error = xfs_iomap_search_extents(ip, offset_fsb, end_fsb, &imap, &eof,
+					 &idx, &got, &found);
 	if (error)
 		goto out_unlock;
 	if (found) {
-		trace_xfs_iomap_found(ip, offset, count, 0, &got);
+		trace_xfs_iomap_found(ip, offset, count, 0, &imap);
 		goto done;
 	}
 
@@ -680,17 +688,18 @@ xfs_file_iomap_begin_delay(
 	}
 
 	trace_xfs_iomap_alloc(ip, offset, count, 0, &got);
+	imap = got;
 done:
-	if (isnullstartblock(got.br_startblock))
-		got.br_startblock = DELAYSTARTBLOCK;
+	if (isnullstartblock(imap.br_startblock))
+		imap.br_startblock = DELAYSTARTBLOCK;
 
-	if (!got.br_startblock) {
-		error = xfs_alert_fsblock_zero(ip, &got);
+	if (!imap.br_startblock) {
+		error = xfs_alert_fsblock_zero(ip, &imap);
 		if (error)
 			goto out_unlock;
 	}
 
-	xfs_bmbt_to_iomap(ip, iomap, &got);
+	xfs_bmbt_to_iomap(ip, iomap, &imap);
 
 out_unlock:
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
-- 
2.7.4

[PATCH v2 3/5] xfs: reuse iomap delalloc code for COW fork reservation

[Brian Foster]

COW fork reservation (delayed allocation) is implemented in
xfs_reflink_reserve_cow() and is generally based on the traditional data
fork delalloc logic in xfs_file_iomap_begin_delay(). In preparation for
further changes to implement more aggressive COW fork preallocation,
refactor the COW reservation code to reuse xfs_file_iomap_begin_delay()
for data fork allocation as well as COW fork reservation. This patch
does not change behavior.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_iomap.c | 74 ++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 55 insertions(+), 19 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 8791ed5..19b7eb0 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -527,9 +527,8 @@ xfs_iomap_prealloc_size(
  * inode data and COW forks. If an existing data extent is shared, determine
  * whether COW fork reservation is necessary.
  *
- * The 'found' parameter indicates whether a writable mapping was found. If the
- * mapping is shared, a COW reservation is performed for the corresponding
- * range.
+ * The 'found' parameter indicates whether a writable mapping was found. If a
+ * shared mapping exists, found is set only if COW reservation exists as well.
  */
 static int
 xfs_iomap_search_extents(
@@ -540,12 +539,14 @@ xfs_iomap_search_extents(
 	int			*eof,
 	int			*idx,
 	struct xfs_bmbt_irec	*got,
+	bool			*shared,
 	bool			*found)		/* found usable extent */
 {
 	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
 	int			error = 0;
+	bool			trimmed;
 
-	*found = false;
+	*shared = *found = false;
 
 	/*
 	 * Look up a preexisting extent directly into imap. Set got for the
@@ -556,17 +557,37 @@ xfs_iomap_search_extents(
 		*got = *imap;
 		return 0;
 	}
+	if (!xfs_is_reflink_inode(ip)) {
+		*found = true;
+		return 0;
+	}
 
-	if (xfs_is_reflink_inode(ip)) {
-		bool		shared;
-
-		xfs_trim_extent(imap, offset_fsb, end_fsb - offset_fsb);
-		error = xfs_reflink_reserve_cow(ip, imap, &shared);
-		if (error)
-			return error;
+	/*
+	 * Found a data extent but we don't know if it is shared. If an extent
+	 * exists in the cow fork, assume that it is. Use got for this lookup as
+	 * imap must retain the data mapping.
+	 */
+	xfs_trim_extent(imap, offset_fsb, end_fsb - offset_fsb);
+	ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
+	*eof = !xfs_iext_lookup_extent(ip, ifp, offset_fsb, idx, got);
+	if (!*eof && got->br_startoff <= imap->br_startoff) {
+		trace_xfs_reflink_cow_found(ip, got);
+		xfs_trim_extent(imap, got->br_startoff, got->br_blockcount);
+		*found = true;
+		return 0;
 	}
 
-	*found = true;
+	/*
+	 * There is no existing cow fork extent. We have to explicitly check if
+	 * the data extent is shared to determine whether COW fork reservation
+	 * is required to map the data extent. Trim the mapping to the next
+	 * (un)shared boundary at the same time.
+	 */
+	error = xfs_reflink_trim_around_shared(ip, imap, shared, &trimmed);
+	if (error)
+		return error;
+	if (!*shared)
+		*found = true;
 
 	return error;
 }
@@ -587,11 +608,13 @@ xfs_file_iomap_begin_delay(
 	xfs_fileoff_t		maxbytes_fsb =
 		XFS_B_TO_FSB(mp, mp->m_super->s_maxbytes);
 	int			error = 0, eof = 0;
+	int			fork = XFS_DATA_FORK;
 	struct xfs_bmbt_irec	imap;
 	struct xfs_bmbt_irec	got;
 	xfs_extnum_t		idx;
 	xfs_fsblock_t		prealloc_blocks = 0;
 	bool			found;
+	bool			shared;
 
 	ASSERT(!XFS_IS_REALTIME_INODE(ip));
 	ASSERT(!xfs_get_extsz_hint(ip));
@@ -620,16 +643,20 @@ xfs_file_iomap_begin_delay(
 
 	/*
 	 * Search for preexisting extents. If an existing data extent is shared,
-	 * this will perform COW fork reservation.
+	 * switch to the COW fork for COW reservation.
 	 */
 	error = xfs_iomap_search_extents(ip, offset_fsb, end_fsb, &imap, &eof,
-					 &idx, &got, &found);
+					 &idx, &got, &shared, &found);
 	if (error)
 		goto out_unlock;
 	if (found) {
 		trace_xfs_iomap_found(ip, offset, count, 0, &imap);
 		goto done;
 	}
+	if (shared) {
+		end_fsb = imap.br_startoff + imap.br_blockcount;
+		fork = XFS_COW_FORK;
+	}
 
 	error = xfs_qm_dqattach_locked(ip, 0);
 	if (error)
@@ -645,9 +672,10 @@ xfs_file_iomap_begin_delay(
 	 * the lower level functions are updated.
 	 */
 	count = min_t(loff_t, count, 1024 * PAGE_SIZE);
-	end_fsb = min(XFS_B_TO_FSB(mp, offset + count), maxbytes_fsb);
+	end_fsb = min(end_fsb, XFS_B_TO_FSB(mp, offset + count));
+	xfs_trim_extent(&imap, offset_fsb, end_fsb - offset_fsb);
 
-	if (eof) {
+	if (eof && fork == XFS_DATA_FORK) {
 		prealloc_blocks = xfs_iomap_prealloc_size(ip, offset, count, idx);
 		if (prealloc_blocks) {
 			xfs_extlen_t	align;
@@ -669,7 +697,7 @@ xfs_file_iomap_begin_delay(
 	}
 
 retry:
-	error = xfs_bmapi_reserve_delalloc(ip, XFS_DATA_FORK, offset_fsb,
+	error = xfs_bmapi_reserve_delalloc(ip, fork, offset_fsb,
 			end_fsb - offset_fsb, prealloc_blocks, &got, &idx, eof);
 	switch (error) {
 	case 0:
@@ -687,8 +715,16 @@ xfs_file_iomap_begin_delay(
 		goto out_unlock;
 	}
 
-	trace_xfs_iomap_alloc(ip, offset, count, 0, &got);
-	imap = got;
+	/*
+	 * For the data fork, the iomap mapping is simply the extent that was
+	 * returned from the delalloc request. Otherwise, use imap as it refers
+	 * to the data fork but is trimmed according to the shared state.
+	 */
+	if (fork == XFS_DATA_FORK) {
+		trace_xfs_iomap_alloc(ip, offset, count, 0, &got);
+		imap = got;
+	} else
+		trace_xfs_reflink_cow_alloc(ip, &got);
 done:
 	if (isnullstartblock(imap.br_startblock))
 		imap.br_startblock = DELAYSTARTBLOCK;
-- 
2.7.4

[PATCH v2 4/5] xfs: free cowblocks and retry on buffered write ENOSPC

[Brian Foster]

XFS runs an eofblocks reclaim scan before returning an ENOSPC error to
userspace for buffered writes. This facilitates aggressive speculative
preallocation without causing user visible side effects such as
premature ENOSPC.

Run a cowblocks scan in the same situation to reclaim lingering COW fork
preallocation throughout the filesystem.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_file.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index bbb9eb6..ef7eaa6 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -655,6 +655,7 @@ xfs_file_buffered_aio_write(
 		eofb.eof_scan_owner = ip->i_ino; /* for locking */
 		eofb.eof_flags = XFS_EOF_FLAGS_SYNC;
 		xfs_icache_free_eofblocks(ip->i_mount, &eofb);
+		xfs_icache_free_cowblocks(ip->i_mount, &eofb);
 		goto write_retry;
 	}
 
-- 
2.7.4

[PATCH v2 5/5] xfs: implement basic COW fork speculative preallocation

[Brian Foster]

COW fork preallocation is currently limited to the value specified by
the COW extent size hint. This is typically much less aggressive than
traditional data fork speculative preallocation performed when
sufficiently large files are extended.

A file extension based algorithm is not relevant for COW reservation
since, by design, COW reservation never involves extending the size of a
file. That said, we can be more aggressive with COW fork preallocation
given that we support cowblocks inode tagging and reclaim
infrastructure. This provides the ability to reclaim COW fork
preallocation in the background or on demand.

Add a simple COW fork speculative preallocation algorithm. Extend COW
fork reservations due to file writes out to the next data fork extent,
unshared boundary or the next preexisting extent in the COW fork,
whichever limit we hit first. This provides a prealloc algorithm that is
based on the size of preexisting extents, similar to the existing
post-eof speculative preallocation algorithm.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_iomap.c | 139 ++++++++++++++++++++++++++++++++---------------------
 1 file changed, 83 insertions(+), 56 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 19b7eb0..ca137b7 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -395,57 +395,84 @@ xfs_iomap_prealloc_size(
 	struct xfs_inode	*ip,
 	loff_t			offset,
 	loff_t			count,
+	int			fork,
 	xfs_extnum_t		idx)
 {
 	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
 	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
-	struct xfs_bmbt_irec	prev;
+	struct xfs_bmbt_irec	base;
 	int			shift = 0;
 	int64_t			freesp;
 	xfs_fsblock_t		qblocks;
 	int			qshift = 0;
 	xfs_fsblock_t		alloc_blocks = 0;
+	int			error = 0;
 
-	if (offset + count <= XFS_ISIZE(ip))
-		return 0;
-
-	if (!(mp->m_flags & XFS_MOUNT_DFLT_IOSIZE) &&
-	    (XFS_ISIZE(ip) < XFS_FSB_TO_B(mp, mp->m_writeio_blocks)))
+	if (fork == XFS_DATA_FORK && offset + count <= XFS_ISIZE(ip))
 		return 0;
 
-	/*
-	 * If an explicit allocsize is set, the file is small, or we
-	 * are writing behind a hole, then use the minimum prealloc:
-	 */
-	if ((mp->m_flags & XFS_MOUNT_DFLT_IOSIZE) ||
-	    XFS_ISIZE(ip) < XFS_FSB_TO_B(mp, mp->m_dalign) ||
-	    !xfs_iext_get_extent(ifp, idx - 1, &prev) ||
-	    prev.br_startoff + prev.br_blockcount < offset_fsb)
+	if (mp->m_flags & XFS_MOUNT_DFLT_IOSIZE)
 		return mp->m_writeio_blocks;
 
 	/*
-	 * Determine the initial size of the preallocation. We are beyond the
-	 * current EOF here, but we need to take into account whether this is
-	 * a sparse write or an extending write when determining the
-	 * preallocation size.  Hence we need to look up the extent that ends
-	 * at the current write offset and use the result to determine the
-	 * preallocation size.
-	 *
-	 * If the extent is a hole, then preallocation is essentially disabled.
-	 * Otherwise we take the size of the preceding data extent as the basis
-	 * for the preallocation size. If the size of the extent is greater than
-	 * half the maximum extent length, then use the current offset as the
-	 * basis. This ensures that for large files the preallocation size
-	 * always extends to MAXEXTLEN rather than falling short due to things
-	 * like stripe unit/width alignment of real extents.
+	 * Determine the initial size of the preallocation depending on which
+	 * fork we are in.
 	 */
-	if (prev.br_blockcount <= (MAXEXTLEN >> 1))
-		alloc_blocks = prev.br_blockcount << 1;
-	else
-		alloc_blocks = XFS_B_TO_FSB(mp, offset);
-	if (!alloc_blocks)
-		goto check_writeio;
+	if (fork == XFS_DATA_FORK) {
+		if (XFS_ISIZE(ip) < XFS_FSB_TO_B(mp, mp->m_writeio_blocks))
+			return 0;
+
+		/*
+		 * Use the minimum prealloc if the file is small or we're
+		 * writing behind a hole.
+		 */
+		if (XFS_ISIZE(ip) < XFS_FSB_TO_B(mp, mp->m_dalign) ||
+		    !xfs_iext_get_extent(ifp, idx - 1, &base) ||
+		    base.br_startoff + base.br_blockcount < offset_fsb)
+			return mp->m_writeio_blocks;
+
+		/*
+		 * Use the size of the preceding data extent as the basis for
+		 * the preallocation size. If the size of the extent is greater
+		 * than half the maximum extent length, then use the current
+		 * offset as the basis. This ensures that for large files the
+		 * preallocation size always extends to MAXEXTLEN rather than
+		 * falling short due to things like stripe unit/width alignment
+		 * of real extents.
+		 */
+		if (base.br_blockcount <= (MAXEXTLEN >> 1))
+			alloc_blocks = base.br_blockcount << 1;
+		else
+			alloc_blocks = XFS_B_TO_FSB(mp, offset);
+		if (!alloc_blocks)
+			goto check_writeio;
+	} else {
+		xfs_extlen_t		len;
+		int			didx;
+		bool			shared, trimmed;
+
+		/* use the data fork extent as the basis for preallocation */
+		shared = xfs_iext_lookup_extent(ip, ifp, offset_fsb, &didx,
+						&base);
+		ASSERT(shared && offset_fsb >= base.br_startoff);
+
+		/*
+		 * Truncate the data fork extent to the next unshared boundary.
+		 * This defines the maximum COW fork preallocation as we do not
+		 * copy-on-write unshared blocks.
+		 */
+		len = base.br_blockcount - (offset_fsb - base.br_startoff);
+		xfs_trim_extent(&base, offset_fsb, len);
+		error = xfs_reflink_trim_around_shared(ip, &base, &shared,
+						       &trimmed);
+		ASSERT(!error && shared);
+		if (!error)
+			alloc_blocks = base.br_startoff + base.br_blockcount -
+					XFS_B_TO_FSB(mp, offset + count);
+		if (!alloc_blocks)
+			return 0;
+	}
 	qblocks = alloc_blocks;
 
 	/*
@@ -501,7 +528,7 @@ xfs_iomap_prealloc_size(
 	 * rounddown_pow_of_two() returns an undefined result if we pass in
 	 * alloc_blocks = 0.
 	 */
-	if (alloc_blocks)
+	if (alloc_blocks && fork == XFS_DATA_FORK)
 		alloc_blocks = rounddown_pow_of_two(alloc_blocks);
 	if (alloc_blocks > MAXEXTLEN)
 		alloc_blocks = MAXEXTLEN;
@@ -540,13 +567,13 @@ xfs_iomap_search_extents(
 	int			*idx,
 	struct xfs_bmbt_irec	*got,
 	bool			*shared,
+	bool			*trimmed,
 	bool			*found)		/* found usable extent */
 {
 	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
 	int			error = 0;
-	bool			trimmed;
 
-	*shared = *found = false;
+	*shared = *trimmed = *found = false;
 
 	/*
 	 * Look up a preexisting extent directly into imap. Set got for the
@@ -583,7 +610,7 @@ xfs_iomap_search_extents(
 	 * is required to map the data extent. Trim the mapping to the next
 	 * (un)shared boundary at the same time.
 	 */
-	error = xfs_reflink_trim_around_shared(ip, imap, shared, &trimmed);
+	error = xfs_reflink_trim_around_shared(ip, imap, shared, trimmed);
 	if (error)
 		return error;
 	if (!*shared)
@@ -614,7 +641,7 @@ xfs_file_iomap_begin_delay(
 	xfs_extnum_t		idx;
 	xfs_fsblock_t		prealloc_blocks = 0;
 	bool			found;
-	bool			shared;
+	bool			shared, trimmed;
 
 	ASSERT(!XFS_IS_REALTIME_INODE(ip));
 	ASSERT(!xfs_get_extsz_hint(ip));
@@ -646,7 +673,7 @@ xfs_file_iomap_begin_delay(
 	 * switch to the COW fork for COW reservation.
 	 */
 	error = xfs_iomap_search_extents(ip, offset_fsb, end_fsb, &imap, &eof,
-					 &idx, &got, &shared, &found);
+					 &idx, &got, &shared, &trimmed, &found);
 	if (error)
 		goto out_unlock;
 	if (found) {
@@ -675,25 +702,25 @@ xfs_file_iomap_begin_delay(
 	end_fsb = min(end_fsb, XFS_B_TO_FSB(mp, offset + count));
 	xfs_trim_extent(&imap, offset_fsb, end_fsb - offset_fsb);
 
-	if (eof && fork == XFS_DATA_FORK) {
-		prealloc_blocks = xfs_iomap_prealloc_size(ip, offset, count, idx);
-		if (prealloc_blocks) {
-			xfs_extlen_t	align;
-			xfs_off_t	end_offset;
-			xfs_fileoff_t	p_end_fsb;
+	if ((fork == XFS_DATA_FORK && eof) ||
+	    (fork == XFS_COW_FORK && !trimmed))
+		prealloc_blocks = xfs_iomap_prealloc_size(ip, offset, count,
+							  fork, idx);
+	if (prealloc_blocks) {
+		xfs_extlen_t	align;
+		xfs_off_t	end_offset;
+		xfs_fileoff_t	p_end_fsb;
 
-			end_offset = XFS_WRITEIO_ALIGN(mp, offset + count - 1);
-			p_end_fsb = XFS_B_TO_FSBT(mp, end_offset) +
-					prealloc_blocks;
+		end_offset = XFS_WRITEIO_ALIGN(mp, offset + count - 1);
+		p_end_fsb = XFS_B_TO_FSBT(mp, end_offset) + prealloc_blocks;
 
-			align = xfs_eof_alignment(ip, 0);
-			if (align)
-				p_end_fsb = roundup_64(p_end_fsb, align);
+		align = xfs_eof_alignment(ip, 0);
+		if (align)
+			p_end_fsb = roundup_64(p_end_fsb, align);
 
-			p_end_fsb = min(p_end_fsb, maxbytes_fsb);
-			ASSERT(p_end_fsb > offset_fsb);
-			prealloc_blocks = p_end_fsb - end_fsb;
-		}
+		p_end_fsb = min(p_end_fsb, maxbytes_fsb);
+		ASSERT(p_end_fsb > offset_fsb);
+		prealloc_blocks = p_end_fsb - end_fsb;
 	}
 
 retry:
-- 
2.7.4

Re: [PATCH v2 4/5] xfs: free cowblocks and retry on buffered write ENOSPC

[Christoph Hellwig]

On Wed, Jan 11, 2017 at 12:54:08PM -0500, Brian Foster wrote:
> XFS runs an eofblocks reclaim scan before returning an ENOSPC error to
> userspace for buffered writes. This facilitates aggressive speculative
> preallocation without causing user visible side effects such as
> premature ENOSPC.
> 
> Run a cowblocks scan in the same situation to reclaim lingering COW fork
> preallocation throughout the filesystem.

This should probably go into 4.10-rc..

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2 3/5] xfs: reuse iomap delalloc code for COW fork reservation

[Christoph Hellwig]

On Wed, Jan 11, 2017 at 12:54:07PM -0500, Brian Foster wrote:
> COW fork reservation (delayed allocation) is implemented in
> xfs_reflink_reserve_cow() and is generally based on the traditional data
> fork delalloc logic in xfs_file_iomap_begin_delay(). In preparation for
> further changes to implement more aggressive COW fork preallocation,
> refactor the COW reservation code to reuse xfs_file_iomap_begin_delay()
> for data fork allocation as well as COW fork reservation. This patch
> does not change behavior.

I'm still trying to understand the point of patches 1-3:  there is no
functionality, but a lot of new code is added to reuse some other code.

How would patch 5 look like without that "reuse"?

Re: [PATCH v2 0/5] xfs: basic cow fork speculative preallocation

[Christoph Hellwig]

On Wed, Jan 11, 2017 at 12:54:04PM -0500, Brian Foster wrote:
> Hi all,
> 
> Here's v2 of the cow fork speculative prealloc patches. No major changes
> here, as outlined below. Thoughts on any of this?

Any results?  Better performance, less extents for certain workloads?

Re: [PATCH v2 3/5] xfs: reuse iomap delalloc code for COW fork reservation

[Brian Foster]

On Fri, Jan 13, 2017 at 09:13:12AM -0800, Christoph Hellwig wrote:
> On Wed, Jan 11, 2017 at 12:54:07PM -0500, Brian Foster wrote:
> > COW fork reservation (delayed allocation) is implemented in
> > xfs_reflink_reserve_cow() and is generally based on the traditional data
> > fork delalloc logic in xfs_file_iomap_begin_delay(). In preparation for
> > further changes to implement more aggressive COW fork preallocation,
> > refactor the COW reservation code to reuse xfs_file_iomap_begin_delay()
> > for data fork allocation as well as COW fork reservation. This patch
> > does not change behavior.
> 
> I'm still trying to understand the point of patches 1-3:  there is no
> functionality, but a lot of new code is added to reuse some other code.
> 

IMO, patches 1-3 stand on their own as cleanup/refactor patches,
regardless of whether we want the actual speculative preallocation patch
(in current form or at all). xfs_reflink_reserve_cow() is mostly a
copy&paste of _iomap_begin_delay() operating on the cow fork rather than
the data fork, so technically we really shouldn't have a need for a
feature specific helper. Duplication aside, I also find the code a bit
confusing to follow in that we have to traverse through several
functions in "do nothing" cases such as non-shared blocks of a reflinked
file.

I would have preferred that code get factored into _begin_delay() from
the start, but it was too late for that by the time I grokked it and
there are also other callers from which prealloc may or may not be
appropriate (and that is why this patch by itself is not sufficient to
kill off _reserve_cow()). That said, we might be able to refactor the
allocation part of both functions such that we can replace the feature
specific helper with a common generic one. IOWs, these patches make it
so I don't have to further duplicate the prealloc stuff between
_write_begin() and _reserve_cow() and in the long term, I think this
helps facilitate further consolidation of duplicate code.

> How would patch 5 look like without that "reuse"?

I suppose we'd copy&paste more of begin_delay() into the reflink
specific helper (e.g., the prealloc bits). Then perhaps add a param to
control whether to perform preallocation.

Brian

> --
> 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

Re: [PATCH v2 0/5] xfs: basic cow fork speculative preallocation

[Brian Foster]

On Fri, Jan 13, 2017 at 09:13:42AM -0800, Christoph Hellwig wrote:
> On Wed, Jan 11, 2017 at 12:54:04PM -0500, Brian Foster wrote:
> > Hi all,
> > 
> > Here's v2 of the cow fork speculative prealloc patches. No major changes
> > here, as outlined below. Thoughts on any of this?
> 
> Any results?  Better performance, less extents for certain workloads?

Sorry, I neglected to pull that forward from the rfc. That discussion is
here:

http://www.spinics.net/lists/linux-xfs/msg02152.html

I should probably repeat some of this testing at this point, but the
general result is a reduction in extent count for parallel cow fork
allocator workloads. E.g., attempting to address the same problem of
parallel data fork allocators competing for contiguous space, resolved
by traditional speculative preallocation.

I was also kind of thinking about whether this could work around the
need to have a default cowextsz hint at all, allowing that feature to be
used more like the traditional extent size hint as well. This is
distinctly more aggressive for use cases like large vm image files,
however. At the same time, the idea is to make it transparent enough
that it shouldn't really affect the user (i.e., via throttling,
background and on demand reclaim, also just like normal preallocation).

Brian

> --
> 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

Re: [PATCH v2 3/5] xfs: reuse iomap delalloc code for COW fork reservation

[Christoph Hellwig]

On Mon, Jan 16, 2017 at 11:26:01AM -0500, Brian Foster wrote:
> IMO, patches 1-3 stand on their own as cleanup/refactor patches,
> regardless of whether we want the actual speculative preallocation patch
> (in current form or at all). xfs_reflink_reserve_cow() is mostly a
> copy&paste of _iomap_begin_delay() operating on the cow fork rather than
> the data fork, so technically we really shouldn't have a need for a
> feature specific helper. Duplication aside, I also find the code a bit
> confusing to follow in that we have to traverse through several
> functions in "do nothing" cases such as non-shared blocks of a reflinked
> file.

I'm usually not a fan of refactor patches that adds lots of new code
without adding functionality.  In terms of readability I'm obviously
biasses having written a lot of the code, but I find the new code
much harder to read.

Re: [PATCH v2 3/5] xfs: reuse iomap delalloc code for COW fork reservation

[Brian Foster]

On Mon, Jan 16, 2017 at 09:37:30AM -0800, Christoph Hellwig wrote:
> On Mon, Jan 16, 2017 at 11:26:01AM -0500, Brian Foster wrote:
> > IMO, patches 1-3 stand on their own as cleanup/refactor patches,
> > regardless of whether we want the actual speculative preallocation patch
> > (in current form or at all). xfs_reflink_reserve_cow() is mostly a
> > copy&paste of _iomap_begin_delay() operating on the cow fork rather than
> > the data fork, so technically we really shouldn't have a need for a
> > feature specific helper. Duplication aside, I also find the code a bit
> > confusing to follow in that we have to traverse through several
> > functions in "do nothing" cases such as non-shared blocks of a reflinked
> > file.
> 
> I'm usually not a fan of refactor patches that adds lots of new code
> without adding functionality.  In terms of readability I'm obviously
> biasses having written a lot of the code, but I find the new code
> much harder to read.

Ok, we disagree on whether the refactoring stands on its own. I'm not
quite sure which part you dislike (note that the iomap_search_extents()
part was based on your suggestion from the rfc). Either way, this series
does add functionality too. :)

I'm not that concerned with getting the refactor in by itself if there
is disagreement. I just didn't want to continue to have two delalloc
write paths that differ only by the target fork (and with this series,
initial prealloc size). So, any thoughts on the mechanism itself, or how
you'd envision it look if not as implemented here?

Brian

> --
> 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