On Mon, Mar 04, 2024 at 01:04:20PM +0000, John Garry wrote:
> For when forcealign is enabled, we want the EOF to be aligned as well, so
> do not free EOF blocks.
>
> Add helper function xfs_get_extsz() to get the guaranteed extent size
> alignment for forcealign enabled. Since this code is only relevant to
> forcealign and forcealign is not possible for RT yet, ignore RT.
>
> Signed-off-by: John Garry <john.g.garry@xxxxxxxxxx>
> ---
> fs/xfs/xfs_bmap_util.c | 7 ++++++-
> fs/xfs/xfs_inode.c | 14 ++++++++++++++
> fs/xfs/xfs_inode.h | 1 +
> 3 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index c2531c28905c..07bfb03c671a 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -542,8 +542,13 @@ 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)
> +
> + /* Do not free blocks when forcing extent sizes */
That comment seems wrong - this just rounds up where we start
trimming from to be aligned...
> + if (xfs_get_extsz(ip) > 1)
> + end_fsb = roundup_64(end_fsb, xfs_get_extsz(ip));
> + else if (XFS_IS_REALTIME_INODE(ip) && mp->m_sb.sb_rextsize > 1)
> end_fsb = xfs_rtb_roundup_rtx(mp, end_fsb);
I think this would be better written as:
/*
* Forced extent alignment requires us to round up where we
* start trimming from so that result is correctly aligned.
*/
if (xfs_inode_forcealign(ip)) {
if (ip->i_extsize > 1)
end_fsb = roundup_64(end_fsb, ip->i_extsize);
else if (XFS_IS_REALTIME_INODE(ip) && mp->m_sb.sb_rextsize > 1)
end_fsb = xfs_rtb_roundup_rtx(mp, end_fsb);
}
Because we only want end-fsb alignment when forced alignment is
required.
Which then makes me wonder: truncate needs this, too, doesn't it?
And the various fallocate() ops like hole punching and extent
shifting?
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 2c439df8c47f..bbb8886f1d32 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -65,6 +65,20 @@ xfs_get_extsz_hint(
> return 0;
> }
>
> +/*
> + * Helper function to extract extent size. It will return a power-of-2,
> + * as forcealign requires this.
> + */
> +xfs_extlen_t
> +xfs_get_extsz(
> + struct xfs_inode *ip)
> +{
> + if (xfs_inode_forcealign(ip) && ip->i_extsize)
> + return ip->i_extsize;
> +
> + return 1;
> +}
This can go away - if it is needed elsewhere, then I think it would
be better open coded because it better documents what the code is
doing...
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
