On Fri, Oct 25, 2019 at 11:36:09AM +0900, Christoph Hellwig wrote:
> Remove xfs_zero_file_space and reorganize xfs_file_fallocate so that a
> single call to xfs_alloc_file_space covers all modes that preallocate
> blocks.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Seems pretty straightforward.
On a side note I think Dave and Eric might have noticed that fallocate
doesn't drain directio which could lead to incorrect file size.
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_bmap_util.c | 37 -------------------------------------
> fs/xfs/xfs_bmap_util.h | 2 --
> fs/xfs/xfs_file.c | 32 ++++++++++++++++++++++++--------
> 3 files changed, 24 insertions(+), 47 deletions(-)
>
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 9b0572a7b03a..11658da40640 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -1133,43 +1133,6 @@ xfs_free_file_space(
> return error;
> }
>
> -/*
> - * Preallocate and zero a range of a file. This mechanism has the allocation
> - * semantics of fallocate and in addition converts data in the range to zeroes.
> - */
> -int
> -xfs_zero_file_space(
> - struct xfs_inode *ip,
> - xfs_off_t offset,
> - xfs_off_t len)
> -{
> - struct xfs_mount *mp = ip->i_mount;
> - uint blksize;
> - int error;
> -
> - trace_xfs_zero_file_space(ip);
> -
> - blksize = 1 << mp->m_sb.sb_blocklog;
> -
> - /*
> - * Punch a hole and prealloc the range. We use hole punch rather than
> - * unwritten extent conversion for two reasons:
> - *
> - * 1.) Hole punch handles partial block zeroing for us.
> - *
> - * 2.) If prealloc returns ENOSPC, the file range is still zero-valued
> - * by virtue of the hole punch.
> - */
> - error = xfs_free_file_space(ip, offset, len);
> - if (error || xfs_is_always_cow_inode(ip))
> - return error;
> -
> - return xfs_alloc_file_space(ip, round_down(offset, blksize),
> - round_up(offset + len, blksize) -
> - round_down(offset, blksize),
> - XFS_BMAPI_PREALLOC);
> -}
> -
> static int
> xfs_prepare_shift(
> struct xfs_inode *ip,
> diff --git a/fs/xfs/xfs_bmap_util.h b/fs/xfs/xfs_bmap_util.h
> index 7a78229cf1a7..3e0fa0d363d1 100644
> --- a/fs/xfs/xfs_bmap_util.h
> +++ b/fs/xfs/xfs_bmap_util.h
> @@ -59,8 +59,6 @@ int xfs_alloc_file_space(struct xfs_inode *ip, xfs_off_t offset,
> xfs_off_t len, int alloc_type);
> int xfs_free_file_space(struct xfs_inode *ip, xfs_off_t offset,
> xfs_off_t len);
> -int xfs_zero_file_space(struct xfs_inode *ip, xfs_off_t offset,
> - xfs_off_t len);
> int xfs_collapse_file_space(struct xfs_inode *, xfs_off_t offset,
> xfs_off_t len);
> int xfs_insert_file_space(struct xfs_inode *, xfs_off_t offset,
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 156238d5af19..525b29b99116 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -880,16 +880,30 @@ xfs_file_fallocate(
> }
>
> if (mode & FALLOC_FL_ZERO_RANGE) {
> - error = xfs_zero_file_space(ip, offset, len);
> + /*
> + * Punch a hole and prealloc the range. We use a hole
> + * punch rather than unwritten extent conversion for two
> + * reasons:
> + *
> + * 1.) Hole punch handles partial block zeroing for us.
> + * 2.) If prealloc returns ENOSPC, the file range is
> + * still zero-valued by virtue of the hole punch.
> + */
> + unsigned int blksize = i_blocksize(inode);
> +
> + trace_xfs_zero_file_space(ip);
> +
> + error = xfs_free_file_space(ip, offset, len);
> + if (error)
> + goto out_unlock;
> +
> + len = round_up(offset + len, blksize) -
> + round_down(offset, blksize);
> + offset = round_down(offset, blksize);
> } else if (mode & FALLOC_FL_UNSHARE_RANGE) {
> error = xfs_reflink_unshare(ip, offset, len);
> if (error)
> goto out_unlock;
> -
> - if (!xfs_is_always_cow_inode(ip)) {
> - error = xfs_alloc_file_space(ip, offset, len,
> - XFS_BMAPI_PREALLOC);
> - }
> } else {
> /*
> * If always_cow mode we can't use preallocations and
> @@ -899,12 +913,14 @@ xfs_file_fallocate(
> error = -EOPNOTSUPP;
> goto out_unlock;
> }
> + }
>
> + if (!xfs_is_always_cow_inode(ip)) {
> error = xfs_alloc_file_space(ip, offset, len,
> XFS_BMAPI_PREALLOC);
> + if (error)
> + goto out_unlock;
> }
> - if (error)
> - goto out_unlock;
> }
>
> if (file->f_flags & O_DSYNC)
> --
> 2.20.1
>
