* [RFC PATCH 0/9] xfs: push perags further into allocation routines
@ 2023-10-04 0:19 Dave Chinner
2023-10-04 0:19 ` [PATCH 1/9] xfs: split xfs_bmap_btalloc_at_eof() Dave Chinner
` (8 more replies)
0 siblings, 9 replies; 27+ messages in thread
From: Dave Chinner @ 2023-10-04 0:19 UTC (permalink / raw)
To: linux-xfs; +Cc: john.g.garry
This series continues the work towards making shrinking a filesystem
possible. We need to be able to stop operations from taking place
on AGs that need to be removed by a shrink, so before shrink can be
implemented we need to have the infrastructure in place to prevent
incursion into AGs that are going to be, or are in the process, of
being removed from active duty.
The focus of this is making operations that depend on access to AGs
use the perag to access and pin the AG in active use, thereby
creating a barrier we can use to delay shrink until all active uses
of an AG have been drained and new uses are prevented.
The previous round of allocator changes pushed per-ags mostly into
the core of the allocator, but it left some rough edges where
allocation routine may or may not be called with perags already
held. This series continues the work on driving the perag further
outwards into the bmap and individual allocation layers to clean up
these warts.
The bmap allocators have some interesting complexities. For example,
they might attempt exact block allocation before attempting aligned
allocation, and then in some cases want to attempt aligned
allocation anywhere in the filesytsem instead of in the same AG as
tehy do in other cases. Hence the code is somewhat complex as it
tries to handle all these different cases.
The first step in untangling it all is splitting the exact block EOF
case away from aligned allocation. If that fails, then we can
attempt a near block aligned allocation. The filestreams allocator
already does this with an attempt to allocate only in the same AG as
the EOF block, but the normal allocator does an "all AG near block"
scan. This latter cases starts with a "near block in the same AG"
pass, then tries any other AG, but it requires dropping the perag
before we start, hence doesn't provide any guarantee that we can
actually get the same start AG again....
This separation then exposes the cases where we should be doing
aligned allocation but we don't or we attempt aligned allocation
when we know it can't succeed. There are several small changes to
take this sort of thing into account when selecting the initial AG
to allocate from.
With that, we then push the perag management out into the intial AG
selection code, thereby guaranteeing that we hold the selected AG
until we've failed all the AG sepcific allocation attempts the
policy defines.
Given that we now largely guarantee we select an AG with enough
space for the initial aligned allocation, there is no longer a need
to do an "all AGs" aligne allocation attempt. We know it can be done
in the selected AG, so failure should be very rare and this allows
us to use the same initial single AG EOF/aligned allocation logic
for both allocation policies.
This then allows us to move to {agno,agbno} based allocation
targets, rather than fsblock based targets. It also means that we
always call xfs_alloc_vextent_exact_bno() with a perag held, so we
can get rid of the conditional perag code in that function. This
makes _near_bno() and _exact_bno() essentially identical except for
the allocation function they call, so they can be collapsed into a
common helper.
And with all this, we now have the APIs simplified to the point
where we can change how allocation failure is signalled. Rather than
having the intenral AG allocators returning success with
arags->agbno == NULLAGBLOCK to indication ENOSPC, and then having to
convert that to returning success with args->fsblock == NULLFSBLOCK
to indicate allocation failure to the higher layers, we can convert
all the code to return -ENOSPC when allocation failure occurs.
This is intended to avoid the problems inherent in detecting
"successful allocation that failed" situation that lead to the data
corruption problem in the 6.3 cycle - if we fail to catch ENOSPC
explicitly now, the allocation will still return an error and fail.
Such a failure will likely result in a filesystem shutdown, which is
a *much* better failure behaviour than writing data to a random
location on the block device....
The end result is a slightly more efficient allocation path that
selects AGs at the highest level possible for initial allocation
attempts, uses ENOSPC errors to detect allocation failures, and only
uses AG iteration based allocation algorithms in the cases where the
initial targetted allocations fail. It also makes it much clearer
where we are doing stripe aligned allocations versus non-aligned
allocations.
This passes fstests and various data tests (e.g fio), but hasn't
been strenuously tested yet. I'm posting it because of the
forced-align functionality that has been talked about, and this
series makes it quite clear what "aligned allocation" currently
means and how that is quite different to what "force-align" is
intended to mean.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/9] xfs: split xfs_bmap_btalloc_at_eof()
2023-10-04 0:19 [RFC PATCH 0/9] xfs: push perags further into allocation routines Dave Chinner
@ 2023-10-04 0:19 ` Dave Chinner
2023-10-04 23:41 ` Darrick J. Wong
2023-10-05 9:41 ` Christoph Hellwig
2023-10-04 0:19 ` [PATCH 2/9] xfs: contiguous EOF allocation across AGs Dave Chinner
` (7 subsequent siblings)
8 siblings, 2 replies; 27+ messages in thread
From: Dave Chinner @ 2023-10-04 0:19 UTC (permalink / raw)
To: linux-xfs; +Cc: john.g.garry
From: Dave Chinner <dchinner@redhat.com>
This function is really implementing two policies. The first is
trying to create contiguous extents at file extension. Failing that,
it attempts to perform aligned allocation. These are really two
separate policies, and it is further complicated by filestream
allocation having different aligned allocation constraints than
the normal bmap allocation.
Split xfs_bmap_btalloc_at_eof() into two parts so we can start to
align the two different allocator policies more closely.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 147 +++++++++++++++++++++------------------
1 file changed, 80 insertions(+), 67 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 30c931b38853..e14671414afb 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3451,81 +3451,81 @@ xfs_bmap_exact_minlen_extent_alloc(
#endif
-/*
- * If we are not low on available data blocks and we are allocating at
- * EOF, optimise allocation for contiguous file extension and/or stripe
- * alignment of the new extent.
- *
- * NOTE: ap->aeof is only set if the allocation length is >= the
- * stripe unit and the allocation offset is at the end of file.
- */
+ /*
+ * Attempt contiguous allocation for file extension.
+ */
+ static int
+ xfs_bmap_btalloc_at_eof(
+ struct xfs_bmalloca *ap,
+ struct xfs_alloc_arg *args,
+ xfs_extlen_t blen,
+ int stripe_align)
+{
+ struct xfs_mount *mp = args->mp;
+ struct xfs_perag *caller_pag = args->pag;
+ xfs_extlen_t nextminlen = 0;
+ int error;
+
+ /*
+ * Compute the minlen+alignment for the next case. Set slop so
+ * that the value of minlen+alignment+slop doesn't go up between
+ * the calls.
+ */
+ args->alignment = 1;
+ if (blen > stripe_align && blen <= args->maxlen)
+ nextminlen = blen - stripe_align;
+ else
+ nextminlen = args->minlen;
+ if (nextminlen + stripe_align > args->minlen + 1)
+ args->minalignslop = nextminlen + stripe_align -
+ args->minlen - 1;
+ else
+ args->minalignslop = 0;
+
+ if (!caller_pag)
+ args->pag = xfs_perag_get(mp, XFS_FSB_TO_AGNO(mp, ap->blkno));
+ error = xfs_alloc_vextent_exact_bno(args, ap->blkno);
+ if (!caller_pag) {
+ xfs_perag_put(args->pag);
+ args->pag = NULL;
+ }
+ if (error)
+ return error;
+
+ if (args->fsbno != NULLFSBLOCK)
+ return 0;
+ /*
+ * Exact allocation failed. Reset to try an aligned allocation
+ * according to the original allocation specification.
+ */
+ args->minlen = nextminlen;
+ return 0;
+}
+
static int
-xfs_bmap_btalloc_at_eof(
+xfs_bmap_btalloc_aligned(
struct xfs_bmalloca *ap,
struct xfs_alloc_arg *args,
xfs_extlen_t blen,
int stripe_align,
bool ag_only)
{
- struct xfs_mount *mp = args->mp;
- struct xfs_perag *caller_pag = args->pag;
+ struct xfs_perag *caller_pag = args->pag;
int error;
/*
- * If there are already extents in the file, try an exact EOF block
- * allocation to extend the file as a contiguous extent. If that fails,
- * or it's the first allocation in a file, just try for a stripe aligned
- * allocation.
+ * If we failed an exact EOF allocation already, stripe
+ * alignment will have already been taken into account in
+ * args->minlen. Hence we only adjust minlen to try to preserve
+ * alignment if no slop has been reserved for alignment
*/
- if (ap->offset) {
- xfs_extlen_t nextminlen = 0;
-
- /*
- * Compute the minlen+alignment for the next case. Set slop so
- * that the value of minlen+alignment+slop doesn't go up between
- * the calls.
- */
- args->alignment = 1;
- if (blen > stripe_align && blen <= args->maxlen)
- nextminlen = blen - stripe_align;
- else
- nextminlen = args->minlen;
- if (nextminlen + stripe_align > args->minlen + 1)
- args->minalignslop = nextminlen + stripe_align -
- args->minlen - 1;
- else
- args->minalignslop = 0;
-
- if (!caller_pag)
- args->pag = xfs_perag_get(mp, XFS_FSB_TO_AGNO(mp, ap->blkno));
- error = xfs_alloc_vextent_exact_bno(args, ap->blkno);
- if (!caller_pag) {
- xfs_perag_put(args->pag);
- args->pag = NULL;
- }
- if (error)
- return error;
-
- if (args->fsbno != NULLFSBLOCK)
- return 0;
- /*
- * Exact allocation failed. Reset to try an aligned allocation
- * according to the original allocation specification.
- */
- args->alignment = stripe_align;
- args->minlen = nextminlen;
- args->minalignslop = 0;
- } else {
- /*
- * Adjust minlen to try and preserve alignment if we
- * can't guarantee an aligned maxlen extent.
- */
- args->alignment = stripe_align;
- if (blen > args->alignment &&
- blen <= args->maxlen + args->alignment)
- args->minlen = blen - args->alignment;
- args->minalignslop = 0;
+ if (args->minalignslop == 0) {
+ if (blen > stripe_align &&
+ blen <= args->maxlen + stripe_align)
+ args->minlen = blen - stripe_align;
}
+ args->alignment = stripe_align;
+ args->minalignslop = 0;
if (ag_only) {
error = xfs_alloc_vextent_near_bno(args, ap->blkno);
@@ -3612,8 +3612,14 @@ xfs_bmap_btalloc_filestreams(
}
args->minlen = xfs_bmap_select_minlen(ap, args, blen);
+ if (ap->aeof && ap->offset)
+ error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align);
+
+ if (error || args->fsbno != NULLFSBLOCK)
+ goto out_low_space;
+
if (ap->aeof)
- error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align,
+ error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align,
true);
if (!error && args->fsbno == NULLFSBLOCK)
@@ -3662,13 +3668,20 @@ xfs_bmap_btalloc_best_length(
* optimal or even aligned allocations in this case, so don't waste time
* trying.
*/
- if (ap->aeof && !(ap->tp->t_flags & XFS_TRANS_LOWMODE)) {
- error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align,
- false);
+ if (ap->aeof && ap->offset && !(ap->tp->t_flags & XFS_TRANS_LOWMODE)) {
+ error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align);
if (error || args->fsbno != NULLFSBLOCK)
return error;
}
+ /* attempt aligned allocation for new EOF extents */
+ if (ap->aeof)
+ error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align,
+ false);
+ if (error || args->fsbno != NULLFSBLOCK)
+ return error;
+
+ /* attempt unaligned allocation */
error = xfs_alloc_vextent_start_ag(args, ap->blkno);
if (error || args->fsbno != NULLFSBLOCK)
return error;
--
2.40.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/9] xfs: contiguous EOF allocation across AGs
2023-10-04 0:19 [RFC PATCH 0/9] xfs: push perags further into allocation routines Dave Chinner
2023-10-04 0:19 ` [PATCH 1/9] xfs: split xfs_bmap_btalloc_at_eof() Dave Chinner
@ 2023-10-04 0:19 ` Dave Chinner
2023-10-05 9:43 ` Christoph Hellwig
2023-10-06 5:27 ` Darrick J. Wong
2023-10-04 0:19 ` [PATCH 3/9] xfs: select the AG with the largest contiguous space Dave Chinner
` (6 subsequent siblings)
8 siblings, 2 replies; 27+ messages in thread
From: Dave Chinner @ 2023-10-04 0:19 UTC (permalink / raw)
To: linux-xfs; +Cc: john.g.garry
From: Dave Chinner <dchinner@redhat.com>
Currently when we allocate at EOF, we set the initial target to the
location of the inode. Then we call xfs_bmap_adjacent(), which sees
that we are doing an EOF extension, so it moves the target to the
last block of the previous extent. This may be in a different AG to
the inode.
When we then go to select the AG with the best free length, the AG
at the target block might not have sufficient free space for the
full allocation, so we may select a different AG. We then do an
exact BNO allocation with the original target (EOF block), which
reverts back to attempting an allocation in an AG that doesn't have
sufficient contiguous free space available.
This generally leads to allocation failure, and then we fall back to
scanning the AGs for one that the allocation will succeed in. This
scan also results in contended AGS being skipped, so we have no idea
what AG we are going to end up allocating in. For sequential writes,
this results in random extents being located in random places in
non-target AGs.
We want to guarantee that we can allocate in the AG that we have
selected as having the "best contiguous free space" efficiently,
so if we select a different AG, we should move the allocation target
and skip the exact EOF allocation as we know it will not succeed.
i.e. we should start with aligned allocation immediately, knowing it
will likely succeed.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index e14671414afb..e64ba7e2d13d 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3252,8 +3252,18 @@ xfs_bmap_btalloc_select_lengths(
if (error && error != -EAGAIN)
break;
error = 0;
- if (*blen >= args->maxlen)
+ if (*blen >= args->maxlen) {
+ /*
+ * We are going to target a different AG than the
+ * incoming target, so we need to reset the target and
+ * skip exact EOF allocation attempts.
+ */
+ if (agno != startag) {
+ ap->blkno = XFS_AGB_TO_FSB(mp, agno, 0);
+ ap->aeof = false;
+ }
break;
+ }
}
if (pag)
xfs_perag_rele(pag);
@@ -3514,10 +3524,10 @@ xfs_bmap_btalloc_aligned(
int error;
/*
- * If we failed an exact EOF allocation already, stripe
- * alignment will have already been taken into account in
- * args->minlen. Hence we only adjust minlen to try to preserve
- * alignment if no slop has been reserved for alignment
+ * If we failed an exact EOF allocation already, stripe alignment will
+ * have already been taken into account in args->minlen. Hence we only
+ * adjust minlen to try to preserve alignment if no slop has been
+ * reserved for alignment
*/
if (args->minalignslop == 0) {
if (blen > stripe_align &&
@@ -3653,6 +3663,16 @@ xfs_bmap_btalloc_best_length(
ap->blkno = XFS_INO_TO_FSB(args->mp, ap->ip->i_ino);
xfs_bmap_adjacent(ap);
+ /*
+ * We only use stripe alignment for EOF allocations. Hence if it isn't
+ * an EOF allocation, clear the stripe alignment. This allows us to
+ * skip exact block EOF allocation yet still do stripe aligned
+ * allocation if we select a different AG to the
+ * exact target block due to a lack of contiguous free space.
+ */
+ if (!ap->aeof)
+ stripe_align = 0;
+
/*
* Search for an allocation group with a single extent large enough for
* the request. If one isn't found, then adjust the minimum allocation
@@ -3675,7 +3695,7 @@ xfs_bmap_btalloc_best_length(
}
/* attempt aligned allocation for new EOF extents */
- if (ap->aeof)
+ if (stripe_align)
error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align,
false);
if (error || args->fsbno != NULLFSBLOCK)
--
2.40.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/9] xfs: select the AG with the largest contiguous space
2023-10-04 0:19 [RFC PATCH 0/9] xfs: push perags further into allocation routines Dave Chinner
2023-10-04 0:19 ` [PATCH 1/9] xfs: split xfs_bmap_btalloc_at_eof() Dave Chinner
2023-10-04 0:19 ` [PATCH 2/9] xfs: contiguous EOF allocation across AGs Dave Chinner
@ 2023-10-04 0:19 ` Dave Chinner
2023-10-05 9:45 ` Christoph Hellwig
2023-10-05 9:46 ` Christoph Hellwig
2023-10-04 0:19 ` [PATCH 4/9] xfs: push the perag outwards in initial allocation Dave Chinner
` (5 subsequent siblings)
8 siblings, 2 replies; 27+ messages in thread
From: Dave Chinner @ 2023-10-04 0:19 UTC (permalink / raw)
To: linux-xfs; +Cc: john.g.garry
From: Dave Chinner <dchinner@redhat.com>
If we don't find an AG with sufficient contiguous free space for a
maxlen allocation, make sure we select the AG with the largest
contiguous free space to allocate from.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index e64ba7e2d13d..ee1c1415c67a 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3233,6 +3233,8 @@ xfs_bmap_btalloc_select_lengths(
struct xfs_mount *mp = args->mp;
struct xfs_perag *pag;
xfs_agnumber_t agno, startag;
+ xfs_agnumber_t max_blen_agno;
+ xfs_extlen_t max_blen = 0;
int error = 0;
if (ap->tp->t_flags & XFS_TRANS_LOWMODE) {
@@ -3264,10 +3266,22 @@ xfs_bmap_btalloc_select_lengths(
}
break;
}
+ if (*blen > max_blen) {
+ max_blen = *blen;
+ max_blen_agno = agno;
+ }
}
if (pag)
xfs_perag_rele(pag);
+ if (max_blen > *blen) {
+ if (max_blen_agno != startag) {
+ ap->blkno = XFS_AGB_TO_FSB(mp, max_blen_agno, 0);
+ ap->aeof = false;
+ }
+ *blen = max_blen;
+ }
+
args->minlen = xfs_bmap_select_minlen(ap, args, *blen);
return error;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 4/9] xfs: push the perag outwards in initial allocation
2023-10-04 0:19 [RFC PATCH 0/9] xfs: push perags further into allocation routines Dave Chinner
` (2 preceding siblings ...)
2023-10-04 0:19 ` [PATCH 3/9] xfs: select the AG with the largest contiguous space Dave Chinner
@ 2023-10-04 0:19 ` Dave Chinner
2023-10-05 11:57 ` Christoph Hellwig
2023-10-04 0:19 ` [PATCH 5/9] xfs: aligned EOF allocations don't need to scan AGs anymore Dave Chinner
` (4 subsequent siblings)
8 siblings, 1 reply; 27+ messages in thread
From: Dave Chinner @ 2023-10-04 0:19 UTC (permalink / raw)
To: linux-xfs; +Cc: john.g.garry
From: Dave Chinner <dchinner@redhat.com>
In xfs_bmap_btalloc_best_length(), we first select the best length
to allocate by scanning all the AGs to check free space and
contiguous extents. From this, we effectively select a target AG,
but we don't take a perag reference anywhere. We only take a perag
reference when we actually go to do an allocation. This means we
end up calling xfs_bmap_btalloc_at_eof() from different places, one
with a perag reference and one without, and so it has to manage
grabbing the perag reference if necessary.
We should be grabbing this perag reference when we determine the
first AG with a suitable extent in it in
xfs_bmap_btalloc_select_lengths(), rather than determining that
there is some AG tha has a suitable contiguous extent in it and then
trying maxlen allocations from the start AG and failing until we
finally hit the AG that has that maxlen extent in it.
We also need to pass the stripe alignment blocks needed to
xfs_bmap_btalloc_select_lengths() so that this is taken into account
when we scan the AGs for an appropriate contiguous extent size.
At this point, we want the near and exact block allocation
algorithms to take the AG they operate in from args->pag rather than
from the the target block number. The high level callers have
already all selected an AG and grabbed a perag, so the "agno"
portion of the target is no longer relevant and may, in fact, be
stale. i.e. the caller should set up args->pag to points to the AG
that we want to allocate from so that we don't end up in the
situation where the target fsbno points to a different AG and we
attempt an allocation on a AG that we don't hold a perag reference
to.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 96 ++++++++++++++++++++++++++--------------
1 file changed, 62 insertions(+), 34 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index ee1c1415c67a..3c250c89f42e 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3224,10 +3224,26 @@ xfs_bmap_select_minlen(
return args->maxlen;
}
+/*
+ * Find the first AG with sufficient contiguous free space to allocate the
+ * entire extent and return with a reference held to that perag in args->pag.
+ * This takes into account stripe alignment rounding for the extent as well, so
+ * we don't end selecting an AG that gets rejected multiple times trying to do
+ * aligned allocation in AGs that don't quite have enough contiguous free space
+ * for aligned allocation to succed.
+ *
+ * If no maxlen contiguous extent can be found, just grab the perag for the AG
+ * that matches the target (startag) and let the allocation routines iterate to
+ * find the best candidate themselves.
+ *
+ * This function also sets the minimum and maximum lengths acceptable for
+ * optimal allocation in this context.
+ */
static int
xfs_bmap_btalloc_select_lengths(
struct xfs_bmalloca *ap,
struct xfs_alloc_arg *args,
+ int stripe_align,
xfs_extlen_t *blen)
{
struct xfs_mount *mp = args->mp;
@@ -3235,26 +3251,24 @@ xfs_bmap_btalloc_select_lengths(
xfs_agnumber_t agno, startag;
xfs_agnumber_t max_blen_agno;
xfs_extlen_t max_blen = 0;
- int error = 0;
-
- if (ap->tp->t_flags & XFS_TRANS_LOWMODE) {
- args->total = ap->minlen;
- args->minlen = ap->minlen;
- return 0;
- }
+ ASSERT(args->pag == NULL);
args->total = ap->total;
+
startag = XFS_FSB_TO_AGNO(mp, ap->blkno);
if (startag == NULLAGNUMBER)
startag = 0;
*blen = 0;
+ max_blen_agno = startag;
for_each_perag_wrap(mp, startag, agno, pag) {
- error = xfs_bmap_longest_free_extent(pag, args->tp, blen);
- if (error && error != -EAGAIN)
- break;
- error = 0;
- if (*blen >= args->maxlen) {
+ int error = xfs_bmap_longest_free_extent(pag, args->tp, blen);
+
+ if (error && error != -EAGAIN) {
+ xfs_perag_rele(pag);
+ return error;
+ }
+ if (*blen >= args->maxlen + stripe_align) {
/*
* We are going to target a different AG than the
* incoming target, so we need to reset the target and
@@ -3264,6 +3278,7 @@ xfs_bmap_btalloc_select_lengths(
ap->blkno = XFS_AGB_TO_FSB(mp, agno, 0);
ap->aeof = false;
}
+ args->pag = pag;
break;
}
if (*blen > max_blen) {
@@ -3271,19 +3286,19 @@ xfs_bmap_btalloc_select_lengths(
max_blen_agno = agno;
}
}
- if (pag)
- xfs_perag_rele(pag);
- if (max_blen > *blen) {
+ if (max_blen >= *blen) {
+ ASSERT(args->pag == NULL);
if (max_blen_agno != startag) {
ap->blkno = XFS_AGB_TO_FSB(mp, max_blen_agno, 0);
ap->aeof = false;
}
*blen = max_blen;
+ args->pag = xfs_perag_grab(mp, max_blen_agno);
}
args->minlen = xfs_bmap_select_minlen(ap, args, *blen);
- return error;
+ return 0;
}
/* Update all inode and quota accounting for the allocation we just did. */
@@ -3485,8 +3500,6 @@ xfs_bmap_exact_minlen_extent_alloc(
xfs_extlen_t blen,
int stripe_align)
{
- struct xfs_mount *mp = args->mp;
- struct xfs_perag *caller_pag = args->pag;
xfs_extlen_t nextminlen = 0;
int error;
@@ -3506,13 +3519,7 @@ xfs_bmap_exact_minlen_extent_alloc(
else
args->minalignslop = 0;
- if (!caller_pag)
- args->pag = xfs_perag_get(mp, XFS_FSB_TO_AGNO(mp, ap->blkno));
error = xfs_alloc_vextent_exact_bno(args, ap->blkno);
- if (!caller_pag) {
- xfs_perag_put(args->pag);
- args->pag = NULL;
- }
if (error)
return error;
@@ -3677,6 +3684,17 @@ xfs_bmap_btalloc_best_length(
ap->blkno = XFS_INO_TO_FSB(args->mp, ap->ip->i_ino);
xfs_bmap_adjacent(ap);
+ /*
+ * If we are in low space mode, then optimal allocation will fail so
+ * prepare for minimal allocation and run the low space algorithm
+ * immediately.
+ */
+ if (ap->tp->t_flags & XFS_TRANS_LOWMODE) {
+ args->minlen = ap->minlen;
+ ASSERT(args->fsbno == NULLFSBLOCK);
+ return xfs_bmap_btalloc_low_space(ap, args);
+ }
+
/*
* We only use stripe alignment for EOF allocations. Hence if it isn't
* an EOF allocation, clear the stripe alignment. This allows us to
@@ -3692,22 +3710,32 @@ xfs_bmap_btalloc_best_length(
* the request. If one isn't found, then adjust the minimum allocation
* size to the largest space found.
*/
- error = xfs_bmap_btalloc_select_lengths(ap, args, &blen);
+ error = xfs_bmap_btalloc_select_lengths(ap, args, stripe_align, &blen);
if (error)
return error;
+ ASSERT(args->pag);
- /*
- * Don't attempt optimal EOF allocation if previous allocations barely
- * succeeded due to being near ENOSPC. It is highly unlikely we'll get
- * optimal or even aligned allocations in this case, so don't waste time
- * trying.
- */
- if (ap->aeof && ap->offset && !(ap->tp->t_flags & XFS_TRANS_LOWMODE)) {
+ if (ap->aeof && ap->offset) {
error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align);
- if (error || args->fsbno != NULLFSBLOCK)
- return error;
}
+ /*
+ * We are now done with the perag reference for the optimal allocation
+ * association provided by xfs_bmap_btalloc_select_lengths(). Release it
+ * now as we've either succeeded, had a fatal error or we are out of
+ * space and need to do a full filesystem scan for free space which will
+ * take it's own references.
+ *
+ * XXX: now that xfs_bmap_btalloc_select_lengths() selects an AG with
+ * enough contiguous free space in it for an aligned allocation, we
+ * can change the aligned allocation at EOF to just be a single AG
+ * allocation.
+ */
+ xfs_perag_rele(args->pag);
+ args->pag = NULL;
+ if (error || args->fsbno != NULLFSBLOCK)
+ return error;
+
/* attempt aligned allocation for new EOF extents */
if (stripe_align)
error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align,
--
2.40.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 5/9] xfs: aligned EOF allocations don't need to scan AGs anymore
2023-10-04 0:19 [RFC PATCH 0/9] xfs: push perags further into allocation routines Dave Chinner
` (3 preceding siblings ...)
2023-10-04 0:19 ` [PATCH 4/9] xfs: push the perag outwards in initial allocation Dave Chinner
@ 2023-10-04 0:19 ` Dave Chinner
2023-10-05 11:59 ` Christoph Hellwig
2023-10-06 5:55 ` Darrick J. Wong
2023-10-04 0:19 ` [PATCH 6/9] xfs: use agno/agbno in xfs_alloc_vextent functions Dave Chinner
` (3 subsequent siblings)
8 siblings, 2 replies; 27+ messages in thread
From: Dave Chinner @ 2023-10-04 0:19 UTC (permalink / raw)
To: linux-xfs; +Cc: john.g.garry
From: Dave Chinner <dchinner@redhat.com>
Now that contiguous free space selection takes into account stripe
alignment, we no longer need to do an "all AGs" allocation scan in
the case the initial AG doesn't have enough contiguous free space
for a stripe aligned allocation. This cleans up
xfs_bmap_btalloc_aligned() the same for both filestreams and the
normal btree allocation code.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 40 +++++++++++++---------------------------
1 file changed, 13 insertions(+), 27 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 3c250c89f42e..c1e2c0707e20 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3538,10 +3538,8 @@ xfs_bmap_btalloc_aligned(
struct xfs_bmalloca *ap,
struct xfs_alloc_arg *args,
xfs_extlen_t blen,
- int stripe_align,
- bool ag_only)
+ int stripe_align)
{
- struct xfs_perag *caller_pag = args->pag;
int error;
/*
@@ -3558,14 +3556,7 @@ xfs_bmap_btalloc_aligned(
args->alignment = stripe_align;
args->minalignslop = 0;
- if (ag_only) {
- error = xfs_alloc_vextent_near_bno(args, ap->blkno);
- } else {
- args->pag = NULL;
- error = xfs_alloc_vextent_start_ag(args, ap->blkno);
- ASSERT(args->pag == NULL);
- args->pag = caller_pag;
- }
+ error = xfs_alloc_vextent_near_bno(args, ap->blkno);
if (error)
return error;
@@ -3650,8 +3641,7 @@ xfs_bmap_btalloc_filestreams(
goto out_low_space;
if (ap->aeof)
- error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align,
- true);
+ error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align);
if (!error && args->fsbno == NULLFSBLOCK)
error = xfs_alloc_vextent_near_bno(args, ap->blkno);
@@ -3715,9 +3705,16 @@ xfs_bmap_btalloc_best_length(
return error;
ASSERT(args->pag);
- if (ap->aeof && ap->offset) {
+ if (ap->aeof && ap->offset)
error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align);
- }
+
+ if (error || args->fsbno != NULLFSBLOCK)
+ goto out_perag_rele;
+
+
+ /* attempt aligned allocation for new EOF extents */
+ if (stripe_align)
+ error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align);
/*
* We are now done with the perag reference for the optimal allocation
@@ -3725,24 +3722,13 @@ xfs_bmap_btalloc_best_length(
* now as we've either succeeded, had a fatal error or we are out of
* space and need to do a full filesystem scan for free space which will
* take it's own references.
- *
- * XXX: now that xfs_bmap_btalloc_select_lengths() selects an AG with
- * enough contiguous free space in it for an aligned allocation, we
- * can change the aligned allocation at EOF to just be a single AG
- * allocation.
*/
+out_perag_rele:
xfs_perag_rele(args->pag);
args->pag = NULL;
if (error || args->fsbno != NULLFSBLOCK)
return error;
- /* attempt aligned allocation for new EOF extents */
- if (stripe_align)
- error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align,
- false);
- if (error || args->fsbno != NULLFSBLOCK)
- return error;
-
/* attempt unaligned allocation */
error = xfs_alloc_vextent_start_ag(args, ap->blkno);
if (error || args->fsbno != NULLFSBLOCK)
--
2.40.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 6/9] xfs: use agno/agbno in xfs_alloc_vextent functions
2023-10-04 0:19 [RFC PATCH 0/9] xfs: push perags further into allocation routines Dave Chinner
` (4 preceding siblings ...)
2023-10-04 0:19 ` [PATCH 5/9] xfs: aligned EOF allocations don't need to scan AGs anymore Dave Chinner
@ 2023-10-04 0:19 ` Dave Chinner
2023-10-05 12:02 ` Christoph Hellwig
2023-10-06 5:56 ` Darrick J. Wong
2023-10-04 0:19 ` [PATCH 7/9] xfs: caller perag always supplied to xfs_alloc_vextent_near_bno Dave Chinner
` (2 subsequent siblings)
8 siblings, 2 replies; 27+ messages in thread
From: Dave Chinner @ 2023-10-04 0:19 UTC (permalink / raw)
To: linux-xfs; +Cc: john.g.garry
From: Dave Chinner <dchinner@redhat.com>
We're moving away from using xfs_fsblock_t to define allocation
targets, and instead we are using {agno,agbno} tuples instead. This
will allow us to eventually move everything to {perag,agbno}. But
before we get there, we need to split the fsblock into {agno,agbno}
and convert the code to use those first.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_alloc.c | 57 +++++++++++++++++++++------------------
1 file changed, 31 insertions(+), 26 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 3069194527dd..289b54911b05 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -3249,7 +3249,8 @@ xfs_alloc_read_agf(
static int
xfs_alloc_vextent_check_args(
struct xfs_alloc_arg *args,
- xfs_fsblock_t target,
+ xfs_agnumber_t target_agno,
+ xfs_agblock_t target_agbno,
xfs_agnumber_t *minimum_agno)
{
struct xfs_mount *mp = args->mp;
@@ -3277,14 +3278,14 @@ xfs_alloc_vextent_check_args(
ASSERT(args->alignment > 0);
ASSERT(args->resv != XFS_AG_RESV_AGFL);
- ASSERT(XFS_FSB_TO_AGNO(mp, target) < mp->m_sb.sb_agcount);
- ASSERT(XFS_FSB_TO_AGBNO(mp, target) < agsize);
+ ASSERT(target_agno < mp->m_sb.sb_agcount);
+ ASSERT(target_agbno < agsize);
ASSERT(args->minlen <= args->maxlen);
ASSERT(args->minlen <= agsize);
ASSERT(args->mod < args->prod);
- if (XFS_FSB_TO_AGNO(mp, target) >= mp->m_sb.sb_agcount ||
- XFS_FSB_TO_AGBNO(mp, target) >= agsize ||
+ if (target_agno >= mp->m_sb.sb_agcount ||
+ target_agbno >= agsize ||
args->minlen > args->maxlen || args->minlen > agsize ||
args->mod >= args->prod) {
trace_xfs_alloc_vextent_badargs(args);
@@ -3438,7 +3439,6 @@ xfs_alloc_vextent_this_ag(
struct xfs_alloc_arg *args,
xfs_agnumber_t agno)
{
- struct xfs_mount *mp = args->mp;
xfs_agnumber_t minimum_agno;
uint32_t alloc_flags = 0;
int error;
@@ -3451,8 +3451,7 @@ xfs_alloc_vextent_this_ag(
trace_xfs_alloc_vextent_this_ag(args);
- error = xfs_alloc_vextent_check_args(args, XFS_AGB_TO_FSB(mp, agno, 0),
- &minimum_agno);
+ error = xfs_alloc_vextent_check_args(args, agno, 0, &minimum_agno);
if (error) {
if (error == -ENOSPC)
return 0;
@@ -3563,7 +3562,8 @@ xfs_alloc_vextent_start_ag(
{
struct xfs_mount *mp = args->mp;
xfs_agnumber_t minimum_agno;
- xfs_agnumber_t start_agno;
+ xfs_agnumber_t target_agno;
+ xfs_agblock_t target_agbno;
xfs_agnumber_t rotorstep = xfs_rotorstep;
bool bump_rotor = false;
uint32_t alloc_flags = XFS_ALLOC_FLAG_TRYLOCK;
@@ -3576,7 +3576,10 @@ xfs_alloc_vextent_start_ag(
trace_xfs_alloc_vextent_start_ag(args);
- error = xfs_alloc_vextent_check_args(args, target, &minimum_agno);
+ target_agno = XFS_FSB_TO_AGNO(mp, target);
+ target_agbno = XFS_FSB_TO_AGBNO(mp, target);
+ error = xfs_alloc_vextent_check_args(args, target_agno, target_agbno,
+ &minimum_agno);
if (error) {
if (error == -ENOSPC)
return 0;
@@ -3591,12 +3594,11 @@ xfs_alloc_vextent_start_ag(
bump_rotor = 1;
}
- start_agno = max(minimum_agno, XFS_FSB_TO_AGNO(mp, target));
- error = xfs_alloc_vextent_iterate_ags(args, minimum_agno, start_agno,
- XFS_FSB_TO_AGBNO(mp, target), alloc_flags);
-
+ target_agno = max(minimum_agno, target_agno);
+ error = xfs_alloc_vextent_iterate_ags(args, minimum_agno, target_agno,
+ target_agbno, alloc_flags);
if (bump_rotor) {
- if (args->agno == start_agno)
+ if (args->agno == target_agno)
mp->m_agfrotor = (mp->m_agfrotor + 1) %
(mp->m_sb.sb_agcount * rotorstep);
else
@@ -3619,7 +3621,8 @@ xfs_alloc_vextent_first_ag(
{
struct xfs_mount *mp = args->mp;
xfs_agnumber_t minimum_agno;
- xfs_agnumber_t start_agno;
+ xfs_agnumber_t target_agno;
+ xfs_agblock_t target_agbno;
uint32_t alloc_flags = XFS_ALLOC_FLAG_TRYLOCK;
int error;
@@ -3630,16 +3633,19 @@ xfs_alloc_vextent_first_ag(
trace_xfs_alloc_vextent_first_ag(args);
- error = xfs_alloc_vextent_check_args(args, target, &minimum_agno);
+ target_agno = XFS_FSB_TO_AGNO(mp, target);
+ target_agbno = XFS_FSB_TO_AGBNO(mp, target);
+ error = xfs_alloc_vextent_check_args(args, target_agno, target_agbno,
+ &minimum_agno);
if (error) {
if (error == -ENOSPC)
return 0;
return error;
}
- start_agno = max(minimum_agno, XFS_FSB_TO_AGNO(mp, target));
- error = xfs_alloc_vextent_iterate_ags(args, minimum_agno, start_agno,
- XFS_FSB_TO_AGBNO(mp, target), alloc_flags);
+ target_agno = max(minimum_agno, target_agno);
+ error = xfs_alloc_vextent_iterate_ags(args, minimum_agno, target_agno,
+ target_agbno, alloc_flags);
return xfs_alloc_vextent_finish(args, minimum_agno, error, true);
}
@@ -3656,15 +3662,13 @@ xfs_alloc_vextent_exact_bno(
xfs_agnumber_t minimum_agno;
int error;
- ASSERT(args->pag != NULL);
- ASSERT(args->pag->pag_agno == XFS_FSB_TO_AGNO(mp, target));
-
- args->agno = XFS_FSB_TO_AGNO(mp, target);
+ args->agno = args->pag->pag_agno;
args->agbno = XFS_FSB_TO_AGBNO(mp, target);
trace_xfs_alloc_vextent_exact_bno(args);
- error = xfs_alloc_vextent_check_args(args, target, &minimum_agno);
+ error = xfs_alloc_vextent_check_args(args, args->agno, args->agbno,
+ &minimum_agno);
if (error) {
if (error == -ENOSPC)
return 0;
@@ -3703,7 +3707,8 @@ xfs_alloc_vextent_near_bno(
trace_xfs_alloc_vextent_near_bno(args);
- error = xfs_alloc_vextent_check_args(args, target, &minimum_agno);
+ error = xfs_alloc_vextent_check_args(args, args->agno, args->agbno,
+ &minimum_agno);
if (error) {
if (error == -ENOSPC)
return 0;
--
2.40.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 7/9] xfs: caller perag always supplied to xfs_alloc_vextent_near_bno
2023-10-04 0:19 [RFC PATCH 0/9] xfs: push perags further into allocation routines Dave Chinner
` (5 preceding siblings ...)
2023-10-04 0:19 ` [PATCH 6/9] xfs: use agno/agbno in xfs_alloc_vextent functions Dave Chinner
@ 2023-10-04 0:19 ` Dave Chinner
2023-10-05 12:02 ` Christoph Hellwig
2023-10-06 5:59 ` Darrick J. Wong
2023-10-04 0:19 ` [PATCH 8/9] xfs: collapse near and exact bno allocation Dave Chinner
2023-10-04 0:19 ` [PATCH 9/9] xfs: return -ENOSPC rather than NULLFSBLOCK from allocation functions Dave Chinner
8 siblings, 2 replies; 27+ messages in thread
From: Dave Chinner @ 2023-10-04 0:19 UTC (permalink / raw)
To: linux-xfs; +Cc: john.g.garry
From: Dave Chinner <dchinner@redhat.com>
So we don't need to conditionally grab the perag anymore.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_alloc.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 289b54911b05..3190c8204903 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -3686,7 +3686,7 @@ xfs_alloc_vextent_exact_bno(
* Allocate an extent as close to the target as possible. If there are not
* viable candidates in the AG, then fail the allocation.
*
- * Caller may or may not have a per-ag reference in args->pag.
+ * Caller is expected to hold a per-ag reference in args->pag.
*/
int
xfs_alloc_vextent_near_bno(
@@ -3695,14 +3695,12 @@ xfs_alloc_vextent_near_bno(
{
struct xfs_mount *mp = args->mp;
xfs_agnumber_t minimum_agno;
- bool needs_perag = args->pag == NULL;
uint32_t alloc_flags = 0;
int error;
- if (!needs_perag)
- ASSERT(args->pag->pag_agno == XFS_FSB_TO_AGNO(mp, target));
+ ASSERT(args->pag->pag_agno == XFS_FSB_TO_AGNO(mp, target));
- args->agno = XFS_FSB_TO_AGNO(mp, target);
+ args->agno = args->pag->pag_agno;
args->agbno = XFS_FSB_TO_AGBNO(mp, target);
trace_xfs_alloc_vextent_near_bno(args);
@@ -3715,14 +3713,11 @@ xfs_alloc_vextent_near_bno(
return error;
}
- if (needs_perag)
- args->pag = xfs_perag_grab(mp, args->agno);
-
error = xfs_alloc_vextent_prepare_ag(args, alloc_flags);
if (!error && args->agbp)
error = xfs_alloc_ag_vextent_near(args, alloc_flags);
- return xfs_alloc_vextent_finish(args, minimum_agno, error, needs_perag);
+ return xfs_alloc_vextent_finish(args, minimum_agno, error, false);
}
/* Ensure that the freelist is at full capacity. */
--
2.40.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 8/9] xfs: collapse near and exact bno allocation
2023-10-04 0:19 [RFC PATCH 0/9] xfs: push perags further into allocation routines Dave Chinner
` (6 preceding siblings ...)
2023-10-04 0:19 ` [PATCH 7/9] xfs: caller perag always supplied to xfs_alloc_vextent_near_bno Dave Chinner
@ 2023-10-04 0:19 ` Dave Chinner
2023-10-05 12:03 ` Christoph Hellwig
2023-10-06 6:01 ` Darrick J. Wong
2023-10-04 0:19 ` [PATCH 9/9] xfs: return -ENOSPC rather than NULLFSBLOCK from allocation functions Dave Chinner
8 siblings, 2 replies; 27+ messages in thread
From: Dave Chinner @ 2023-10-04 0:19 UTC (permalink / raw)
To: linux-xfs; +Cc: john.g.garry
From: Dave Chinner <dchinner@redhat.com>
As they are now identical functions exact for the allocation
function they call.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_alloc.c | 84 ++++++++++++++++++---------------------
fs/xfs/xfs_trace.h | 1 +
2 files changed, 40 insertions(+), 45 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 3190c8204903..27c62f303488 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -3649,6 +3649,43 @@ xfs_alloc_vextent_first_ag(
return xfs_alloc_vextent_finish(args, minimum_agno, error, true);
}
+static int
+xfs_alloc_vextent_bno(
+ struct xfs_alloc_arg *args,
+ xfs_fsblock_t target,
+ bool exact)
+{
+ struct xfs_mount *mp = args->mp;
+ xfs_agnumber_t minimum_agno;
+ int error;
+
+ ASSERT(args->pag != NULL);
+ ASSERT(args->pag->pag_agno == XFS_FSB_TO_AGNO(mp, target));
+
+ args->agno = args->pag->pag_agno;
+ args->agbno = XFS_FSB_TO_AGBNO(mp, target);
+
+ trace_xfs_alloc_vextent_bno(args);
+
+ error = xfs_alloc_vextent_check_args(args, args->agno, args->agbno,
+ &minimum_agno);
+ if (error) {
+ if (error == -ENOSPC)
+ return 0;
+ return error;
+ }
+
+ error = xfs_alloc_vextent_prepare_ag(args, 0);
+ if (!error && args->agbp) {
+ if (exact)
+ error = xfs_alloc_ag_vextent_exact(args);
+ else
+ error = xfs_alloc_ag_vextent_near(args, 0);
+ }
+
+ return xfs_alloc_vextent_finish(args, minimum_agno, error, false);
+}
+
/*
* Allocate at the exact block target or fail. Caller is expected to hold a
* perag reference in args->pag.
@@ -3658,28 +3695,8 @@ xfs_alloc_vextent_exact_bno(
struct xfs_alloc_arg *args,
xfs_fsblock_t target)
{
- struct xfs_mount *mp = args->mp;
- xfs_agnumber_t minimum_agno;
- int error;
-
- args->agno = args->pag->pag_agno;
- args->agbno = XFS_FSB_TO_AGBNO(mp, target);
-
trace_xfs_alloc_vextent_exact_bno(args);
-
- error = xfs_alloc_vextent_check_args(args, args->agno, args->agbno,
- &minimum_agno);
- if (error) {
- if (error == -ENOSPC)
- return 0;
- return error;
- }
-
- error = xfs_alloc_vextent_prepare_ag(args, 0);
- if (!error && args->agbp)
- error = xfs_alloc_ag_vextent_exact(args);
-
- return xfs_alloc_vextent_finish(args, minimum_agno, error, false);
+ return xfs_alloc_vextent_bno(args, target, true);
}
/*
@@ -3693,31 +3710,8 @@ xfs_alloc_vextent_near_bno(
struct xfs_alloc_arg *args,
xfs_fsblock_t target)
{
- struct xfs_mount *mp = args->mp;
- xfs_agnumber_t minimum_agno;
- uint32_t alloc_flags = 0;
- int error;
-
- ASSERT(args->pag->pag_agno == XFS_FSB_TO_AGNO(mp, target));
-
- args->agno = args->pag->pag_agno;
- args->agbno = XFS_FSB_TO_AGBNO(mp, target);
-
trace_xfs_alloc_vextent_near_bno(args);
-
- error = xfs_alloc_vextent_check_args(args, args->agno, args->agbno,
- &minimum_agno);
- if (error) {
- if (error == -ENOSPC)
- return 0;
- return error;
- }
-
- error = xfs_alloc_vextent_prepare_ag(args, alloc_flags);
- if (!error && args->agbp)
- error = xfs_alloc_ag_vextent_near(args, alloc_flags);
-
- return xfs_alloc_vextent_finish(args, minimum_agno, error, false);
+ return xfs_alloc_vextent_bno(args, target, false);
}
/* Ensure that the freelist is at full capacity. */
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 3926cf7f2a6e..628da36b20b9 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -1884,6 +1884,7 @@ DEFINE_ALLOC_EVENT(xfs_alloc_vextent_start_ag);
DEFINE_ALLOC_EVENT(xfs_alloc_vextent_first_ag);
DEFINE_ALLOC_EVENT(xfs_alloc_vextent_exact_bno);
DEFINE_ALLOC_EVENT(xfs_alloc_vextent_near_bno);
+DEFINE_ALLOC_EVENT(xfs_alloc_vextent_bno);
DEFINE_ALLOC_EVENT(xfs_alloc_vextent_finish);
TRACE_EVENT(xfs_alloc_cur_check,
--
2.40.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 9/9] xfs: return -ENOSPC rather than NULLFSBLOCK from allocation functions
2023-10-04 0:19 [RFC PATCH 0/9] xfs: push perags further into allocation routines Dave Chinner
` (7 preceding siblings ...)
2023-10-04 0:19 ` [PATCH 8/9] xfs: collapse near and exact bno allocation Dave Chinner
@ 2023-10-04 0:19 ` Dave Chinner
2023-10-05 12:21 ` Christoph Hellwig
8 siblings, 1 reply; 27+ messages in thread
From: Dave Chinner @ 2023-10-04 0:19 UTC (permalink / raw)
To: linux-xfs; +Cc: john.g.garry
From: Dave Chinner <dchinner@redhat.com>
The core allocation routines may fail due to an ENOSPC condition.
However, they do not report that via an ENOSPC error, they report it
via a "allocation succeeded" return value but with a allocated block
of NULLFSBLOCK.
This behaviour recently lead to a data corruption bug where failure
to allocate a block was not caught correctly - the failure was
treated as a success and an uninitialised fsblock was used for a
data write instead. This would have been avoided if we returned
ENOSPC for ENOSPC conditions, but we don't so bad things happened.
Make sure we don't have a repeat of this situation by changing the
API to explicitly return ENOSPC when we fail to allocate. If we fail
to capture this correctly, it will lead to failures being noticed
either by ENOSPC escaping to userspace or by causing filesystem
shutdowns when allocations failure where they really shouldn't.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_alloc.c | 73 +++++++------------
fs/xfs/libxfs/xfs_bmap.c | 112 +++++++++++------------------
fs/xfs/libxfs/xfs_bmap_btree.c | 19 +++--
fs/xfs/libxfs/xfs_ialloc.c | 26 ++++---
fs/xfs/libxfs/xfs_ialloc_btree.c | 8 +--
fs/xfs/libxfs/xfs_refcount_btree.c | 8 +--
6 files changed, 98 insertions(+), 148 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 27c62f303488..13fda27fabcb 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -1157,9 +1157,9 @@ xfs_alloc_ag_vextent_small(
* Can't do the allocation, give up.
*/
if (flen < args->minlen) {
- args->agbno = NULLAGBLOCK;
trace_xfs_alloc_small_notenough(args);
- flen = 0;
+ error = -ENOSPC;
+ goto error;
}
*fbnop = fbno;
*flenp = flen;
@@ -1279,9 +1279,8 @@ xfs_alloc_ag_vextent_exact(
not_found:
/* Didn't find it, return null. */
xfs_btree_del_cursor(bno_cur, XFS_BTREE_NOERROR);
- args->agbno = NULLAGBLOCK;
trace_xfs_alloc_exact_notfound(args);
- return 0;
+ return -ENOSPC;
error0:
xfs_btree_del_cursor(bno_cur, XFS_BTREE_ERROR);
@@ -1630,7 +1629,7 @@ xfs_alloc_ag_vextent_near(
goto restart;
}
trace_xfs_alloc_size_neither(args);
- args->agbno = NULLAGBLOCK;
+ error = -ENOSPC;
goto out;
}
@@ -1882,8 +1881,7 @@ xfs_alloc_ag_vextent_size(
out_nominleft:
xfs_btree_del_cursor(cnt_cur, XFS_BTREE_NOERROR);
trace_xfs_alloc_size_nominleft(args);
- args->agbno = NULLAGBLOCK;
- return 0;
+ return -ENOSPC;
}
/*
@@ -2742,16 +2740,15 @@ xfs_alloc_fix_freelist(
/* Allocate as many blocks as possible at once. */
error = xfs_alloc_ag_vextent_size(&targs, alloc_flags);
- if (error)
- goto out_agflbp_relse;
- /*
- * Stop if we run out. Won't happen if callers are obeying
- * the restrictions correctly. Can happen for free calls
- * on a completely full ag.
- */
- if (targs.agbno == NULLAGBLOCK) {
- if (alloc_flags & XFS_ALLOC_FLAG_FREEING)
+ if (error) {
+ /*
+ * Stop if we run out. Won't happen if callers are
+ * obeying the restrictions correctly. Can happen for
+ * free calls on a completely full ag.
+ */
+ if (error == -ENOSPC &&
+ (alloc_flags & XFS_ALLOC_FLAG_FREEING))
break;
goto out_agflbp_relse;
}
@@ -3324,14 +3321,12 @@ xfs_alloc_vextent_prepare_ag(
trace_xfs_alloc_vextent_nofix(args);
if (need_pag)
xfs_perag_put(args->pag);
- args->agbno = NULLAGBLOCK;
return error;
}
if (!args->agbp) {
/* cannot allocate in this AG at all */
trace_xfs_alloc_vextent_noagbp(args);
- args->agbno = NULLAGBLOCK;
- return 0;
+ return -ENOSPC;
}
args->wasfromfl = 0;
return 0;
@@ -3375,14 +3370,7 @@ xfs_alloc_vextent_finish(
args->agno > minimum_agno))
args->tp->t_highest_agno = args->agno;
- /*
- * If the allocation failed with an error or we had an ENOSPC result,
- * preserve the returned error whilst also marking the allocation result
- * as "no extent allocated". This ensures that callers that fail to
- * capture the error will still treat it as a failed allocation.
- */
- if (alloc_error || args->agbno == NULLAGBLOCK) {
- args->fsbno = NULLFSBLOCK;
+ if (alloc_error) {
error = alloc_error;
goto out_drop_perag;
}
@@ -3452,11 +3440,8 @@ xfs_alloc_vextent_this_ag(
trace_xfs_alloc_vextent_this_ag(args);
error = xfs_alloc_vextent_check_args(args, agno, 0, &minimum_agno);
- if (error) {
- if (error == -ENOSPC)
- return 0;
+ if (error)
return error;
- }
error = xfs_alloc_vextent_prepare_ag(args, alloc_flags);
if (!error && args->agbp)
@@ -3503,7 +3488,7 @@ xfs_alloc_vextent_iterate_ags(
mp->m_sb.sb_agcount, agno, args->pag) {
args->agno = agno;
error = xfs_alloc_vextent_prepare_ag(args, alloc_flags);
- if (error)
+ if (error && error != -ENOSPC)
break;
if (!args->agbp) {
trace_xfs_alloc_vextent_loopfailed(args);
@@ -3523,13 +3508,13 @@ xfs_alloc_vextent_iterate_ags(
}
break;
}
- if (error) {
+ if (error && error != -ENOSPC) {
xfs_perag_rele(args->pag);
args->pag = NULL;
return error;
}
if (args->agbp)
- return 0;
+ return error;
/*
* We didn't find an AG we can alloation from. If we were given
@@ -3544,7 +3529,7 @@ xfs_alloc_vextent_iterate_ags(
ASSERT(args->pag == NULL);
trace_xfs_alloc_vextent_allfailed(args);
- return 0;
+ return -ENOSPC;
}
/*
@@ -3580,11 +3565,8 @@ xfs_alloc_vextent_start_ag(
target_agbno = XFS_FSB_TO_AGBNO(mp, target);
error = xfs_alloc_vextent_check_args(args, target_agno, target_agbno,
&minimum_agno);
- if (error) {
- if (error == -ENOSPC)
- return 0;
+ if (error)
return error;
- }
if ((args->datatype & XFS_ALLOC_INITIAL_USER_DATA) &&
xfs_is_inode32(mp)) {
@@ -3637,11 +3619,8 @@ xfs_alloc_vextent_first_ag(
target_agbno = XFS_FSB_TO_AGBNO(mp, target);
error = xfs_alloc_vextent_check_args(args, target_agno, target_agbno,
&minimum_agno);
- if (error) {
- if (error == -ENOSPC)
- return 0;
+ if (error)
return error;
- }
target_agno = max(minimum_agno, target_agno);
error = xfs_alloc_vextent_iterate_ags(args, minimum_agno, target_agno,
@@ -3669,11 +3648,8 @@ xfs_alloc_vextent_bno(
error = xfs_alloc_vextent_check_args(args, args->agno, args->agbno,
&minimum_agno);
- if (error) {
- if (error == -ENOSPC)
- return 0;
+ if (error)
return error;
- }
error = xfs_alloc_vextent_prepare_ag(args, 0);
if (!error && args->agbp) {
@@ -3688,7 +3664,8 @@ xfs_alloc_vextent_bno(
/*
* Allocate at the exact block target or fail. Caller is expected to hold a
- * perag reference in args->pag.
+ * perag reference in args->pag. If the exact block required cannot be
+ * allocated, this will return -ENOSPC.
*/
int
xfs_alloc_vextent_exact_bno(
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index c1e2c0707e20..00cebf9eb682 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -659,14 +659,6 @@ xfs_bmap_extents_to_btree(
if (error)
goto out_root_realloc;
- /*
- * Allocation can't fail, the space was reserved.
- */
- if (WARN_ON_ONCE(args.fsbno == NULLFSBLOCK)) {
- error = -ENOSPC;
- goto out_root_realloc;
- }
-
cur->bc_ino.allocated++;
ip->i_nblocks++;
xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_BCOUNT, 1L);
@@ -724,6 +716,8 @@ xfs_bmap_extents_to_btree(
ASSERT(ifp->if_broot == NULL);
xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
+ /* Allocation shouldn't fail with -ENOSPC because space was reserved. */
+ WARN_ON_ONCE(error == -ENOSPC);
return error;
}
@@ -808,8 +802,6 @@ xfs_bmap_local_to_extents(
if (error)
goto done;
- /* Can't fail, the space was reserved. */
- ASSERT(args.fsbno != NULLFSBLOCK);
ASSERT(args.len == 1);
error = xfs_trans_get_buf(tp, args.mp->m_ddev_targp,
XFS_FSB_TO_DADDR(args.mp, args.fsbno),
@@ -849,6 +841,9 @@ xfs_bmap_local_to_extents(
done:
*logflagsp = flags;
+
+ /* Allocation shouldn't fail with -ENOSPC because space was reserved. */
+ ASSERT(error != -ENOSPC);
return error;
}
@@ -3435,11 +3430,8 @@ xfs_bmap_exact_minlen_extent_alloc(
ASSERT(ap->length);
- if (ap->minlen != 1) {
- ap->blkno = NULLFSBLOCK;
- ap->length = 0;
- return 0;
- }
+ if (ap->minlen != 1)
+ return -ENOSPC;
orig_offset = ap->offset;
orig_length = ap->length;
@@ -3474,14 +3466,7 @@ xfs_bmap_exact_minlen_extent_alloc(
if (error)
return error;
- if (args.fsbno != NULLFSBLOCK) {
- xfs_bmap_process_allocated_extent(ap, &args, orig_offset,
- orig_length);
- } else {
- ap->blkno = NULLFSBLOCK;
- ap->length = 0;
- }
-
+ xfs_bmap_process_allocated_extent(ap, &args, orig_offset, orig_length);
return 0;
}
#else
@@ -3520,17 +3505,14 @@ xfs_bmap_exact_minlen_extent_alloc(
args->minalignslop = 0;
error = xfs_alloc_vextent_exact_bno(args, ap->blkno);
- if (error)
- return error;
-
- if (args->fsbno != NULLFSBLOCK)
- return 0;
- /*
- * Exact allocation failed. Reset to try an aligned allocation
- * according to the original allocation specification.
- */
- args->minlen = nextminlen;
- return 0;
+ if (error == -ENOSPC) {
+ /*
+ * Exact allocation failed. Reset to try an aligned allocation
+ * according to the original allocation specification.
+ */
+ args->minlen = nextminlen;
+ }
+ return error;
}
static int
@@ -3557,19 +3539,15 @@ xfs_bmap_btalloc_aligned(
args->minalignslop = 0;
error = xfs_alloc_vextent_near_bno(args, ap->blkno);
- if (error)
- return error;
-
- if (args->fsbno != NULLFSBLOCK)
- return 0;
-
- /*
- * Allocation failed, so turn return the allocation args to their
- * original non-aligned state so the caller can proceed on allocation
- * failure as if this function was never called.
- */
- args->alignment = 1;
- return 0;
+ if (error == -ENOSPC) {
+ /*
+ * Allocation failed, so turn return the allocation args to
+ * their original non-aligned state so the caller can proceed on
+ * allocation failure as if this function was never called.
+ */
+ args->alignment = 1;
+ }
+ return error;
}
/*
@@ -3594,17 +3572,15 @@ xfs_bmap_btalloc_low_space(
if (args->minlen > ap->minlen) {
args->minlen = ap->minlen;
error = xfs_alloc_vextent_start_ag(args, ap->blkno);
- if (error || args->fsbno != NULLFSBLOCK)
+ if (error != -ENOSPC)
return error;
}
/* Last ditch attempt before failure is declared. */
args->total = ap->minlen;
error = xfs_alloc_vextent_first_ag(args, 0);
- if (error)
- return error;
ap->tp->t_flags |= XFS_TRANS_LOWMODE;
- return 0;
+ return error;
}
static int
@@ -3633,17 +3609,18 @@ xfs_bmap_btalloc_filestreams(
goto out_low_space;
}
+ error = -ENOSPC;
args->minlen = xfs_bmap_select_minlen(ap, args, blen);
if (ap->aeof && ap->offset)
error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align);
- if (error || args->fsbno != NULLFSBLOCK)
+ if (error != -ENOSPC)
goto out_low_space;
if (ap->aeof)
error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align);
- if (!error && args->fsbno == NULLFSBLOCK)
+ if (error == -ENOSPC)
error = xfs_alloc_vextent_near_bno(args, ap->blkno);
out_low_space:
@@ -3656,7 +3633,7 @@ xfs_bmap_btalloc_filestreams(
*/
xfs_perag_rele(args->pag);
args->pag = NULL;
- if (error || args->fsbno != NULLFSBLOCK)
+ if (error != -ENOSPC)
return error;
return xfs_bmap_btalloc_low_space(ap, args);
@@ -3705,10 +3682,11 @@ xfs_bmap_btalloc_best_length(
return error;
ASSERT(args->pag);
+ error = -ENOSPC;
if (ap->aeof && ap->offset)
error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align);
- if (error || args->fsbno != NULLFSBLOCK)
+ if (error != -ENOSPC)
goto out_perag_rele;
@@ -3726,12 +3704,12 @@ xfs_bmap_btalloc_best_length(
out_perag_rele:
xfs_perag_rele(args->pag);
args->pag = NULL;
- if (error || args->fsbno != NULLFSBLOCK)
+ if (error != -ENOSPC)
return error;
/* attempt unaligned allocation */
error = xfs_alloc_vextent_start_ag(args, ap->blkno);
- if (error || args->fsbno != NULLFSBLOCK)
+ if (error != -ENOSPC)
return error;
return xfs_bmap_btalloc_low_space(ap, args);
@@ -3773,17 +3751,10 @@ xfs_bmap_btalloc(
error = xfs_bmap_btalloc_filestreams(ap, &args, stripe_align);
else
error = xfs_bmap_btalloc_best_length(ap, &args, stripe_align);
- if (error)
- return error;
-
- if (args.fsbno != NULLFSBLOCK) {
+ if (!error)
xfs_bmap_process_allocated_extent(ap, &args, orig_offset,
orig_length);
- } else {
- ap->blkno = NULLFSBLOCK;
- ap->length = 0;
- }
- return 0;
+ return error;
}
/* Trim extent to fit a logical block range. */
@@ -4189,7 +4160,7 @@ xfs_bmapi_allocate(
} else {
error = xfs_bmap_alloc_userdata(bma);
}
- if (error || bma->blkno == NULLFSBLOCK)
+ if (error)
return error;
if (bma->flags & XFS_BMAPI_ZERO) {
@@ -4497,10 +4468,10 @@ xfs_bmapi_write(
ASSERT(len > 0);
ASSERT(bma.length > 0);
error = xfs_bmapi_allocate(&bma);
+ if (error == -ENOSPC)
+ break;
if (error)
goto error0;
- if (bma.blkno == NULLFSBLOCK)
- break;
/*
* If this is a CoW allocation, record the data in
@@ -4656,9 +4627,6 @@ xfs_bmapi_convert_delalloc(
if (error)
goto out_finish;
- error = -ENOSPC;
- if (WARN_ON_ONCE(bma.blkno == NULLFSBLOCK))
- goto out_finish;
error = -EFSCORRUPTED;
if (WARN_ON_ONCE(!xfs_valid_startblock(ip, bma.got.br_startblock)))
goto out_finish;
diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
index bf3f1b36fdd2..c1f6d0a7d960 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.c
+++ b/fs/xfs/libxfs/xfs_bmap_btree.c
@@ -225,25 +225,24 @@ xfs_bmbt_alloc_block(
cur->bc_ino.whichfork);
error = xfs_alloc_vextent_start_ag(&args, be64_to_cpu(start->l));
- if (error)
- return error;
-
- if (args.fsbno == NULLFSBLOCK && args.minleft) {
+ if (error == -ENOSPC && args.minleft) {
/*
- * Could not find an AG with enough free space to satisfy
- * a full btree split. Try again and if
- * successful activate the lowspace algorithm.
+ * Could not find an AG with enough free space to satisfy a full
+ * btree split. Try again and then activate the lowspace
+ * algorithm.
*/
args.minleft = 0;
error = xfs_alloc_vextent_start_ag(&args, 0);
- if (error)
- return error;
cur->bc_tp->t_flags |= XFS_TRANS_LOWMODE;
}
- if (WARN_ON_ONCE(args.fsbno == NULLFSBLOCK)) {
+
+ /* This allocation really should not fail. */
+ if (WARN_ON_ONCE(error == -ENOSPC)) {
*stat = 0;
return 0;
}
+ if (error)
+ return error;
ASSERT(args.len == 1);
cur->bc_ino.allocated++;
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index b83e54c70906..96ccf01a3a74 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -686,6 +686,7 @@ xfs_ialloc_ag_alloc(
igeo->ialloc_blks;
if (do_sparse)
goto sparse_alloc;
+ error = -ENOSPC;
if (likely(newino != NULLAGINO &&
(args.agbno < be32_to_cpu(agi->agi_length)))) {
args.prod = 1;
@@ -711,7 +712,7 @@ xfs_ialloc_ag_alloc(
error = xfs_alloc_vextent_exact_bno(&args,
XFS_AGB_TO_FSB(args.mp, pag->pag_agno,
args.agbno));
- if (error)
+ if (error && error != -ENOSPC)
return error;
/*
@@ -727,7 +728,7 @@ xfs_ialloc_ag_alloc(
args.minalignslop = 0;
}
- if (unlikely(args.fsbno == NULLFSBLOCK)) {
+ if (error == -ENOSPC) {
/*
* Set the alignment for the allocation.
* If stripe alignment is turned on then align at stripe unit
@@ -754,7 +755,7 @@ xfs_ialloc_ag_alloc(
error = xfs_alloc_vextent_near_bno(&args,
XFS_AGB_TO_FSB(args.mp, pag->pag_agno,
be32_to_cpu(agi->agi_root)));
- if (error)
+ if (error && error != -ENOSPC)
return error;
}
@@ -762,12 +763,12 @@ xfs_ialloc_ag_alloc(
* If stripe alignment is turned on, then try again with cluster
* alignment.
*/
- if (isaligned && args.fsbno == NULLFSBLOCK) {
+ if (error == -ENOSPC && isaligned) {
args.alignment = igeo->cluster_align;
error = xfs_alloc_vextent_near_bno(&args,
XFS_AGB_TO_FSB(args.mp, pag->pag_agno,
be32_to_cpu(agi->agi_root)));
- if (error)
+ if (error && error != -ENOSPC)
return error;
}
@@ -775,9 +776,9 @@ xfs_ialloc_ag_alloc(
* Finally, try a sparse allocation if the filesystem supports it and
* the sparse allocation length is smaller than a full chunk.
*/
- if (xfs_has_sparseinodes(args.mp) &&
- igeo->ialloc_min_blks < igeo->ialloc_blks &&
- args.fsbno == NULLFSBLOCK) {
+ if (error == -ENOSPC &&
+ xfs_has_sparseinodes(args.mp) &&
+ igeo->ialloc_min_blks < igeo->ialloc_blks) {
sparse_alloc:
args.alignment = args.mp->m_sb.sb_spino_align;
args.prod = 1;
@@ -803,7 +804,7 @@ xfs_ialloc_ag_alloc(
error = xfs_alloc_vextent_near_bno(&args,
XFS_AGB_TO_FSB(args.mp, pag->pag_agno,
be32_to_cpu(agi->agi_root)));
- if (error)
+ if (error && error != -ENOSPC)
return error;
newlen = XFS_AGB_TO_AGINO(args.mp, args.len);
@@ -811,7 +812,12 @@ xfs_ialloc_ag_alloc(
allocmask = (1 << (newlen / XFS_INODES_PER_HOLEMASK_BIT)) - 1;
}
- if (args.fsbno == NULLFSBLOCK)
+ /*
+ * There really is not available space for inode allocation in this AG,
+ * so return a -EAGAIN error to the caller to tell it to try a different
+ * AG.
+ */
+ if (error == -ENOSPC)
return -EAGAIN;
ASSERT(args.len == args.minlen);
diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
index 9258f01c0015..cbc4413042f2 100644
--- a/fs/xfs/libxfs/xfs_ialloc_btree.c
+++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
@@ -112,13 +112,13 @@ __xfs_inobt_alloc_block(
error = xfs_alloc_vextent_near_bno(&args,
XFS_AGB_TO_FSB(args.mp, args.pag->pag_agno, sbno));
- if (error)
- return error;
-
- if (args.fsbno == NULLFSBLOCK) {
+ if (error == -ENOSPC) {
*stat = 0;
return 0;
}
+ if (error)
+ return error;
+
ASSERT(args.len == 1);
new->s = cpu_to_be32(XFS_FSB_TO_AGBNO(args.mp, args.fsbno));
diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
index 5c3987d8dc24..d5fbdd4e25b6 100644
--- a/fs/xfs/libxfs/xfs_refcount_btree.c
+++ b/fs/xfs/libxfs/xfs_refcount_btree.c
@@ -75,14 +75,14 @@ xfs_refcountbt_alloc_block(
error = xfs_alloc_vextent_near_bno(&args,
XFS_AGB_TO_FSB(args.mp, args.pag->pag_agno,
xfs_refc_block(args.mp)));
+ if (error == -ENOSPC) {
+ *stat = 0;
+ return 0;
+ }
if (error)
goto out_error;
trace_xfs_refcountbt_alloc_block(cur->bc_mp, cur->bc_ag.pag->pag_agno,
args.agbno, 1);
- if (args.fsbno == NULLFSBLOCK) {
- *stat = 0;
- return 0;
- }
ASSERT(args.agno == cur->bc_ag.pag->pag_agno);
ASSERT(args.len == 1);
--
2.40.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 1/9] xfs: split xfs_bmap_btalloc_at_eof()
2023-10-04 0:19 ` [PATCH 1/9] xfs: split xfs_bmap_btalloc_at_eof() Dave Chinner
@ 2023-10-04 23:41 ` Darrick J. Wong
2023-10-05 9:41 ` Christoph Hellwig
1 sibling, 0 replies; 27+ messages in thread
From: Darrick J. Wong @ 2023-10-04 23:41 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
On Wed, Oct 04, 2023 at 11:19:35AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> This function is really implementing two policies. The first is
> trying to create contiguous extents at file extension. Failing that,
> it attempts to perform aligned allocation. These are really two
> separate policies, and it is further complicated by filestream
> allocation having different aligned allocation constraints than
> the normal bmap allocation.
>
> Split xfs_bmap_btalloc_at_eof() into two parts so we can start to
> align the two different allocator policies more closely.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 147 +++++++++++++++++++++------------------
> 1 file changed, 80 insertions(+), 67 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 30c931b38853..e14671414afb 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3451,81 +3451,81 @@ xfs_bmap_exact_minlen_extent_alloc(
>
> #endif
>
> -/*
> - * If we are not low on available data blocks and we are allocating at
> - * EOF, optimise allocation for contiguous file extension and/or stripe
> - * alignment of the new extent.
> - *
> - * NOTE: ap->aeof is only set if the allocation length is >= the
> - * stripe unit and the allocation offset is at the end of file.
> - */
> + /*
> + * Attempt contiguous allocation for file extension.
> + */
> + static int
> + xfs_bmap_btalloc_at_eof(
> + struct xfs_bmalloca *ap,
> + struct xfs_alloc_arg *args,
> + xfs_extlen_t blen,
> + int stripe_align)
> +{
> + struct xfs_mount *mp = args->mp;
> + struct xfs_perag *caller_pag = args->pag;
> + xfs_extlen_t nextminlen = 0;
> + int error;
> +
> + /*
> + * Compute the minlen+alignment for the next case. Set slop so
> + * that the value of minlen+alignment+slop doesn't go up between
> + * the calls.
> + */
> + args->alignment = 1;
> + if (blen > stripe_align && blen <= args->maxlen)
> + nextminlen = blen - stripe_align;
> + else
> + nextminlen = args->minlen;
> + if (nextminlen + stripe_align > args->minlen + 1)
> + args->minalignslop = nextminlen + stripe_align -
> + args->minlen - 1;
> + else
> + args->minalignslop = 0;
> +
> + if (!caller_pag)
> + args->pag = xfs_perag_get(mp, XFS_FSB_TO_AGNO(mp, ap->blkno));
> + error = xfs_alloc_vextent_exact_bno(args, ap->blkno);
> + if (!caller_pag) {
> + xfs_perag_put(args->pag);
> + args->pag = NULL;
Splitting these two if cases into two less ambitious functions makes it
easier for me to regrok what this code was doing. Thank you,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> + }
> + if (error)
> + return error;
> +
> + if (args->fsbno != NULLFSBLOCK)
> + return 0;
> + /*
> + * Exact allocation failed. Reset to try an aligned allocation
> + * according to the original allocation specification.
> + */
> + args->minlen = nextminlen;
> + return 0;
> +}
> +
> static int
> -xfs_bmap_btalloc_at_eof(
> +xfs_bmap_btalloc_aligned(
> struct xfs_bmalloca *ap,
> struct xfs_alloc_arg *args,
> xfs_extlen_t blen,
> int stripe_align,
> bool ag_only)
> {
> - struct xfs_mount *mp = args->mp;
> - struct xfs_perag *caller_pag = args->pag;
> + struct xfs_perag *caller_pag = args->pag;
> int error;
>
> /*
> - * If there are already extents in the file, try an exact EOF block
> - * allocation to extend the file as a contiguous extent. If that fails,
> - * or it's the first allocation in a file, just try for a stripe aligned
> - * allocation.
> + * If we failed an exact EOF allocation already, stripe
> + * alignment will have already been taken into account in
> + * args->minlen. Hence we only adjust minlen to try to preserve
> + * alignment if no slop has been reserved for alignment
> */
> - if (ap->offset) {
> - xfs_extlen_t nextminlen = 0;
> -
> - /*
> - * Compute the minlen+alignment for the next case. Set slop so
> - * that the value of minlen+alignment+slop doesn't go up between
> - * the calls.
> - */
> - args->alignment = 1;
> - if (blen > stripe_align && blen <= args->maxlen)
> - nextminlen = blen - stripe_align;
> - else
> - nextminlen = args->minlen;
> - if (nextminlen + stripe_align > args->minlen + 1)
> - args->minalignslop = nextminlen + stripe_align -
> - args->minlen - 1;
> - else
> - args->minalignslop = 0;
> -
> - if (!caller_pag)
> - args->pag = xfs_perag_get(mp, XFS_FSB_TO_AGNO(mp, ap->blkno));
> - error = xfs_alloc_vextent_exact_bno(args, ap->blkno);
> - if (!caller_pag) {
> - xfs_perag_put(args->pag);
> - args->pag = NULL;
> - }
> - if (error)
> - return error;
> -
> - if (args->fsbno != NULLFSBLOCK)
> - return 0;
> - /*
> - * Exact allocation failed. Reset to try an aligned allocation
> - * according to the original allocation specification.
> - */
> - args->alignment = stripe_align;
> - args->minlen = nextminlen;
> - args->minalignslop = 0;
> - } else {
> - /*
> - * Adjust minlen to try and preserve alignment if we
> - * can't guarantee an aligned maxlen extent.
> - */
> - args->alignment = stripe_align;
> - if (blen > args->alignment &&
> - blen <= args->maxlen + args->alignment)
> - args->minlen = blen - args->alignment;
> - args->minalignslop = 0;
> + if (args->minalignslop == 0) {
> + if (blen > stripe_align &&
> + blen <= args->maxlen + stripe_align)
> + args->minlen = blen - stripe_align;
> }
> + args->alignment = stripe_align;
> + args->minalignslop = 0;
>
> if (ag_only) {
> error = xfs_alloc_vextent_near_bno(args, ap->blkno);
> @@ -3612,8 +3612,14 @@ xfs_bmap_btalloc_filestreams(
> }
>
> args->minlen = xfs_bmap_select_minlen(ap, args, blen);
> + if (ap->aeof && ap->offset)
> + error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align);
> +
> + if (error || args->fsbno != NULLFSBLOCK)
> + goto out_low_space;
> +
> if (ap->aeof)
> - error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align,
> + error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align,
> true);
>
> if (!error && args->fsbno == NULLFSBLOCK)
> @@ -3662,13 +3668,20 @@ xfs_bmap_btalloc_best_length(
> * optimal or even aligned allocations in this case, so don't waste time
> * trying.
> */
> - if (ap->aeof && !(ap->tp->t_flags & XFS_TRANS_LOWMODE)) {
> - error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align,
> - false);
> + if (ap->aeof && ap->offset && !(ap->tp->t_flags & XFS_TRANS_LOWMODE)) {
> + error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align);
> if (error || args->fsbno != NULLFSBLOCK)
> return error;
> }
>
> + /* attempt aligned allocation for new EOF extents */
> + if (ap->aeof)
> + error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align,
> + false);
> + if (error || args->fsbno != NULLFSBLOCK)
> + return error;
> +
> + /* attempt unaligned allocation */
> error = xfs_alloc_vextent_start_ag(args, ap->blkno);
> if (error || args->fsbno != NULLFSBLOCK)
> return error;
> --
> 2.40.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/9] xfs: split xfs_bmap_btalloc_at_eof()
2023-10-04 0:19 ` [PATCH 1/9] xfs: split xfs_bmap_btalloc_at_eof() Dave Chinner
2023-10-04 23:41 ` Darrick J. Wong
@ 2023-10-05 9:41 ` Christoph Hellwig
1 sibling, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2023-10-05 9:41 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
The split looks good, and much easier to understand than before.
I have a minor nitpick on the callsites below:
> @@ -3612,8 +3612,14 @@ xfs_bmap_btalloc_filestreams(
> }
>
> args->minlen = xfs_bmap_select_minlen(ap, args, blen);
> + if (ap->aeof && ap->offset)
> + error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align);
> +
> + if (error || args->fsbno != NULLFSBLOCK)
> + goto out_low_space;
> +
> if (ap->aeof)
> - error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align,
> + error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align,
> true);
>
> if (!error && args->fsbno == NULLFSBLOCK)
I find the way how this is structured not very helpful to the read,
although most of the blame lies with the pre-existing code. If we'd
check the error where it happens I think it would be way easier to read:
args->minlen = xfs_bmap_select_minlen(ap, args, blen);
if (ap->aeof) {
if (ap->offset) {
error = xfs_bmap_btalloc_at_eof(ap, args, blen,
stripe_align);
if (error || args->fsbno != NULLFSBLOCK)
goto out_low_space;
}
error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align,
true);
if (error || args->fsbno != NULLFSBLOCK)
goto out_low_space;
}
error = xfs_alloc_vextent_near_bno(args, ap->blkno);
The same applies to xfs_bmap_btalloc_best_length.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/9] xfs: contiguous EOF allocation across AGs
2023-10-04 0:19 ` [PATCH 2/9] xfs: contiguous EOF allocation across AGs Dave Chinner
@ 2023-10-05 9:43 ` Christoph Hellwig
2023-10-06 5:27 ` Darrick J. Wong
1 sibling, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2023-10-05 9:43 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] xfs: select the AG with the largest contiguous space
2023-10-04 0:19 ` [PATCH 3/9] xfs: select the AG with the largest contiguous space Dave Chinner
@ 2023-10-05 9:45 ` Christoph Hellwig
2023-10-05 9:46 ` Christoph Hellwig
1 sibling, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2023-10-05 9:45 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
> + if (max_blen > *blen) {
> + if (max_blen_agno != startag) {
> + ap->blkno = XFS_AGB_TO_FSB(mp, max_blen_agno, 0);
> + ap->aeof = false;
> + }
> + *blen = max_blen;
> + }
A comment based on the commit message on why we do this would be nice
here.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] xfs: select the AG with the largest contiguous space
2023-10-04 0:19 ` [PATCH 3/9] xfs: select the AG with the largest contiguous space Dave Chinner
2023-10-05 9:45 ` Christoph Hellwig
@ 2023-10-05 9:46 ` Christoph Hellwig
2023-10-24 16:25 ` Darrick J. Wong
1 sibling, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2023-10-05 9:46 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
On Wed, Oct 04, 2023 at 11:19:37AM +1100, Dave Chinner wrote:
> + if (max_blen > *blen) {
> + if (max_blen_agno != startag) {
> + ap->blkno = XFS_AGB_TO_FSB(mp, max_blen_agno, 0);
> + ap->aeof = false;
> + }
> + *blen = max_blen;
> + }
A comment explaining that we at least want the longest freespace
if no perfect match is available here would be useful to future
readers.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/9] xfs: push the perag outwards in initial allocation
2023-10-04 0:19 ` [PATCH 4/9] xfs: push the perag outwards in initial allocation Dave Chinner
@ 2023-10-05 11:57 ` Christoph Hellwig
0 siblings, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2023-10-05 11:57 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
The logic looks good, and really helps to distinguis the lowspace
case:
Reviewed-by: Christoph Hellwig <hch@lst.de>
However I stll find the loop in xfs_bmap_btalloc_select_lengths a little
suboptimal. I'd go for something like this incremental patch:
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 3c250c89f42e92..c1da9e9cfe05f2 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3268,35 +3268,31 @@ xfs_bmap_btalloc_select_lengths(
xfs_perag_rele(pag);
return error;
}
- if (*blen >= args->maxlen + stripe_align) {
- /*
- * We are going to target a different AG than the
- * incoming target, so we need to reset the target and
- * skip exact EOF allocation attempts.
- */
- if (agno != startag) {
- ap->blkno = XFS_AGB_TO_FSB(mp, agno, 0);
- ap->aeof = false;
- }
- args->pag = pag;
- break;
- }
+
if (*blen > max_blen) {
max_blen = *blen;
max_blen_agno = agno;
+ if (*blen >= args->maxlen + stripe_align)
+ goto out;
}
}
- if (max_blen >= *blen) {
- ASSERT(args->pag == NULL);
- if (max_blen_agno != startag) {
- ap->blkno = XFS_AGB_TO_FSB(mp, max_blen_agno, 0);
- ap->aeof = false;
- }
- *blen = max_blen;
- args->pag = xfs_perag_grab(mp, max_blen_agno);
+ /*
+ * We did not find a perfect fit, so pick the AG with the longest
+ * available free space.
+ */
+ *blen = max_blen;
+ pag = xfs_perag_grab(mp, max_blen_agno);
+out:
+ /*
+ * If we are going to target a different AG than the incoming target,
+ * reset the target and skip exact EOF allocation attempts.
+ */
+ if (max_blen_agno != startag) {
+ ap->blkno = XFS_AGB_TO_FSB(mp, max_blen_agno, 0);
+ ap->aeof = false;
}
-
+ args->pag = pag;
args->minlen = xfs_bmap_select_minlen(ap, args, *blen);
return 0;
}
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 5/9] xfs: aligned EOF allocations don't need to scan AGs anymore
2023-10-04 0:19 ` [PATCH 5/9] xfs: aligned EOF allocations don't need to scan AGs anymore Dave Chinner
@ 2023-10-05 11:59 ` Christoph Hellwig
2023-10-06 5:55 ` Darrick J. Wong
1 sibling, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2023-10-05 11:59 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
> - if (ap->aeof && ap->offset) {
> + if (ap->aeof && ap->offset)
> error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align);
> - }
> +
> + if (error || args->fsbno != NULLFSBLOCK)
> + goto out_perag_rele;
Maybe just personal preference, but I find it much easier to read
if the error handling is indented and sits in a block with the
condition that can cause it, i.e.
if (ap->aeof && ap->offset) {
error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align);
if (error || args->fsbno != NULLFSBLOCK)
goto out_perag_rele;
}
but except for that nipicks this looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/9] xfs: use agno/agbno in xfs_alloc_vextent functions
2023-10-04 0:19 ` [PATCH 6/9] xfs: use agno/agbno in xfs_alloc_vextent functions Dave Chinner
@ 2023-10-05 12:02 ` Christoph Hellwig
2023-10-06 5:56 ` Darrick J. Wong
1 sibling, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2023-10-05 12:02 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 7/9] xfs: caller perag always supplied to xfs_alloc_vextent_near_bno
2023-10-04 0:19 ` [PATCH 7/9] xfs: caller perag always supplied to xfs_alloc_vextent_near_bno Dave Chinner
@ 2023-10-05 12:02 ` Christoph Hellwig
2023-10-06 5:59 ` Darrick J. Wong
1 sibling, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2023-10-05 12:02 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 8/9] xfs: collapse near and exact bno allocation
2023-10-04 0:19 ` [PATCH 8/9] xfs: collapse near and exact bno allocation Dave Chinner
@ 2023-10-05 12:03 ` Christoph Hellwig
2023-10-06 6:01 ` Darrick J. Wong
1 sibling, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2023-10-05 12:03 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
On Wed, Oct 04, 2023 at 11:19:42AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> As they are now identical functions exact for the allocation
> function they call.
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
Although I'd be tempted to just nuke xfs_alloc_vextent_exact_bno
and xfs_alloc_vextent_near_bno and call xfs_alloc_vextent_bno directly.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 9/9] xfs: return -ENOSPC rather than NULLFSBLOCK from allocation functions
2023-10-04 0:19 ` [PATCH 9/9] xfs: return -ENOSPC rather than NULLFSBLOCK from allocation functions Dave Chinner
@ 2023-10-05 12:21 ` Christoph Hellwig
0 siblings, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2023-10-05 12:21 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
> Make sure we don't have a repeat of this situation by changing the
> API to explicitly return ENOSPC when we fail to allocate. If we fail
> to capture this correctly, it will lead to failures being noticed
> either by ENOSPC escaping to userspace or by causing filesystem
> shutdowns when allocations failure where they really shouldn't.
Yes, the retur 0 on ENOSPC has driven me crazy in the past.
Note that you now also drop the XXX comment on xfs_alloc_vextent_finish
about this.
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 27c62f303488..13fda27fabcb 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -1157,9 +1157,9 @@ xfs_alloc_ag_vextent_small(
> * Can't do the allocation, give up.
> */
> if (flen < args->minlen) {
> - args->agbno = NULLAGBLOCK;
> trace_xfs_alloc_small_notenough(args);
> - flen = 0;
> + error = -ENOSPC;
> + goto error;
I suspect a direct return -ENOSPC here might be better, as we already
have the trace_xfs_alloc_small_notenough tracepoint here, and also
hitting trace_xfs_alloc_small_error wouldn't make much sense (and be
a pointles behavior change).
Looking at the callers of xfs_alloc_ag_vextent_small, both seem to
need an update to check for -ENOSPC explicitly, as they first check
for an error and only after that for i == 0 || len == 0 to detect
the no space case.
> @@ -3375,14 +3370,7 @@ xfs_alloc_vextent_finish(
> args->agno > minimum_agno))
> args->tp->t_highest_agno = args->agno;
>
> - /*
> - * If the allocation failed with an error or we had an ENOSPC result,
> - * preserve the returned error whilst also marking the allocation result
> - * as "no extent allocated". This ensures that callers that fail to
> - * capture the error will still treat it as a failed allocation.
> - */
> - if (alloc_error || args->agbno == NULLAGBLOCK) {
> - args->fsbno = NULLFSBLOCK;
> + if (alloc_error) {
> error = alloc_error;
> goto out_drop_perag;
> }
Maybe throw in a
ASSERT(args->agbno != NULLAGBLOCK);
after this conditional to catch backporting errors and the like?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/9] xfs: contiguous EOF allocation across AGs
2023-10-04 0:19 ` [PATCH 2/9] xfs: contiguous EOF allocation across AGs Dave Chinner
2023-10-05 9:43 ` Christoph Hellwig
@ 2023-10-06 5:27 ` Darrick J. Wong
1 sibling, 0 replies; 27+ messages in thread
From: Darrick J. Wong @ 2023-10-06 5:27 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
On Wed, Oct 04, 2023 at 11:19:36AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Currently when we allocate at EOF, we set the initial target to the
> location of the inode. Then we call xfs_bmap_adjacent(), which sees
> that we are doing an EOF extension, so it moves the target to the
> last block of the previous extent. This may be in a different AG to
> the inode.
>
> When we then go to select the AG with the best free length, the AG
> at the target block might not have sufficient free space for the
> full allocation, so we may select a different AG. We then do an
> exact BNO allocation with the original target (EOF block), which
> reverts back to attempting an allocation in an AG that doesn't have
> sufficient contiguous free space available.
>
> This generally leads to allocation failure, and then we fall back to
> scanning the AGs for one that the allocation will succeed in. This
> scan also results in contended AGS being skipped, so we have no idea
> what AG we are going to end up allocating in. For sequential writes,
> this results in random extents being located in random places in
> non-target AGs.
>
> We want to guarantee that we can allocate in the AG that we have
> selected as having the "best contiguous free space" efficiently,
> so if we select a different AG, we should move the allocation target
> and skip the exact EOF allocation as we know it will not succeed.
> i.e. we should start with aligned allocation immediately, knowing it
> will likely succeed.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
Sounds good to me.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> fs/xfs/libxfs/xfs_bmap.c | 32 ++++++++++++++++++++++++++------
> 1 file changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index e14671414afb..e64ba7e2d13d 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3252,8 +3252,18 @@ xfs_bmap_btalloc_select_lengths(
> if (error && error != -EAGAIN)
> break;
> error = 0;
> - if (*blen >= args->maxlen)
> + if (*blen >= args->maxlen) {
> + /*
> + * We are going to target a different AG than the
> + * incoming target, so we need to reset the target and
> + * skip exact EOF allocation attempts.
> + */
> + if (agno != startag) {
> + ap->blkno = XFS_AGB_TO_FSB(mp, agno, 0);
> + ap->aeof = false;
> + }
> break;
> + }
> }
> if (pag)
> xfs_perag_rele(pag);
> @@ -3514,10 +3524,10 @@ xfs_bmap_btalloc_aligned(
> int error;
>
> /*
> - * If we failed an exact EOF allocation already, stripe
> - * alignment will have already been taken into account in
> - * args->minlen. Hence we only adjust minlen to try to preserve
> - * alignment if no slop has been reserved for alignment
> + * If we failed an exact EOF allocation already, stripe alignment will
> + * have already been taken into account in args->minlen. Hence we only
> + * adjust minlen to try to preserve alignment if no slop has been
> + * reserved for alignment
> */
> if (args->minalignslop == 0) {
> if (blen > stripe_align &&
> @@ -3653,6 +3663,16 @@ xfs_bmap_btalloc_best_length(
> ap->blkno = XFS_INO_TO_FSB(args->mp, ap->ip->i_ino);
> xfs_bmap_adjacent(ap);
>
> + /*
> + * We only use stripe alignment for EOF allocations. Hence if it isn't
> + * an EOF allocation, clear the stripe alignment. This allows us to
> + * skip exact block EOF allocation yet still do stripe aligned
> + * allocation if we select a different AG to the
> + * exact target block due to a lack of contiguous free space.
> + */
> + if (!ap->aeof)
> + stripe_align = 0;
> +
> /*
> * Search for an allocation group with a single extent large enough for
> * the request. If one isn't found, then adjust the minimum allocation
> @@ -3675,7 +3695,7 @@ xfs_bmap_btalloc_best_length(
> }
>
> /* attempt aligned allocation for new EOF extents */
> - if (ap->aeof)
> + if (stripe_align)
> error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align,
> false);
> if (error || args->fsbno != NULLFSBLOCK)
> --
> 2.40.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 5/9] xfs: aligned EOF allocations don't need to scan AGs anymore
2023-10-04 0:19 ` [PATCH 5/9] xfs: aligned EOF allocations don't need to scan AGs anymore Dave Chinner
2023-10-05 11:59 ` Christoph Hellwig
@ 2023-10-06 5:55 ` Darrick J. Wong
1 sibling, 0 replies; 27+ messages in thread
From: Darrick J. Wong @ 2023-10-06 5:55 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
On Wed, Oct 04, 2023 at 11:19:39AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Now that contiguous free space selection takes into account stripe
> alignment, we no longer need to do an "all AGs" allocation scan in
> the case the initial AG doesn't have enough contiguous free space
> for a stripe aligned allocation. This cleans up
> xfs_bmap_btalloc_aligned() the same for both filestreams and the
> normal btree allocation code.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 40 +++++++++++++---------------------------
> 1 file changed, 13 insertions(+), 27 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 3c250c89f42e..c1e2c0707e20 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3538,10 +3538,8 @@ xfs_bmap_btalloc_aligned(
> struct xfs_bmalloca *ap,
> struct xfs_alloc_arg *args,
> xfs_extlen_t blen,
> - int stripe_align,
> - bool ag_only)
> + int stripe_align)
> {
> - struct xfs_perag *caller_pag = args->pag;
> int error;
>
> /*
> @@ -3558,14 +3556,7 @@ xfs_bmap_btalloc_aligned(
> args->alignment = stripe_align;
> args->minalignslop = 0;
>
> - if (ag_only) {
> - error = xfs_alloc_vextent_near_bno(args, ap->blkno);
> - } else {
> - args->pag = NULL;
> - error = xfs_alloc_vextent_start_ag(args, ap->blkno);
> - ASSERT(args->pag == NULL);
> - args->pag = caller_pag;
> - }
> + error = xfs_alloc_vextent_near_bno(args, ap->blkno);
> if (error)
> return error;
>
> @@ -3650,8 +3641,7 @@ xfs_bmap_btalloc_filestreams(
> goto out_low_space;
>
> if (ap->aeof)
> - error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align,
> - true);
> + error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align);
>
> if (!error && args->fsbno == NULLFSBLOCK)
> error = xfs_alloc_vextent_near_bno(args, ap->blkno);
> @@ -3715,9 +3705,16 @@ xfs_bmap_btalloc_best_length(
> return error;
> ASSERT(args->pag);
>
> - if (ap->aeof && ap->offset) {
> + if (ap->aeof && ap->offset)
> error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align);
> - }
> +
> + if (error || args->fsbno != NULLFSBLOCK)
> + goto out_perag_rele;
> +
> +
Double blank lines here. With that fixed,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
I like the simplifications going on here.
--D
> + /* attempt aligned allocation for new EOF extents */
> + if (stripe_align)
> + error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align);
>
> /*
> * We are now done with the perag reference for the optimal allocation
> @@ -3725,24 +3722,13 @@ xfs_bmap_btalloc_best_length(
> * now as we've either succeeded, had a fatal error or we are out of
> * space and need to do a full filesystem scan for free space which will
> * take it's own references.
> - *
> - * XXX: now that xfs_bmap_btalloc_select_lengths() selects an AG with
> - * enough contiguous free space in it for an aligned allocation, we
> - * can change the aligned allocation at EOF to just be a single AG
> - * allocation.
> */
> +out_perag_rele:
> xfs_perag_rele(args->pag);
> args->pag = NULL;
> if (error || args->fsbno != NULLFSBLOCK)
> return error;
>
> - /* attempt aligned allocation for new EOF extents */
> - if (stripe_align)
> - error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align,
> - false);
> - if (error || args->fsbno != NULLFSBLOCK)
> - return error;
> -
> /* attempt unaligned allocation */
> error = xfs_alloc_vextent_start_ag(args, ap->blkno);
> if (error || args->fsbno != NULLFSBLOCK)
> --
> 2.40.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/9] xfs: use agno/agbno in xfs_alloc_vextent functions
2023-10-04 0:19 ` [PATCH 6/9] xfs: use agno/agbno in xfs_alloc_vextent functions Dave Chinner
2023-10-05 12:02 ` Christoph Hellwig
@ 2023-10-06 5:56 ` Darrick J. Wong
1 sibling, 0 replies; 27+ messages in thread
From: Darrick J. Wong @ 2023-10-06 5:56 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
On Wed, Oct 04, 2023 at 11:19:40AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> We're moving away from using xfs_fsblock_t to define allocation
> targets, and instead we are using {agno,agbno} tuples instead. This
> will allow us to eventually move everything to {perag,agbno}. But
> before we get there, we need to split the fsblock into {agno,agbno}
> and convert the code to use those first.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
Looks promising,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> fs/xfs/libxfs/xfs_alloc.c | 57 +++++++++++++++++++++------------------
> 1 file changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 3069194527dd..289b54911b05 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -3249,7 +3249,8 @@ xfs_alloc_read_agf(
> static int
> xfs_alloc_vextent_check_args(
> struct xfs_alloc_arg *args,
> - xfs_fsblock_t target,
> + xfs_agnumber_t target_agno,
> + xfs_agblock_t target_agbno,
> xfs_agnumber_t *minimum_agno)
> {
> struct xfs_mount *mp = args->mp;
> @@ -3277,14 +3278,14 @@ xfs_alloc_vextent_check_args(
> ASSERT(args->alignment > 0);
> ASSERT(args->resv != XFS_AG_RESV_AGFL);
>
> - ASSERT(XFS_FSB_TO_AGNO(mp, target) < mp->m_sb.sb_agcount);
> - ASSERT(XFS_FSB_TO_AGBNO(mp, target) < agsize);
> + ASSERT(target_agno < mp->m_sb.sb_agcount);
> + ASSERT(target_agbno < agsize);
> ASSERT(args->minlen <= args->maxlen);
> ASSERT(args->minlen <= agsize);
> ASSERT(args->mod < args->prod);
>
> - if (XFS_FSB_TO_AGNO(mp, target) >= mp->m_sb.sb_agcount ||
> - XFS_FSB_TO_AGBNO(mp, target) >= agsize ||
> + if (target_agno >= mp->m_sb.sb_agcount ||
> + target_agbno >= agsize ||
> args->minlen > args->maxlen || args->minlen > agsize ||
> args->mod >= args->prod) {
> trace_xfs_alloc_vextent_badargs(args);
> @@ -3438,7 +3439,6 @@ xfs_alloc_vextent_this_ag(
> struct xfs_alloc_arg *args,
> xfs_agnumber_t agno)
> {
> - struct xfs_mount *mp = args->mp;
> xfs_agnumber_t minimum_agno;
> uint32_t alloc_flags = 0;
> int error;
> @@ -3451,8 +3451,7 @@ xfs_alloc_vextent_this_ag(
>
> trace_xfs_alloc_vextent_this_ag(args);
>
> - error = xfs_alloc_vextent_check_args(args, XFS_AGB_TO_FSB(mp, agno, 0),
> - &minimum_agno);
> + error = xfs_alloc_vextent_check_args(args, agno, 0, &minimum_agno);
> if (error) {
> if (error == -ENOSPC)
> return 0;
> @@ -3563,7 +3562,8 @@ xfs_alloc_vextent_start_ag(
> {
> struct xfs_mount *mp = args->mp;
> xfs_agnumber_t minimum_agno;
> - xfs_agnumber_t start_agno;
> + xfs_agnumber_t target_agno;
> + xfs_agblock_t target_agbno;
> xfs_agnumber_t rotorstep = xfs_rotorstep;
> bool bump_rotor = false;
> uint32_t alloc_flags = XFS_ALLOC_FLAG_TRYLOCK;
> @@ -3576,7 +3576,10 @@ xfs_alloc_vextent_start_ag(
>
> trace_xfs_alloc_vextent_start_ag(args);
>
> - error = xfs_alloc_vextent_check_args(args, target, &minimum_agno);
> + target_agno = XFS_FSB_TO_AGNO(mp, target);
> + target_agbno = XFS_FSB_TO_AGBNO(mp, target);
> + error = xfs_alloc_vextent_check_args(args, target_agno, target_agbno,
> + &minimum_agno);
> if (error) {
> if (error == -ENOSPC)
> return 0;
> @@ -3591,12 +3594,11 @@ xfs_alloc_vextent_start_ag(
> bump_rotor = 1;
> }
>
> - start_agno = max(minimum_agno, XFS_FSB_TO_AGNO(mp, target));
> - error = xfs_alloc_vextent_iterate_ags(args, minimum_agno, start_agno,
> - XFS_FSB_TO_AGBNO(mp, target), alloc_flags);
> -
> + target_agno = max(minimum_agno, target_agno);
> + error = xfs_alloc_vextent_iterate_ags(args, minimum_agno, target_agno,
> + target_agbno, alloc_flags);
> if (bump_rotor) {
> - if (args->agno == start_agno)
> + if (args->agno == target_agno)
> mp->m_agfrotor = (mp->m_agfrotor + 1) %
> (mp->m_sb.sb_agcount * rotorstep);
> else
> @@ -3619,7 +3621,8 @@ xfs_alloc_vextent_first_ag(
> {
> struct xfs_mount *mp = args->mp;
> xfs_agnumber_t minimum_agno;
> - xfs_agnumber_t start_agno;
> + xfs_agnumber_t target_agno;
> + xfs_agblock_t target_agbno;
> uint32_t alloc_flags = XFS_ALLOC_FLAG_TRYLOCK;
> int error;
>
> @@ -3630,16 +3633,19 @@ xfs_alloc_vextent_first_ag(
>
> trace_xfs_alloc_vextent_first_ag(args);
>
> - error = xfs_alloc_vextent_check_args(args, target, &minimum_agno);
> + target_agno = XFS_FSB_TO_AGNO(mp, target);
> + target_agbno = XFS_FSB_TO_AGBNO(mp, target);
> + error = xfs_alloc_vextent_check_args(args, target_agno, target_agbno,
> + &minimum_agno);
> if (error) {
> if (error == -ENOSPC)
> return 0;
> return error;
> }
>
> - start_agno = max(minimum_agno, XFS_FSB_TO_AGNO(mp, target));
> - error = xfs_alloc_vextent_iterate_ags(args, minimum_agno, start_agno,
> - XFS_FSB_TO_AGBNO(mp, target), alloc_flags);
> + target_agno = max(minimum_agno, target_agno);
> + error = xfs_alloc_vextent_iterate_ags(args, minimum_agno, target_agno,
> + target_agbno, alloc_flags);
> return xfs_alloc_vextent_finish(args, minimum_agno, error, true);
> }
>
> @@ -3656,15 +3662,13 @@ xfs_alloc_vextent_exact_bno(
> xfs_agnumber_t minimum_agno;
> int error;
>
> - ASSERT(args->pag != NULL);
> - ASSERT(args->pag->pag_agno == XFS_FSB_TO_AGNO(mp, target));
> -
> - args->agno = XFS_FSB_TO_AGNO(mp, target);
> + args->agno = args->pag->pag_agno;
> args->agbno = XFS_FSB_TO_AGBNO(mp, target);
>
> trace_xfs_alloc_vextent_exact_bno(args);
>
> - error = xfs_alloc_vextent_check_args(args, target, &minimum_agno);
> + error = xfs_alloc_vextent_check_args(args, args->agno, args->agbno,
> + &minimum_agno);
> if (error) {
> if (error == -ENOSPC)
> return 0;
> @@ -3703,7 +3707,8 @@ xfs_alloc_vextent_near_bno(
>
> trace_xfs_alloc_vextent_near_bno(args);
>
> - error = xfs_alloc_vextent_check_args(args, target, &minimum_agno);
> + error = xfs_alloc_vextent_check_args(args, args->agno, args->agbno,
> + &minimum_agno);
> if (error) {
> if (error == -ENOSPC)
> return 0;
> --
> 2.40.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 7/9] xfs: caller perag always supplied to xfs_alloc_vextent_near_bno
2023-10-04 0:19 ` [PATCH 7/9] xfs: caller perag always supplied to xfs_alloc_vextent_near_bno Dave Chinner
2023-10-05 12:02 ` Christoph Hellwig
@ 2023-10-06 5:59 ` Darrick J. Wong
1 sibling, 0 replies; 27+ messages in thread
From: Darrick J. Wong @ 2023-10-06 5:59 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
On Wed, Oct 04, 2023 at 11:19:41AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> So we don't need to conditionally grab the perag anymore.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/libxfs/xfs_alloc.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 289b54911b05..3190c8204903 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -3686,7 +3686,7 @@ xfs_alloc_vextent_exact_bno(
> * Allocate an extent as close to the target as possible. If there are not
> * viable candidates in the AG, then fail the allocation.
> *
> - * Caller may or may not have a per-ag reference in args->pag.
> + * Caller is expected to hold a per-ag reference in args->pag.
"Caller must hold..." ?
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> */
> int
> xfs_alloc_vextent_near_bno(
> @@ -3695,14 +3695,12 @@ xfs_alloc_vextent_near_bno(
> {
> struct xfs_mount *mp = args->mp;
> xfs_agnumber_t minimum_agno;
> - bool needs_perag = args->pag == NULL;
> uint32_t alloc_flags = 0;
> int error;
>
> - if (!needs_perag)
> - ASSERT(args->pag->pag_agno == XFS_FSB_TO_AGNO(mp, target));
> + ASSERT(args->pag->pag_agno == XFS_FSB_TO_AGNO(mp, target));
>
> - args->agno = XFS_FSB_TO_AGNO(mp, target);
> + args->agno = args->pag->pag_agno;
> args->agbno = XFS_FSB_TO_AGBNO(mp, target);
>
> trace_xfs_alloc_vextent_near_bno(args);
> @@ -3715,14 +3713,11 @@ xfs_alloc_vextent_near_bno(
> return error;
> }
>
> - if (needs_perag)
> - args->pag = xfs_perag_grab(mp, args->agno);
> -
> error = xfs_alloc_vextent_prepare_ag(args, alloc_flags);
> if (!error && args->agbp)
> error = xfs_alloc_ag_vextent_near(args, alloc_flags);
>
> - return xfs_alloc_vextent_finish(args, minimum_agno, error, needs_perag);
> + return xfs_alloc_vextent_finish(args, minimum_agno, error, false);
> }
>
> /* Ensure that the freelist is at full capacity. */
> --
> 2.40.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 8/9] xfs: collapse near and exact bno allocation
2023-10-04 0:19 ` [PATCH 8/9] xfs: collapse near and exact bno allocation Dave Chinner
2023-10-05 12:03 ` Christoph Hellwig
@ 2023-10-06 6:01 ` Darrick J. Wong
1 sibling, 0 replies; 27+ messages in thread
From: Darrick J. Wong @ 2023-10-06 6:01 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, john.g.garry
On Wed, Oct 04, 2023 at 11:19:42AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> As they are now identical functions exact for the allocation
> function they call.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/libxfs/xfs_alloc.c | 84 ++++++++++++++++++---------------------
> fs/xfs/xfs_trace.h | 1 +
> 2 files changed, 40 insertions(+), 45 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 3190c8204903..27c62f303488 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -3649,6 +3649,43 @@ xfs_alloc_vextent_first_ag(
> return xfs_alloc_vextent_finish(args, minimum_agno, error, true);
> }
>
> +static int
> +xfs_alloc_vextent_bno(
> + struct xfs_alloc_arg *args,
> + xfs_fsblock_t target,
> + bool exact)
> +{
> + struct xfs_mount *mp = args->mp;
> + xfs_agnumber_t minimum_agno;
> + int error;
> +
> + ASSERT(args->pag != NULL);
> + ASSERT(args->pag->pag_agno == XFS_FSB_TO_AGNO(mp, target));
> +
> + args->agno = args->pag->pag_agno;
> + args->agbno = XFS_FSB_TO_AGBNO(mp, target);
> +
> + trace_xfs_alloc_vextent_bno(args);
Not sure why we need this new tracepoint when a tracepoint is triggered
immediately prior to both callsites?
--D
> +
> + error = xfs_alloc_vextent_check_args(args, args->agno, args->agbno,
> + &minimum_agno);
> + if (error) {
> + if (error == -ENOSPC)
> + return 0;
> + return error;
> + }
> +
> + error = xfs_alloc_vextent_prepare_ag(args, 0);
> + if (!error && args->agbp) {
> + if (exact)
> + error = xfs_alloc_ag_vextent_exact(args);
> + else
> + error = xfs_alloc_ag_vextent_near(args, 0);
> + }
> +
> + return xfs_alloc_vextent_finish(args, minimum_agno, error, false);
> +}
> +
> /*
> * Allocate at the exact block target or fail. Caller is expected to hold a
> * perag reference in args->pag.
> @@ -3658,28 +3695,8 @@ xfs_alloc_vextent_exact_bno(
> struct xfs_alloc_arg *args,
> xfs_fsblock_t target)
> {
> - struct xfs_mount *mp = args->mp;
> - xfs_agnumber_t minimum_agno;
> - int error;
> -
> - args->agno = args->pag->pag_agno;
> - args->agbno = XFS_FSB_TO_AGBNO(mp, target);
> -
> trace_xfs_alloc_vextent_exact_bno(args);
> -
> - error = xfs_alloc_vextent_check_args(args, args->agno, args->agbno,
> - &minimum_agno);
> - if (error) {
> - if (error == -ENOSPC)
> - return 0;
> - return error;
> - }
> -
> - error = xfs_alloc_vextent_prepare_ag(args, 0);
> - if (!error && args->agbp)
> - error = xfs_alloc_ag_vextent_exact(args);
> -
> - return xfs_alloc_vextent_finish(args, minimum_agno, error, false);
> + return xfs_alloc_vextent_bno(args, target, true);
> }
>
> /*
> @@ -3693,31 +3710,8 @@ xfs_alloc_vextent_near_bno(
> struct xfs_alloc_arg *args,
> xfs_fsblock_t target)
> {
> - struct xfs_mount *mp = args->mp;
> - xfs_agnumber_t minimum_agno;
> - uint32_t alloc_flags = 0;
> - int error;
> -
> - ASSERT(args->pag->pag_agno == XFS_FSB_TO_AGNO(mp, target));
> -
> - args->agno = args->pag->pag_agno;
> - args->agbno = XFS_FSB_TO_AGBNO(mp, target);
> -
> trace_xfs_alloc_vextent_near_bno(args);
> -
> - error = xfs_alloc_vextent_check_args(args, args->agno, args->agbno,
> - &minimum_agno);
> - if (error) {
> - if (error == -ENOSPC)
> - return 0;
> - return error;
> - }
> -
> - error = xfs_alloc_vextent_prepare_ag(args, alloc_flags);
> - if (!error && args->agbp)
> - error = xfs_alloc_ag_vextent_near(args, alloc_flags);
> -
> - return xfs_alloc_vextent_finish(args, minimum_agno, error, false);
> + return xfs_alloc_vextent_bno(args, target, false);
> }
>
> /* Ensure that the freelist is at full capacity. */
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 3926cf7f2a6e..628da36b20b9 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -1884,6 +1884,7 @@ DEFINE_ALLOC_EVENT(xfs_alloc_vextent_start_ag);
> DEFINE_ALLOC_EVENT(xfs_alloc_vextent_first_ag);
> DEFINE_ALLOC_EVENT(xfs_alloc_vextent_exact_bno);
> DEFINE_ALLOC_EVENT(xfs_alloc_vextent_near_bno);
> +DEFINE_ALLOC_EVENT(xfs_alloc_vextent_bno);
> DEFINE_ALLOC_EVENT(xfs_alloc_vextent_finish);
>
> TRACE_EVENT(xfs_alloc_cur_check,
> --
> 2.40.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] xfs: select the AG with the largest contiguous space
2023-10-05 9:46 ` Christoph Hellwig
@ 2023-10-24 16:25 ` Darrick J. Wong
0 siblings, 0 replies; 27+ messages in thread
From: Darrick J. Wong @ 2023-10-24 16:25 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Dave Chinner, linux-xfs, john.g.garry
On Thu, Oct 05, 2023 at 02:46:18AM -0700, Christoph Hellwig wrote:
> On Wed, Oct 04, 2023 at 11:19:37AM +1100, Dave Chinner wrote:
> > + if (max_blen > *blen) {
> > + if (max_blen_agno != startag) {
> > + ap->blkno = XFS_AGB_TO_FSB(mp, max_blen_agno, 0);
> > + ap->aeof = false;
> > + }
> > + *blen = max_blen;
> > + }
>
> A comment explaining that we at least want the longest freespace
> if no perfect match is available here would be useful to future
> readers.
With Christoph's request for an extra comment added,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
>
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2023-10-24 16:25 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-04 0:19 [RFC PATCH 0/9] xfs: push perags further into allocation routines Dave Chinner
2023-10-04 0:19 ` [PATCH 1/9] xfs: split xfs_bmap_btalloc_at_eof() Dave Chinner
2023-10-04 23:41 ` Darrick J. Wong
2023-10-05 9:41 ` Christoph Hellwig
2023-10-04 0:19 ` [PATCH 2/9] xfs: contiguous EOF allocation across AGs Dave Chinner
2023-10-05 9:43 ` Christoph Hellwig
2023-10-06 5:27 ` Darrick J. Wong
2023-10-04 0:19 ` [PATCH 3/9] xfs: select the AG with the largest contiguous space Dave Chinner
2023-10-05 9:45 ` Christoph Hellwig
2023-10-05 9:46 ` Christoph Hellwig
2023-10-24 16:25 ` Darrick J. Wong
2023-10-04 0:19 ` [PATCH 4/9] xfs: push the perag outwards in initial allocation Dave Chinner
2023-10-05 11:57 ` Christoph Hellwig
2023-10-04 0:19 ` [PATCH 5/9] xfs: aligned EOF allocations don't need to scan AGs anymore Dave Chinner
2023-10-05 11:59 ` Christoph Hellwig
2023-10-06 5:55 ` Darrick J. Wong
2023-10-04 0:19 ` [PATCH 6/9] xfs: use agno/agbno in xfs_alloc_vextent functions Dave Chinner
2023-10-05 12:02 ` Christoph Hellwig
2023-10-06 5:56 ` Darrick J. Wong
2023-10-04 0:19 ` [PATCH 7/9] xfs: caller perag always supplied to xfs_alloc_vextent_near_bno Dave Chinner
2023-10-05 12:02 ` Christoph Hellwig
2023-10-06 5:59 ` Darrick J. Wong
2023-10-04 0:19 ` [PATCH 8/9] xfs: collapse near and exact bno allocation Dave Chinner
2023-10-05 12:03 ` Christoph Hellwig
2023-10-06 6:01 ` Darrick J. Wong
2023-10-04 0:19 ` [PATCH 9/9] xfs: return -ENOSPC rather than NULLFSBLOCK from allocation functions Dave Chinner
2023-10-05 12:21 ` Christoph Hellwig
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 NNTP newsgroup(s).