On 03/04/2024 00:44, Dave Chinner wrote:
I seem to have at least 2x problems:
- unexpected -ENOSPC in some case
- sometimes misaligned extents from ordered combo of punch, truncate, write
Punch and truncate do not enforce extent alignment and never have.
They will trim extents to whatever the new EOF block is supposed to
be. This is by design - they are intended to be able to remove
extents beyond EOF that may have been done due to extent size hints
and/or speculative delayed allocation to minimise wasted space.
Again, forced alignment is introducing new constraints, so anything
that is truncating EOF blocks (punch, truncate, eof block freeing
during inactivation or blockgc) is going to need to take forced
extent alignment constraints into account.
Sure
This is likely something that needs to be done in
xfs_itruncate_extents_flags() for the truncate/inactivation/blockgc
cases (i.e. correct calculation of first_unmap_block). Punching and
insert/collapse are a bit more complex - they'll need their
start/end offsets to be aligned, the chunk sizes they work in to be
aligned, and the rounding in xfs_flush_unmap_range() to be forced
alignment aware.
Ack
I don't know if it is a good use of time for me to try to debug, as I guess
you could spot problems in the changes almost immediately.
Next steps:
I would like to send out a new series for XFS support for atomic writes
soon, which so far included forcealign support.
Please advise on your preference for what I should do, like wait for your
forcealign update and then combine into the XFS support for atomic write
series. Or just post the doubtful patches now, as above, and go from there?
I just sent out the forced allocation alignment series for review.
cheers, I'll give them a spin.
Forced truncate/punch extent alignment will need to be implemented
and reviewed as a separate patch set...
Below are my changes.
I think that the xfs_is_falloc_aligned() change is sound. As for the
other two, I'm really not sure.
There is also
https://lore.kernel.org/linux-xfs/ZeeaKrmVEkcXYjbK@xxxxxxxxxxxxxxxxxxx/T/#m73430d56d96e60f2908bab9ce3e6a0d56d27929c,
which still is still a candidate change.
Please check them, below.
Thanks,
John
------>8-------
From ec86dd3add7153062a612cb1141f36544f34e0cd Mon Sep 17 00:00:00 2001
From: John Garry <john.g.garry@xxxxxxxxxx>
Date: Wed, 13 Mar 2024 18:14:37 +0000
Subject: [PATCH 1/3] fs: xfs: Update xfs_is_falloc_aligned() mask forcealign
For when forcealign is enabled, we want the alignment mask to cover an
aligned extent, similar to rtvol.
Signed-off-by: John Garry <john.g.garry@xxxxxxxxxx>
---
fs/xfs/xfs_file.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 632653e00906..e81e01e6b22b 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -61,7 +61,10 @@ xfs_is_falloc_aligned(
}
mask = XFS_FSB_TO_B(mp, mp->m_sb.sb_rextsize) - 1;
} else {
- mask = mp->m_sb.sb_blocksize - 1;
+ if (xfs_inode_has_forcealign(ip) && ip->i_extsize > 1)
+ mask = (mp->m_sb.sb_blocksize * ip->i_extsize) - 1;
+ else
+ mask = mp->m_sb.sb_blocksize - 1;
}
return !((pos | len) & mask);
------8<--------
From c0866d2a5753f1c487872ef3add4e08c03c22d00 Mon Sep 17 00:00:00 2001
From: John Garry <john.g.garry@xxxxxxxxxx>
Date: Fri, 22 Mar 2024 11:24:45 +0000
Subject: [PATCH 2/3] fs: xfs: Unmap blocks according to forcealign
For when forcealign is enabled, blocks in an inode need to be unmapped
according to extent alignment, like what is already done for rtvol.
Signed-off-by: John Garry <john.g.garry@xxxxxxxxxx>
---
fs/xfs/libxfs/xfs_bmap.c | 41 ++++++++++++++++++++++++++++++++++------
fs/xfs/xfs_inode.h | 5 +++++
2 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 7a0ef0900097..5dd7a62625db 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5322,6 +5322,15 @@ xfs_bmap_del_extent_real(
return 0;
}
+/* Return the offset of an block number within an extent for forcealign. */
+static xfs_extlen_t
+xfs_forcealign_extent_offset(
+ struct xfs_inode *ip,
+ xfs_fsblock_t bno)
+{
+ return bno & (ip->i_extsize - 1);
+}
+
/*
* Unmap (remove) blocks from a file.
* If nexts is nonzero then the number of extents to remove is limited to
@@ -5344,6 +5353,7 @@ __xfs_bunmapi(
struct xfs_bmbt_irec got; /* current extent record */
struct xfs_ifork *ifp; /* inode fork pointer */
int isrt; /* freeing in rt area */
+ int isforcealign; /* freeing for file inode with forcealign */
int logflags; /* transaction logging flags */
xfs_extlen_t mod; /* rt extent offset */
struct xfs_mount *mp = ip->i_mount;
@@ -5380,7 +5390,10 @@ __xfs_bunmapi(
return 0;
}
XFS_STATS_INC(mp, xs_blk_unmap);
- isrt = xfs_ifork_is_realtime(ip, whichfork);
+ isrt = (whichfork == XFS_DATA_FORK) && XFS_IS_REALTIME_INODE(ip);
+ isforcealign = (whichfork == XFS_DATA_FORK) &&
+ xfs_inode_has_forcealign(ip) &&
+ xfs_inode_has_extsize(ip) && ip->i_extsize > 1;
end = start + len;
if (!xfs_iext_lookup_extent_before(ip, ifp, &end, &icur, &got)) {
@@ -5442,11 +5455,17 @@ __xfs_bunmapi(
if (del.br_startoff + del.br_blockcount > end + 1)
del.br_blockcount = end + 1 - del.br_startoff;
- if (!isrt || (flags & XFS_BMAPI_REMAP))
+ if ((!isrt && !isforcealign) || (flags & XFS_BMAPI_REMAP))
goto delete;
- mod = xfs_rtb_to_rtxoff(mp,
- del.br_startblock + del.br_blockcount);
+ if (isrt)
+ mod = xfs_rtb_to_rtxoff(mp,
+ del.br_startblock + del.br_blockcount);
+ else if (isforcealign)
+ mod = xfs_forcealign_extent_offset(ip,
+ del.br_startblock + del.br_blockcount);
if (mod) {
/*
* Realtime extent not lined up at the end.
@@ -5494,9 +5513,19 @@ __xfs_bunmapi(
goto nodelete;
}
- mod = xfs_rtb_to_rtxoff(mp, del.br_startblock);
+ if (isrt)
+ mod = xfs_rtb_to_rtxoff(mp, del.br_startblock);
+ else if (isforcealign)
+ mod = xfs_forcealign_extent_offset(ip,
+ del.br_startblock);
+
if (mod) {
- xfs_extlen_t off = mp->m_sb.sb_rextsize - mod;
+ xfs_extlen_t off;
+ if (isrt)
+ off = mp->m_sb.sb_rextsize - mod;
+ else if (isforcealign) {
+ off = ip->i_extsize - mod;
+ }
/*
* Realtime extent is lined up at the end but not
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 065028789473..3f13943ab3a3 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -316,6 +316,11 @@ static inline bool xfs_inode_has_forcealign(struct
xfs_inode *ip)
return ip->i_diflags2 & XFS_DIFLAG2_FORCEALIGN;
}
+static inline bool xfs_inode_has_extsize(struct xfs_inode *ip)
+{
+ return ip->i_diflags & XFS_DIFLAG_EXTSIZE;
+}
+
/*
* Return the buftarg used for data allocations on a given inode.
------>8-------
From 8cb2b61fa419b961d22609e12f2d941ce976d0f0 Mon Sep 17 00:00:00 2001
From: John Garry <john.g.garry@xxxxxxxxxx>
Date: Wed, 20 Mar 2024 13:05:39 +0000
Subject: [PATCH 3/3] fs: xfs: Only free full extents for forcealign
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 178a4865d1ed..e3e6e27a33bf 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -848,8 +848,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);
}
------8<--------
EOM
