diff mbox series

[V3,07/10] xfs: Check for extent overflow when inserting a hole

Message ID 20200820054349.5525-8-chandanrlinux@gmail.com (mailing list archive)
State New, archived
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
The extent mapping the file offset at which a hole has to be
inserted will be split into two extents causing extent count to
increase by 1.

Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 fs/xfs/libxfs/xfs_inode_fork.h | 6 ++++++
 fs/xfs/xfs_bmap_util.c         | 9 +++++++++
 2 files changed, 15 insertions(+)

Comments

Darrick J. Wong Aug. 31, 2020, 4:46 p.m. UTC | #1
On Thu, Aug 20, 2020 at 11:13:46AM +0530, Chandan Babu R wrote:
> The extent mapping the file offset at which a hole has to be
> inserted will be split into two extents causing extent count to
> increase by 1.
> 
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_inode_fork.h | 6 ++++++
>  fs/xfs/xfs_bmap_util.c         | 9 +++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
> index 83ff90e2a5fe..d0e49b015b62 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.h
> +++ b/fs/xfs/libxfs/xfs_inode_fork.h
> @@ -73,6 +73,12 @@ struct xfs_ifork {
>   * Hence extent count can increase by 2.
>   */
>  #define XFS_IEXT_WRITE_UNWRITTEN_CNT	(2)
> +/*
> + * The extent mapping the file offset at which a hole has to be inserted will be
> + * split into two extents causing extent count to increase by 1.
> + */
> +#define XFS_IEXT_INSERT_HOLE_CNT	(1)

Given my earlier comments about how patch 3 is really only testing the
hole punching case, maybe it should be folded into this one?  They're
both inserting holes in the extent map.

Otherwise this patch looks good to me.

--D

> +
>  
>  /*
>   * Fork handling.
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 59d4da38aadf..e682eecebb1f 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -1165,6 +1165,15 @@ xfs_insert_file_space(
>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
>  	xfs_trans_ijoin(tp, ip, 0);
>  
> +	/*
> +	 * Splitting the extent mapping containing stop_fsb will cause
> +	 * extent count to increase by 1.
> +	 */
> +	error = xfs_iext_count_may_overflow(ip, XFS_DATA_FORK,
> +			XFS_IEXT_INSERT_HOLE_CNT);
> +	if (error)
> +		goto out_trans_cancel;
> +
>  	/*
>  	 * The extent shifting code works on extent granularity. So, if stop_fsb
>  	 * is not the starting block of extent, we need to split the extent at
> -- 
> 2.28.0
>
Chandan Babu R Sept. 1, 2020, 9:44 a.m. UTC | #2
On Monday 31 August 2020 10:16:35 PM IST Darrick J. Wong wrote:
> On Thu, Aug 20, 2020 at 11:13:46AM +0530, Chandan Babu R wrote:
> > The extent mapping the file offset at which a hole has to be
> > inserted will be split into two extents causing extent count to
> > increase by 1.
> > 
> > Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
> > ---
> >  fs/xfs/libxfs/xfs_inode_fork.h | 6 ++++++
> >  fs/xfs/xfs_bmap_util.c         | 9 +++++++++
> >  2 files changed, 15 insertions(+)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
> > index 83ff90e2a5fe..d0e49b015b62 100644
> > --- a/fs/xfs/libxfs/xfs_inode_fork.h
> > +++ b/fs/xfs/libxfs/xfs_inode_fork.h
> > @@ -73,6 +73,12 @@ struct xfs_ifork {
> >   * Hence extent count can increase by 2.
> >   */
> >  #define XFS_IEXT_WRITE_UNWRITTEN_CNT	(2)
> > +/*
> > + * The extent mapping the file offset at which a hole has to be inserted will be
> > + * split into two extents causing extent count to increase by 1.
> > + */
> > +#define XFS_IEXT_INSERT_HOLE_CNT	(1)
> 
> Given my earlier comments about how patch 3 is really only testing the
> hole punching case, maybe it should be folded into this one?  They're
> both inserting holes in the extent map.
>
I agree. I will make the required changes.
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index 83ff90e2a5fe..d0e49b015b62 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -73,6 +73,12 @@  struct xfs_ifork {
  * Hence extent count can increase by 2.
  */
 #define XFS_IEXT_WRITE_UNWRITTEN_CNT	(2)
+/*
+ * The extent mapping the file offset at which a hole has to be inserted will be
+ * split into two extents causing extent count to increase by 1.
+ */
+#define XFS_IEXT_INSERT_HOLE_CNT	(1)
+
 
 /*
  * Fork handling.
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 59d4da38aadf..e682eecebb1f 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1165,6 +1165,15 @@  xfs_insert_file_space(
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, ip, 0);
 
+	/*
+	 * Splitting the extent mapping containing stop_fsb will cause
+	 * extent count to increase by 1.
+	 */
+	error = xfs_iext_count_may_overflow(ip, XFS_DATA_FORK,
+			XFS_IEXT_INSERT_HOLE_CNT);
+	if (error)
+		goto out_trans_cancel;
+
 	/*
 	 * The extent shifting code works on extent granularity. So, if stop_fsb
 	 * is not the starting block of extent, we need to split the extent at