On 30/04/2024 23:54, Dave Chinner wrote:
On Mon, Apr 29, 2024 at 05:47:34PM +0000, John Garry wrote:
For when forcealign is enabled, we want the EOF to be aligned as well, so
do not free EOF blocks.
This is doesn't match what the code does. The code is correct - it
rounds the range to be trimmed up to the aligned offset beyond EOF
and then frees them. The description needs to be updated to reflect
this.
ok, fine
Signed-off-by: John Garry <john.g.garry@xxxxxxxxxx>
---
fs/xfs/xfs_bmap_util.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 19e11d1da660..f26d1570b9bd 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 */
+ if (xfs_inode_has_forcealign(ip) && ip->i_extsize > 1)
I see this sort of check all through the remaining patches.
Given there are significant restrictions on forced alignment,
shouldn't this all the details be pushed inside the helper function?
e.g.
/*
* Forced extent alignment is dependent on extent size hints being
* set to define the alignment. Alignment is only necessary when the
* extent size hint is larger than a single block.
*
* If reflink is enabled on the file or we are in always_cow mode,
* we can't easily do forced alignment.
*
* We don't support forced alignment on realtime files.
* XXX(dgc): why not?
There is no technical reason to not be able to support forcealign on RT,
AFAIK. My idea is to support RT after non-RT is supported.
*/
static inline bool
xfs_inode_has_forcealign(struct xfs_inode *ip)
{
if (!(ip->di_flags & XFS_DIFLAG_EXTSIZE))
return false;
if (ip->i_extsize <= 1)
return false;
if (xfs_is_cow_inode(ip))
return false;
Could we just include this in the forcealign validate checks? Currently
we just check CoW extsize is zero there.
if (ip->di_flags & XFS_DIFLAG_REALTIME)
return false;
We check this in xfs_inode_validate_forcealign()
return ip->di_flags2 & XFS_DIFLAG2_FORCEALIGN;
}
So can we simply have:
static inline bool
xfs_inode_has_forcealign(struct xfs_inode *ip)
{
if (!(ip->di_flags & XFS_DIFLAG_EXTSIZE))
return false;
if (ip->i_extsize <= 1)
return false;
return ip->di_flags2 & XFS_DIFLAG2_FORCEALIGN;
}
Thanks,
John
