On Mon, May 07, 2018 at 02:11:36PM -0400, Brian Foster wrote:
> We've had reports of online discard operations being sent from XFS
> on write-only workloads. These discards occur as a result of
> eofblocks trims that can occur after a large file copy completes.
>
> These discards are slightly confusing for users who might be paying
> close attention to online discards (i.e., vdo) due to performance
> sensitivity. They also happen to be spurious because freed post-eof
> blocks by definition have not been written to during the current
> allocation cycle.
>
> Update xfs_free_eofblocks() to skip discards that are purely
> attributed to eofblocks trims. This cuts down the number of spurious
> discards that may occur on write-only workloads due to normal
> preallocation activity.
>
> Note that discards of post-eof extents can still occur from other
> codepaths that do not isolate handling of post-eof blocks from those
> within eof. For example, file unlinks and truncates may still cause
> discards for any file blocks affected by the operation.
>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
> fs/xfs/xfs_attr_inactive.c | 3 ++-
> fs/xfs/xfs_bmap_util.c | 2 +-
> fs/xfs/xfs_inode.c | 11 ++++++++---
> fs/xfs/xfs_inode.h | 2 +-
> fs/xfs/xfs_iops.c | 3 ++-
> fs/xfs/xfs_qm_syscalls.c | 2 +-
> 6 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
> index 52818ea2eb50..639311e60c5d 100644
> --- a/fs/xfs/xfs_attr_inactive.c
> +++ b/fs/xfs/xfs_attr_inactive.c
> @@ -434,7 +434,8 @@ xfs_attr_inactive(
> if (error)
> goto out_cancel;
>
> - error = xfs_itruncate_extents(&trans, dp, XFS_ATTR_FORK, 0);
> + error = xfs_itruncate_extents(&trans, dp, XFS_ATTR_FORK, 0,
> + false);
> if (error)
> goto out_cancel;
> }
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 8cd8c412f52d..7b95d8976488 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -872,7 +872,7 @@ xfs_free_eofblocks(
> * may be full of holes (ie NULL files bug).
> */
> error = xfs_itruncate_extents(&tp, ip, XFS_DATA_FORK,
> - XFS_ISIZE(ip));
> + XFS_ISIZE(ip), true);
xfs_itruncate_extents_nodiscard(), and then all the other
xfs_itruncate_extents() can remain unchanged.
> if (error) {
> /*
> * If we get an error at this point we simply don't
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 2b70c8b4cee2..03b2b7099f90 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1538,7 +1538,8 @@ xfs_itruncate_extents(
> struct xfs_trans **tpp,
> struct xfs_inode *ip,
> int whichfork,
> - xfs_fsize_t new_size)
> + xfs_fsize_t new_size,
> + bool skip_discard)
> {
> struct xfs_mount *mp = ip->i_mount;
> struct xfs_trans *tp = *tpp;
> @@ -1549,6 +1550,7 @@ xfs_itruncate_extents(
> xfs_filblks_t unmap_len;
> int error = 0;
> int done = 0;
> + int flags = 0;
>
> ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> ASSERT(!atomic_read(&VFS_I(ip)->i_count) ||
> @@ -1561,6 +1563,9 @@ xfs_itruncate_extents(
>
> trace_xfs_itruncate_extents_start(ip, new_size);
>
> + if (skip_discard)
> + flags |= XFS_BMAPI_NODISCARD;
> +
flags = xfs_bmapi_aflag(whichfork);
if (skip_discard)
flags |= XFS_BMAPI_NODISCARD;
> /*
> * Since it is possible for space to become allocated beyond
> * the end of the file (in a crash where the space is allocated
> @@ -1581,7 +1586,7 @@ xfs_itruncate_extents(
> xfs_defer_init(&dfops, &first_block);
> error = xfs_bunmapi(tp, ip,
> first_unmap_block, unmap_len,
> - xfs_bmapi_aflag(whichfork),
> + xfs_bmapi_aflag(whichfork) | flags,
so this gets simpler and easier to read.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
