diff mbox series

[1/2] xfs: cap longest free extent to maximum allocatable

Message ID 20190918082453.25266-2-cmaiolino@redhat.com (mailing list archive)
State Superseded
Headers show
Series A small improvement in the allocation algorithm | expand

Commit Message

Carlos Maiolino Sept. 18, 2019, 8:24 a.m. UTC
From: Dave Chinner <dchinner@redhat.com>

Cap longest extent to the largest we can allocate based on limits
calculated at mount time. Dynamic state (such as finobt blocks)
can result in the longest free extent exceeding the size we can
allocate, and that results in failure to align full AG allocations
when the AG is empty.

Result:

xfs_io-4413  [003]   426.412459: xfs_alloc_vextent_loopfailed: dev 8:96 agno 0 agbno 32 minlen 243968 maxlen 244000 mod 0 prod 1 minleft 1 total 262148 alignment 32 minalignslop 0 len 0 type NEAR_BNO otype START_BNO wasdel 0 wasfromfl 0 resv 0 datatype 0x5 firstblock 0xffffffffffffffff

minlen and maxlen are now separated by the alignment size, and
allocation fails because args.total > free space in the AG.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/libxfs/xfs_alloc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Brian Foster Sept. 18, 2019, 12:27 p.m. UTC | #1
On Wed, Sep 18, 2019 at 10:24:52AM +0200, Carlos Maiolino wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Cap longest extent to the largest we can allocate based on limits
> calculated at mount time. Dynamic state (such as finobt blocks)
> can result in the longest free extent exceeding the size we can
> allocate, and that results in failure to align full AG allocations
> when the AG is empty.
> 
> Result:
> 
> xfs_io-4413  [003]   426.412459: xfs_alloc_vextent_loopfailed: dev 8:96 agno 0 agbno 32 minlen 243968 maxlen 244000 mod 0 prod 1 minleft 1 total 262148 alignment 32 minalignslop 0 len 0 type NEAR_BNO otype START_BNO wasdel 0 wasfromfl 0 resv 0 datatype 0x5 firstblock 0xffffffffffffffff
> 
> minlen and maxlen are now separated by the alignment size, and
> allocation fails because args.total > free space in the AG.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---

Seems fine, but what about the bma.minlen alignment fix (in
xfs_bmap_btalloc()) Dave suggested in the previous thread?

Brian

>  fs/xfs/libxfs/xfs_alloc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 372ad55631fc..35b39fc863a0 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -1989,7 +1989,8 @@ xfs_alloc_longest_free_extent(
>  	 * reservations and AGFL rules in place, we can return this extent.
>  	 */
>  	if (pag->pagf_longest > delta)
> -		return pag->pagf_longest - delta;
> +		return min_t(xfs_extlen_t, pag->pag_mount->m_ag_max_usable,
> +				pag->pagf_longest - delta);
>  
>  	/* Otherwise, let the caller try for 1 block if there's space. */
>  	return pag->pagf_flcount > 0 || pag->pagf_longest > 0;
> -- 
> 2.20.1
>
Carlos Maiolino Sept. 23, 2019, 12:25 p.m. UTC | #2
On Wed, Sep 18, 2019 at 08:27:26AM -0400, Brian Foster wrote:
> On Wed, Sep 18, 2019 at 10:24:52AM +0200, Carlos Maiolino wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Cap longest extent to the largest we can allocate based on limits
> > calculated at mount time. Dynamic state (such as finobt blocks)
> > can result in the longest free extent exceeding the size we can
> > allocate, and that results in failure to align full AG allocations
> > when the AG is empty.
> > 
> > Result:
> > 
> > xfs_io-4413  [003]   426.412459: xfs_alloc_vextent_loopfailed: dev 8:96 agno 0 agbno 32 minlen 243968 maxlen 244000 mod 0 prod 1 minleft 1 total 262148 alignment 32 minalignslop 0 len 0 type NEAR_BNO otype START_BNO wasdel 0 wasfromfl 0 resv 0 datatype 0x5 firstblock 0xffffffffffffffff
> > 
> > minlen and maxlen are now separated by the alignment size, and
> > allocation fails because args.total > free space in the AG.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> 
> Seems fine, but what about the bma.minlen alignment fix (in
> xfs_bmap_btalloc()) Dave suggested in the previous thread?

I forgot to git add that while playing with this set :P

> 
> Brian
> 
> >  fs/xfs/libxfs/xfs_alloc.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> > index 372ad55631fc..35b39fc863a0 100644
> > --- a/fs/xfs/libxfs/xfs_alloc.c
> > +++ b/fs/xfs/libxfs/xfs_alloc.c
> > @@ -1989,7 +1989,8 @@ xfs_alloc_longest_free_extent(
> >  	 * reservations and AGFL rules in place, we can return this extent.
> >  	 */
> >  	if (pag->pagf_longest > delta)
> > -		return pag->pagf_longest - delta;
> > +		return min_t(xfs_extlen_t, pag->pag_mount->m_ag_max_usable,
> > +				pag->pagf_longest - delta);
> >  
> >  	/* Otherwise, let the caller try for 1 block if there's space. */
> >  	return pag->pagf_flcount > 0 || pag->pagf_longest > 0;
> > -- 
> > 2.20.1
> >
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 372ad55631fc..35b39fc863a0 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -1989,7 +1989,8 @@  xfs_alloc_longest_free_extent(
 	 * reservations and AGFL rules in place, we can return this extent.
 	 */
 	if (pag->pagf_longest > delta)
-		return pag->pagf_longest - delta;
+		return min_t(xfs_extlen_t, pag->pag_mount->m_ag_max_usable,
+				pag->pagf_longest - delta);
 
 	/* Otherwise, let the caller try for 1 block if there's space. */
 	return pag->pagf_flcount > 0 || pag->pagf_longest > 0;