diff mbox series

[v3,11/14] xfs: Only free full extents for forcealign

Message ID 20240801163057.3981192-12-john.g.garry@oracle.com (mailing list archive)
State Superseded, archived
Headers show
Series forcealign for xfs | expand

Commit Message

John Garry Aug. 1, 2024, 4:30 p.m. UTC 
Like we already do for rtvol, only free full extents for forcealign in
xfs_free_file_space().

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> #earlier version
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 fs/xfs/xfs_bmap_util.c |  7 ++-----
 fs/xfs/xfs_inode.c     | 14 ++++++++++++++
 fs/xfs/xfs_inode.h     |  2 ++
 3 files changed, 18 insertions(+), 5 deletions(-)

Comments

Darrick J. Wong Aug. 6, 2024, 7:27 p.m. UTC | #1 
On Thu, Aug 01, 2024 at 04:30:54PM +0000, John Garry wrote:
> Like we already do for rtvol, only free full extents for forcealign in
> xfs_free_file_space().
> 
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> #earlier version
> Signed-off-by: John Garry <john.g.garry@oracle.com>
> ---
>  fs/xfs/xfs_bmap_util.c |  7 ++-----
>  fs/xfs/xfs_inode.c     | 14 ++++++++++++++
>  fs/xfs/xfs_inode.h     |  2 ++
>  3 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 60389ac8bd45..46eebecd7bba 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -854,11 +854,8 @@ xfs_free_file_space(
>  	startoffset_fsb = XFS_B_TO_FSB(mp, offset);
>  	endoffset_fsb = XFS_B_TO_FSBT(mp, offset + len);
>  
> -	/* We can only free complete realtime extents. */
> -	if (xfs_inode_has_bigrtalloc(ip)) {
> -		startoffset_fsb = xfs_rtb_roundup_rtx(mp, startoffset_fsb);
> -		endoffset_fsb = xfs_rtb_rounddown_rtx(mp, endoffset_fsb);
> -	}
> +	/* Free only complete extents. */
> +	xfs_roundin_to_alloc_fsbsize(ip, &startoffset_fsb, &endoffset_fsb);

...and then this becomes:

	/* We can only free complete allocation units. */
	startoffset_fsb = xfs_inode_roundup_alloc_unit(ip, startoffset_fsb);
	endoffset_fsb = xfs_inode_rounddown_alloc_unit(ip, endoffset_fsb);

I guess "roundout" means "extend start and end to fit allocation unit"
whereas "roundin" means "shrink start and end to fit allocation unit"?

--D

>  
>  	/*
>  	 * Need to zero the stuff we're not freeing, on disk.
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index d765dedebc15..e7fa155fcbde 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -3143,6 +3143,20 @@ xfs_roundout_to_alloc_fsbsize(
>  	*end = roundup_64(*end, blocks);
>  }
>  
> +void
> +xfs_roundin_to_alloc_fsbsize(
> +	struct xfs_inode	*ip,
> +	xfs_fileoff_t		*start,
> +	xfs_fileoff_t		*end)
> +{
> +	unsigned int		blocks = xfs_inode_alloc_fsbsize(ip);
> +
> +	if (blocks == 1)
> +		return;
> +	*start = roundup_64(*start, blocks);
> +	*end = rounddown_64(*end, blocks);
> +}
> +
>  /* Should we always be using copy on write for file writes? */
>  bool
>  xfs_is_always_cow_inode(
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 7f86c4781bd8..6dd8055c98b3 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -645,6 +645,8 @@ unsigned int xfs_inode_alloc_fsbsize(struct xfs_inode *ip);
>  unsigned int xfs_inode_alloc_unitsize(struct xfs_inode *ip);
>  void xfs_roundout_to_alloc_fsbsize(struct xfs_inode *ip,
>  		xfs_fileoff_t *start, xfs_fileoff_t *end);
> +void xfs_roundin_to_alloc_fsbsize(struct xfs_inode *ip,
> +		xfs_fileoff_t *start, xfs_fileoff_t *end);
>  
>  int xfs_icreate_dqalloc(const struct xfs_icreate_args *args,
>  		struct xfs_dquot **udqpp, struct xfs_dquot **gdqpp,
> -- 
> 2.31.1
> 
>
Dave Chinner Aug. 7, 2024, 12:08 a.m. UTC | #2 
On Tue, Aug 06, 2024 at 12:27:38PM -0700, Darrick J. Wong wrote:
> On Thu, Aug 01, 2024 at 04:30:54PM +0000, John Garry wrote:
> > Like we already do for rtvol, only free full extents for forcealign in
> > xfs_free_file_space().
> > 
> > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> #earlier version
> > Signed-off-by: John Garry <john.g.garry@oracle.com>
> > ---
> >  fs/xfs/xfs_bmap_util.c |  7 ++-----
> >  fs/xfs/xfs_inode.c     | 14 ++++++++++++++
> >  fs/xfs/xfs_inode.h     |  2 ++
> >  3 files changed, 18 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> > index 60389ac8bd45..46eebecd7bba 100644
> > --- a/fs/xfs/xfs_bmap_util.c
> > +++ b/fs/xfs/xfs_bmap_util.c
> > @@ -854,11 +854,8 @@ xfs_free_file_space(
> >  	startoffset_fsb = XFS_B_TO_FSB(mp, offset);
> >  	endoffset_fsb = XFS_B_TO_FSBT(mp, offset + len);
> >  
> > -	/* We can only free complete realtime extents. */
> > -	if (xfs_inode_has_bigrtalloc(ip)) {
> > -		startoffset_fsb = xfs_rtb_roundup_rtx(mp, startoffset_fsb);
> > -		endoffset_fsb = xfs_rtb_rounddown_rtx(mp, endoffset_fsb);
> > -	}
> > +	/* Free only complete extents. */
> > +	xfs_roundin_to_alloc_fsbsize(ip, &startoffset_fsb, &endoffset_fsb);
> 
> ...and then this becomes:
> 
> 	/* We can only free complete allocation units. */
> 	startoffset_fsb = xfs_inode_roundup_alloc_unit(ip, startoffset_fsb);
> 	endoffset_fsb = xfs_inode_rounddown_alloc_unit(ip, endoffset_fsb);

I much prefer this (roundup/rounddown) to the "in/out" API.

-Dave.
John Garry Aug. 7, 2024, 1:06 p.m. UTC | #3 
On 06/08/2024 20:27, Darrick J. Wong wrote:
> I guess "roundout" means "extend start and end to fit allocation unit"
> whereas "roundin" means "shrink start and end to fit allocation unit"?

Yes, I was inspired by wording "inwards" and "outwards" used in a 
discussion in the previous series. Anyway, I can change it as suggested.
diff mbox series

Patch

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 60389ac8bd45..46eebecd7bba 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -854,11 +854,8 @@  xfs_free_file_space(
 	startoffset_fsb = XFS_B_TO_FSB(mp, offset);
 	endoffset_fsb = XFS_B_TO_FSBT(mp, offset + len);
 
-	/* We can only free complete realtime extents. */
-	if (xfs_inode_has_bigrtalloc(ip)) {
-		startoffset_fsb = xfs_rtb_roundup_rtx(mp, startoffset_fsb);
-		endoffset_fsb = xfs_rtb_rounddown_rtx(mp, endoffset_fsb);
-	}
+	/* Free only complete extents. */
+	xfs_roundin_to_alloc_fsbsize(ip, &startoffset_fsb, &endoffset_fsb);
 
 	/*
 	 * Need to zero the stuff we're not freeing, on disk.
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index d765dedebc15..e7fa155fcbde 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -3143,6 +3143,20 @@  xfs_roundout_to_alloc_fsbsize(
 	*end = roundup_64(*end, blocks);
 }
 
+void
+xfs_roundin_to_alloc_fsbsize(
+	struct xfs_inode	*ip,
+	xfs_fileoff_t		*start,
+	xfs_fileoff_t		*end)
+{
+	unsigned int		blocks = xfs_inode_alloc_fsbsize(ip);
+
+	if (blocks == 1)
+		return;
+	*start = roundup_64(*start, blocks);
+	*end = rounddown_64(*end, blocks);
+}
+
 /* Should we always be using copy on write for file writes? */
 bool
 xfs_is_always_cow_inode(
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 7f86c4781bd8..6dd8055c98b3 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -645,6 +645,8 @@  unsigned int xfs_inode_alloc_fsbsize(struct xfs_inode *ip);
 unsigned int xfs_inode_alloc_unitsize(struct xfs_inode *ip);
 void xfs_roundout_to_alloc_fsbsize(struct xfs_inode *ip,
 		xfs_fileoff_t *start, xfs_fileoff_t *end);
+void xfs_roundin_to_alloc_fsbsize(struct xfs_inode *ip,
+		xfs_fileoff_t *start, xfs_fileoff_t *end);
 
 int xfs_icreate_dqalloc(const struct xfs_icreate_args *args,
 		struct xfs_dquot **udqpp, struct xfs_dquot **gdqpp,