Re: [PATCH 1/3] xfs: open code insert range extent split helper

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

On Fri, Dec 13, 2019 at 12:12:56PM -0500, Brian Foster wrote:
> The insert range operation currently splits the extent at the target
> offset in a separate transaction and lock cycle from the one that
> shifts extents. In preparation for reworking insert range into an
> atomic operation, lift the code into the caller so it can be easily
> condensed to a single rolling transaction and lock cycle and
> eliminate the helper. No functional changes.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/libxfs/xfs_bmap.c | 32 ++------------------------------
>  fs/xfs/libxfs/xfs_bmap.h |  3 ++-
>  fs/xfs/xfs_bmap_util.c   | 14 +++++++++++++-
>  3 files changed, 17 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index a9ad1f991ba3..2bba0f983e4f 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -6021,8 +6021,8 @@ xfs_bmap_insert_extents(
>   * @split_fsb is a block where the extents is split.  If split_fsb lies in a
>   * hole or the first block of extents, just return 0.
>   */
> -STATIC int
> -xfs_bmap_split_extent_at(
> +int
> +xfs_bmap_split_extent(
>  	struct xfs_trans	*tp,
>  	struct xfs_inode	*ip,
>  	xfs_fileoff_t		split_fsb)
> @@ -6138,34 +6138,6 @@ xfs_bmap_split_extent_at(
>  	return error;
>  }
>  
> -int
> -xfs_bmap_split_extent(
> -	struct xfs_inode        *ip,
> -	xfs_fileoff_t           split_fsb)
> -{
> -	struct xfs_mount        *mp = ip->i_mount;
> -	struct xfs_trans        *tp;
> -	int                     error;
> -
> -	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write,
> -			XFS_DIOSTRAT_SPACE_RES(mp, 0), 0, 0, &tp);
> -	if (error)
> -		return error;
> -
> -	xfs_ilock(ip, XFS_ILOCK_EXCL);
> -	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> -
> -	error = xfs_bmap_split_extent_at(tp, ip, split_fsb);
> -	if (error)
> -		goto out;
> -
> -	return xfs_trans_commit(tp);
> -
> -out:
> -	xfs_trans_cancel(tp);
> -	return error;
> -}
> -
>  /* Deferred mapping is only for real extents in the data fork. */
>  static bool
>  xfs_bmap_is_update_needed(
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index 14d25e0b7d9c..f3259ad5c22c 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -222,7 +222,8 @@ int	xfs_bmap_can_insert_extents(struct xfs_inode *ip, xfs_fileoff_t off,
>  int	xfs_bmap_insert_extents(struct xfs_trans *tp, struct xfs_inode *ip,
>  		xfs_fileoff_t *next_fsb, xfs_fileoff_t offset_shift_fsb,
>  		bool *done, xfs_fileoff_t stop_fsb);
> -int	xfs_bmap_split_extent(struct xfs_inode *ip, xfs_fileoff_t split_offset);
> +int	xfs_bmap_split_extent(struct xfs_trans *tp, struct xfs_inode *ip,
> +		xfs_fileoff_t split_offset);
>  int	xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
>  		xfs_fileoff_t off, xfs_filblks_t len, xfs_filblks_t prealloc,
>  		struct xfs_bmbt_irec *got, struct xfs_iext_cursor *cur,
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 2efd78a9719e..829ab1a804c9 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -1139,7 +1139,19 @@ xfs_insert_file_space(
>  	 * is not the starting block of extent, we need to split the extent at
>  	 * stop_fsb.
>  	 */
> -	error = xfs_bmap_split_extent(ip, stop_fsb);
> +	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write,
> +			XFS_DIOSTRAT_SPACE_RES(mp, 0), 0, 0, &tp);
> +	if (error)
> +		return error;
> +
> +	xfs_ilock(ip, XFS_ILOCK_EXCL);
> +	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> +
> +	error = xfs_bmap_split_extent(tp, ip, stop_fsb);
> +	if (error)
> +		goto out_trans_cancel;
> +
> +	error = xfs_trans_commit(tp);
>  	if (error)
>  		return error;
>  
> -- 
> 2.20.1
>