Re: [PATCH 1/2] xfs: introduce xfs_bmap_split_da_extent

[Dave Chinner]

On Thu, Dec 26, 2019 at 09:47:20PM +0800, yu kuai wrote:
> Add a new function xfs_bmap_split_da_extent to split a delalloc extent
> into two delalloc extents.
> 
> Signed-off-by: yu kuai <yukuai3@xxxxxxxxxx>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 26 ++++++++++++++++++++++++--
>  fs/xfs/libxfs/xfs_bmap.h |  1 +
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 4c2e046fbfad..8247054c1e2b 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -6117,7 +6117,7 @@ xfs_bmap_split_extent_at(
>  	/*
>  	 * Convert to a btree if necessary.
>  	 */
> -	if (xfs_bmap_needs_btree(ip, whichfork)) {
> +	if (tp && xfs_bmap_needs_btree(ip, whichfork)) {
>  		int tmp_logflags; /* partial log flag return val */
>  
>  		ASSERT(cur == NULL);

You can't use xfs_bmap_split_extent_at() to split delalloc extents
just by avoiding transactional context.

delalloc extents only exist in the in-core extent list, not in the
on-disk BMBT extent list. xfs_bmap_split_extent_at() is for splitting
on-disk BMBT extents and, as such, it modifies the BMBT directly. It
may not be obvious that the bmbt cursor holds a pointer to the
transaction context, but it does and that's how we log all the
modifications to the BMBT done via the generic btree code.

IOWs, if the inode extent list is in btree format, this code will
not work correctly.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx