On 04/03/2024 22:15, Dave Chinner wrote:
On Mon, Mar 04, 2024 at 01:04:16PM +0000, John Garry wrote:
The low-space allocator doesn't honour the alignment requirement, so don't
attempt to even use it (when we have an alignment requirement).
Signed-off-by: John Garry <john.g.garry@xxxxxxxxxx>
---
fs/xfs/libxfs/xfs_bmap.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index f362345467fa..60d100134280 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3584,6 +3584,10 @@ xfs_bmap_btalloc_low_space(
{
int error;
+ /* The allocator doesn't honour args->alignment */
+ if (args->alignment > 1)
+ return 0;
I think that's wrong.
The alignment argument here is purely a best effort consideration -
we ignore it several different allocation situations, not just low
space.
Sure, but I am simply addressing the low-space allocator here.
In this series I am /we are effectively trying to conflate
args->alignment > 1 with forcealign. I thought that args->alignment was
guaranteed to be honoured, with some caveats. For forcealign, we
obviously require a guarantee.
e.g. xfs_bmap_btalloc_at_eof() will try exact block
allocation regardless of whether an alignment parameter is set.
For this specific issue, I think that we are ok, as:
- in xfs_bmap_compute_alignments(), stripe_align is aligned with
args->alignment for forcealign
- xfs_bmap_btalloc_at_eof() has the optimisation to alloc according to
stripe alignment
But obviously we should not be relying on optimisations.
Please also note that I have a modification later in this series to
always have EOF aligned for forcealign.
It
will then fall back to stripe alignment if exact block fails.
If stripe aligned allocation fails, it will then set args->alignment
= 1 and try a full filesystem allocation scan without alignment.
And if that fails, then we finally get to the low space allocator
with args->alignment = 1 even though we might be trying to allocate
an aligned extent for an atomic IO....
IOWs, I think this indicates deeper surgery is needed to ensure
aligned allocations fail immediately and don't fall back to
unaligned allocations and set XFS_TRANS_LOW_MODE...
ok, I'll look at what you write about all of this in the later patch review.
Thanks,
John
