On 01/05/2024 01:53, Dave Chinner wrote:
On Mon, Apr 29, 2024 at 05:47:37PM +0000, John Garry wrote:
Like we already do for rtvol, only free full extents for forcealign in
xfs_free_file_space().
Signed-off-by: John Garry <john.g.garry@xxxxxxxxxx>
---
fs/xfs/xfs_bmap_util.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index f26d1570b9bd..1dd45dfb2811 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -847,8 +847,11 @@ 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_IS_REALTIME_INODE(ip) && mp->m_sb.sb_rextsize > 1) {
+ /* Free only complete extents. */
+ if (xfs_inode_has_forcealign(ip) && ip->i_extsize > 1) {
+ startoffset_fsb = roundup_64(startoffset_fsb, ip->i_extsize);
+ endoffset_fsb = rounddown_64(endoffset_fsb, ip->i_extsize);
+ } else if (XFS_IS_REALTIME_INODE(ip) && mp->m_sb.sb_rextsize > 1) {
startoffset_fsb = xfs_rtb_roundup_rtx(mp, startoffset_fsb);
endoffset_fsb = xfs_rtb_rounddown_rtx(mp, endoffset_fsb);
}
When you look at xfs_rtb_roundup_rtx() you'll find it's just a one
line wrapper around roundup_64().
So lets get rid of the obfuscation that the one line RT wrapper
introduces, and it turns into this:
rounding = 1;
if (xfs_inode_has_forcealign(ip)
rounding = ip->i_extsize;
else if (XFS_IS_REALTIME_INODE(ip))
rounding = mp->m_sb.sb_rextsize;
if (rounding > 1) {
startoffset_fsb = roundup_64(startoffset_fsb, rounding);
endoffset_fsb = rounddown_64(endoffset_fsb, rounding);
}
ok, and the same idea for xfs_can_free_eofblocks() with
xfs_rtb_roundup_rtx(), right?
What this points out is that the prep steps for fallocate operations
also need to handle both forced alignment and rtextsize rounding,
and it does neither right now. xfs_flush_unmap_range() is the main
offender here, but xfs_prepare_shift() also needs fixing.
When you say fix, is this something to spin off separately for RT? This
series is big enough already...
Hence:
static inline xfs_extlen_t
xfs_extent_alignment(
struct xfs_inode *ip)
{
if (xfs_inode_has_forcealign(ip))
return ip->i_extsize;
if (XFS_IS_REALTIME_INODE(ip))
return mp->m_sb.sb_rextsize;
return 1;
}
In xfs_flush_unmap_range():
/*
* Make sure we extend the flush out to extent alignment
* boundaries so any extent range overlapping the start/end
* of the modification we are about to do is clean and idle.
*/
rounding = XFS_FSB_TO_B(mp, xfs_extent_alignment(ip));
rounding = max(rounding, PAGE_SIZE);
...
in xfs_free_file_space()
/*
* Round the range we are going to free inwards to extent
* alignment boundaries so we don't free blocks outside the
* range requested.
*/
rounding = xfs_extent_alignment(ip);
if (rounding > 1 ) {
startoffset_fsb = roundup_64(startoffset_fsb, rounding);
endoffset_fsb = rounddown_64(endoffset_fsb, rounding);
}
and in xfs_prepare_shift()
/*
* Shift operations must stabilize the start block offset boundary along
* with the full range of the operation. If we don't, a COW writeback
* completion could race with an insert, front merge with the start
* extent (after split) during the shift and corrupt the file. Start
* with the aligned block just prior to the start to stabilize the boundary.
*/
rounding = XFS_FSB_TO_B(mp, xfs_extent_alignment(ip));
offset = round_down(offset, rounding);
if (offset)
offset -= rounding;
Also, I think that the changes I suggested earlier to
xfs_is_falloc_aligned() could use this xfs_extent_alignment()
helper...
Overall this makes the code a whole lot easier to read and it also
allows forced alignment to work correctly on RT devices...
ok, fine
Thanks,
John
