diff mbox series

[5/7] xfs: hoist multi-fsb allocation unit detection to a helper

Message ID 171150380216.3216450.3675851752965499332.stgit@frogsfrogsfrogs (mailing list archive)
State Superseded
Headers show
Series [1/7] xfs: move inode lease breaking functions to xfs_inode.c | expand

Commit Message

Darrick J. Wong March 27, 2024, 1:52 a.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

Replace the open-coded logic to decide if a file has a multi-fsb
allocation unit to a helper to make the code easier to read.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/xfs_bmap_util.c |    4 ++--
 fs/xfs/xfs_inode.h     |    9 +++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

Comments

Christoph Hellwig March 27, 2024, 11:05 a.m. UTC | #1 
Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Dave Chinner April 7, 2024, 11:07 p.m. UTC | #2 
On Tue, Mar 26, 2024 at 06:52:18PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Replace the open-coded logic to decide if a file has a multi-fsb
> allocation unit to a helper to make the code easier to read.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  fs/xfs/xfs_bmap_util.c |    4 ++--
>  fs/xfs/xfs_inode.h     |    9 +++++++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 19e11d1da6607..c17b5858fed62 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -542,7 +542,7 @@ xfs_can_free_eofblocks(
>  	 * forever.
>  	 */
>  	end_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)XFS_ISIZE(ip));
> -	if (XFS_IS_REALTIME_INODE(ip) && mp->m_sb.sb_rextsize > 1)
> +	if (xfs_inode_has_bigallocunit(ip))
>  		end_fsb = xfs_rtb_roundup_rtx(mp, end_fsb);

This makes no sense with the upcoming forced alignment changes to
this code.

That essentially brings "big alloc unit" to the data device based on
extent size hints, and it will need to do different rounding
calculations depending on whether it is a RT inode or not. Hence I
don't think hiding the RT specific allocation/truncation setup like
this is compatible with those changes - it will simply have to be
undone before it can be reworked....

-Dave.
Darrick J. Wong April 9, 2024, 9:09 p.m. UTC | #3 
On Mon, Apr 08, 2024 at 09:07:29AM +1000, Dave Chinner wrote:
> On Tue, Mar 26, 2024 at 06:52:18PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Replace the open-coded logic to decide if a file has a multi-fsb
> > allocation unit to a helper to make the code easier to read.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  fs/xfs/xfs_bmap_util.c |    4 ++--
> >  fs/xfs/xfs_inode.h     |    9 +++++++++
> >  2 files changed, 11 insertions(+), 2 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> > index 19e11d1da6607..c17b5858fed62 100644
> > --- a/fs/xfs/xfs_bmap_util.c
> > +++ b/fs/xfs/xfs_bmap_util.c
> > @@ -542,7 +542,7 @@ xfs_can_free_eofblocks(
> >  	 * forever.
> >  	 */
> >  	end_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)XFS_ISIZE(ip));
> > -	if (XFS_IS_REALTIME_INODE(ip) && mp->m_sb.sb_rextsize > 1)
> > +	if (xfs_inode_has_bigallocunit(ip))
> >  		end_fsb = xfs_rtb_roundup_rtx(mp, end_fsb);
> 
> This makes no sense with the upcoming forced alignment changes to
> this code.
> 
> That essentially brings "big alloc unit" to the data device based on
> extent size hints, and it will need to do different rounding
> calculations depending on whether it is a RT inode or not. Hence I
> don't think hiding the RT specific allocation/truncation setup like
> this is compatible with those changes - it will simply have to be
> undone before it can be reworked....

So undo it when you and John and Catherine have a full patchset
implementing forced alignment.

--D

> -Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
>
diff mbox series

Patch

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 19e11d1da6607..c17b5858fed62 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -542,7 +542,7 @@  xfs_can_free_eofblocks(
 	 * forever.
 	 */
 	end_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)XFS_ISIZE(ip));
-	if (XFS_IS_REALTIME_INODE(ip) && mp->m_sb.sb_rextsize > 1)
+	if (xfs_inode_has_bigallocunit(ip))
 		end_fsb = xfs_rtb_roundup_rtx(mp, end_fsb);
 	last_fsb = XFS_B_TO_FSB(mp, mp->m_super->s_maxbytes);
 	if (last_fsb <= end_fsb)
@@ -843,7 +843,7 @@  xfs_free_file_space(
 	endoffset_fsb = XFS_B_TO_FSBT(mp, offset + len);
 
 	/* We can only free complete realtime extents. */
-	if (XFS_IS_REALTIME_INODE(ip) && mp->m_sb.sb_rextsize > 1) {
+	if (xfs_inode_has_bigallocunit(ip)) {
 		startoffset_fsb = xfs_rtb_roundup_rtx(mp, startoffset_fsb);
 		endoffset_fsb = xfs_rtb_rounddown_rtx(mp, endoffset_fsb);
 	}
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index fa3e605901e24..89ba114fdb468 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -311,6 +311,15 @@  static inline bool xfs_inode_has_large_extent_counts(struct xfs_inode *ip)
 	return ip->i_diflags2 & XFS_DIFLAG2_NREXT64;
 }
 
+/*
+ * Decide if the file data allocation unit for this file is larger than
+ * a single filesystem block.
+ */
+static inline bool xfs_inode_has_bigallocunit(struct xfs_inode *ip)
+{
+	return XFS_IS_REALTIME_INODE(ip) && ip->i_mount->m_sb.sb_rextsize > 1;
+}
+
 /*
  * Return the buftarg used for data allocations on a given inode.
  */