Re: [PATCH 03/17] xfs: don't set XFS_BTCUR_BPRV_WASDEL in xfs_bunmapi

Brian Foster
2017-09-07T15:46:51Z

On Sun, Sep 03, 2017 at 09:29:42AM +0200, Christoph Hellwig wrote:
> The XFS_BTCUR_BPRV_WASDEL flag is supposed to indicate that we are
> converting a delayed allocation to a real one, which isn't the case
> in xfs_bunmapi. Setting it could theoretically lead to misaccounting
> here, but it's unlikely that we ever hit it in practice.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 4867df0c1bcd..b5b30415aa2b 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5725,11 +5725,7 @@ __xfs_bunmapi(
> XFS_QMOPT_RES_REGBLKS);
> }
> ip->i_delayed_blks -= del.br_blockcount;
> - if (cur)
> - cur->bc_private.b.flags |=
> - XFS_BTCUR_BPRV_WASDEL;
> - } else if (cur)
> - cur->bc_private.b.flags &= ~XFS_BTCUR_BPRV_WASDEL;
> + }

Ok, this flag is checked in xfs_bmbt_alloc_block() which is not a
codepath we should hit when removing a delalloc extent. Indeed, the
del_extent() call below appropriately skips any cur actions based on the
delalloc state. This seems fine to me:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>
> error = xfs_bmap_del_extent(ip, tp, &lastx, dfops, cur, &del,
> &tmp_logflags, whichfork, flags);
> --
> 2.11.0
>
> --
> 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