Re: [PATCH V3 09/10] xfs: Check for extent overflow when remapping an extent

Darrick J. Wong
2020-08-31T16:25:39Z

On Thu, Aug 20, 2020 at 11:13:48AM +0530, Chandan Babu R wrote:
> Remapping an extent involves unmapping the existing extent and mapping
> in the new extent. When unmapping, an extent containing the entire unmap
> range can be split into two extents,
> i.e. | Old extent | hole | Old extent |
> Hence extent count increases by 1.
>
> Mapping in the new extent into the destination file can increase the
> extent count by 1.
>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
> ---
> fs/xfs/libxfs/xfs_inode_fork.h | 14 ++++++++++++++
> fs/xfs/xfs_reflink.c | 5 +++++
> 2 files changed, 19 insertions(+)
>
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
> index 850d53162545..d1c675cf803a 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.h
> +++ b/fs/xfs/libxfs/xfs_inode_fork.h
> @@ -86,6 +86,20 @@ struct xfs_ifork {
> * Hence number of extents increases by 2.
> */
> #define XFS_IEXT_REFLINK_END_COW_CNT (2)
> +/*

It's usually considered good style to put a blank line between the
previous definition and the new comment.

With that fixed,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> + * Remapping an extent involves unmapping the existing extent and mapping in the
> + * new extent.
> + *
> + * When unmapping, an extent containing the entire unmap range can be split into
> + * two extents,
> + * i.e. | Old extent | hole | Old extent |
> + * Hence extent count increases by 1.
> + *
> + * Mapping in the new extent into the destination file can increase the extent
> + * count by 1.
> + */
> +#define XFS_IEXT_REFLINK_REMAP_CNT(smap_real, dmap_written) \
> + (((smap_real) ? 1 : 0) + ((dmap_written) ? 1 : 0))
>
> /*
> * Fork handling.
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index c1d2a741e1af..9884fd51efee 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -1099,6 +1099,11 @@ xfs_reflink_remap_extent(
> goto out_cancel;
> }
>
> + error = xfs_iext_count_may_overflow(ip, XFS_DATA_FORK,
> + XFS_IEXT_REFLINK_REMAP_CNT(smap_real, dmap_written));
> + if (error)
> + goto out_cancel;
> +
> if (smap_real) {
> /*
> * If the extent we're unmapping is backed by storage (written
> --
> 2.28.0
>