diff mbox series

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

Message ID 20200820054349.5525-10-chandanrlinux@gmail.com (mailing list archive)
State Superseded
Headers show
Series Bail out if transaction can cause extent count to overflow | expand

Commit Message

Chandan Babu R Aug. 20, 2020, 5:43 a.m. UTC
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(+)

Comments

Darrick J. Wong Aug. 31, 2020, 4:23 p.m. UTC | #1
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
>
Chandan Babu R Sept. 1, 2020, 9:45 a.m. UTC | #2
On Monday 31 August 2020 9:53:35 PM IST Darrick J. Wong wrote:
> 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.
>

Ok. I will fix that.

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

Patch

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)
+/*
+ * 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