From: John Garry <john.g.garry@oracle.com>
To: Dave Chinner <david@fromorbit.com>, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 1/5] xfs: only allow minlen allocations when near ENOSPC
Date: Wed, 3 Apr 2024 17:31:49 +0100 [thread overview]
Message-ID: <c515119e-5f0e-41ba-8bde-ae9f6283b3d8@oracle.com> (raw)
In-Reply-To: <20240402233006.1210262-2-david@fromorbit.com>
On 03/04/2024 00:28, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> When we are near ENOSPC and don't have enough free
> space for an args->maxlen allocation, xfs_alloc_space_available()
> will trim args->maxlen to equal the available space. However, this
> function has only checked that there is enough contiguous free space
> for an aligned args->minlen allocation to succeed. Hence there is no
> guarantee that an args->maxlen allocation will succeed, nor that the
> available space will allow for correct alignment of an args->maxlen
> allocation.
>
> Further, by trimming args->maxlen arbitrarily, it breaks an
> assumption made in xfs_alloc_fix_len() that if the caller wants
> aligned allocation, then args->maxlen will be set to an aligned
> value. It then skips the tail alignment and so we end up with
> extents that aren't aligned to extent size hint boundaries as we
> approach ENOSPC.
>
> To avoid this problem, don't reduce args->maxlen by some random,
> arbitrary amount. If args->maxlen is too large for the available
> space, reduce the allocation to a minlen allocation as we know we
> have contiguous free space available for this to succeed and always
> be correctly aligned.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
This change seems to cause or at least expose a problem for me - I say
that because it is the only difference to what I already had from
https://lore.kernel.org/linux-xfs/ZeeaKrmVEkcXYjbK@dread.disaster.area/T/#me7abe09fe85292ca880f169a4af651eac5ed1424
and the xfs_alloc_fix_len() fix.
With forcealign extsize=64KB, when I write to the end of a file I get 2x
new extents, both of which are not a multiple of 64KB in size. Note that
I am including
https://lore.kernel.org/linux-xfs/20240304130428.13026-7-john.g.garry@oracle.com/,
but I don't think it makes a difference.
Before:
EXT: FILE-OFFSET BLOCK-RANGE AG AG-OFFSET TOTAL
0: [0..383]: 73216..73599 0 (73216..73599) 384
1: [384..511]: 70528..70655 0 (70528..70655) 128
After:
EXT: FILE-OFFSET BLOCK-RANGE AG AG-OFFSET TOTAL
0: [0..383]: 73216..73599 0 (73216..73599) 384
1: [384..511]: 70528..70655 0 (70528..70655) 128
2: [512..751]: 30336..30575 0 (30336..30575) 240
3: [752..767]: 48256..48271 0 (48256..48271) 16
> ---
> fs/xfs/libxfs/xfs_alloc.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 9da52e92172a..215265e0f68f 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -2411,14 +2411,23 @@ xfs_alloc_space_available(
> if (available < (int)max(args->total, alloc_len))
> return false;
>
> + if (flags & XFS_ALLOC_FLAG_CHECK)
> + return true;
> +
> /*
> - * Clamp maxlen to the amount of free space available for the actual
> - * extent allocation.
> + * If we can't do a maxlen allocation, then we must reduce the size of
> + * the allocation to match the available free space. We know how big
> + * the largest contiguous free space we can allocate is, so that's our
> + * upper bound. However, we don't exaclty know what alignment/siz > + * constraints have been placed on the allocation, so we can't
> + * arbitrarily select some new max size. Hence make this a minlen
> + * allocation as we know that will definitely succeed and match the
> + * callers alignment constraints.
> */
> - if (available < (int)args->maxlen && !(flags & XFS_ALLOC_FLAG_CHECK)) {
> - args->maxlen = available;
> + alloc_len = args->maxlen + (args->alignment - 1) + args->minalignslop;
I added some kernel logs to assist debugging, and if I am reading them
correctly, for ext #2 allocation we had at this point:
longest = 46, alloc_len = 47, args->minlen=30, maxlen=32, alignslop=0
available=392; longest < alloc_len, so we set args->maxlen =
args->minlen (= 30)
For ext3:
longest = 32, alloc_len = 17, args->minlen=2, args->maxlen=2,
alignslop=0, available=362; longest > alloc_len, so do nothing
> + if (longest < alloc_len) {
> + args->maxlen = args->minlen;
> ASSERT(args->maxlen > 0);
> - ASSERT(args->maxlen >= args->minlen);
> }
>
> return true;
next prev parent reply other threads:[~2024-04-03 16:31 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-02 23:28 [PATCH 0/5] xfs: allocation alignment for forced alignment Dave Chinner
2024-04-02 23:28 ` [PATCH 1/5] xfs: only allow minlen allocations when near ENOSPC Dave Chinner
2024-04-03 16:31 ` John Garry [this message]
2024-04-03 23:15 ` Dave Chinner
2024-04-04 13:54 ` John Garry
2024-04-02 23:28 ` [PATCH 2/5] xfs: always tail align maxlen allocations Dave Chinner
2024-04-02 23:28 ` [PATCH 3/5] xfs: simplify extent allocation alignment Dave Chinner
2024-04-02 23:28 ` [PATCH 4/5] xfs: make EOF allocation simpler Dave Chinner
2024-04-02 23:28 ` [PATCH 5/5] xfs: introduce forced allocation alignment Dave Chinner
2024-04-10 12:44 ` [PATCH 0/5] xfs: allocation alignment for forced alignment John Garry
2024-04-16 0:38 ` Dave Chinner
2024-04-16 6:51 ` John Garry
2024-04-29 14:06 ` John Garry
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c515119e-5f0e-41ba-8bde-ae9f6283b3d8@oracle.com \
--to=john.g.garry@oracle.com \
--cc=david@fromorbit.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).