Re: [PATCH v2 02/14] fs: xfs: Don't use low-space allocator for alignment > 1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux