fix GFP_ flag use in the buffer cache

fix GFP_ flag use in the buffer cache

[Christoph Hellwig]

Hi all,

usage of the GFP_ flags in the buffer cache, including mixing
__GFP_NOFAIL and __GFP_NORETRY which is nonsensical, and passing
__GFP_NORETRY to the dedicated fallback.

Diffstat:
 xfs_buf.c |  105 +++++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 60 insertions(+), 45 deletions(-)

[PATCH 1/4] xfs: split up xfs_buf_alloc_backing_mem

[Christoph Hellwig]

Split out helpers for folio and vmalloc allocations to prepare for a bug
fix.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf.c | 61 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 21 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 0cea458f1353..d3d44e3ff001 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -120,6 +120,22 @@ xfs_buf_free(
 	call_rcu(&bp->b_rcu, xfs_buf_free_callback);
 }
 
+static int
+xfs_buf_alloc_folio(
+	struct xfs_buf		*bp,
+	size_t			size,
+	gfp_t			gfp_mask)
+{
+	struct folio		*folio;
+
+	folio = folio_alloc(gfp_mask, get_order(size));
+	if (!folio)
+		return -ENOMEM;
+	bp->b_addr = folio_address(folio);
+	trace_xfs_buf_backing_folio(bp, _RET_IP_);
+	return 0;
+}
+
 static int
 xfs_buf_alloc_kmem(
 	struct xfs_buf		*bp,
@@ -148,6 +164,27 @@ xfs_buf_alloc_kmem(
 	return 0;
 }
 
+static int
+xfs_buf_alloc_vmalloc(
+	struct xfs_buf		*bp,
+	size_t			size,
+	gfp_t			gfp_mask,
+	xfs_buf_flags_t		flags)
+{
+	for (;;) {
+		bp->b_addr = __vmalloc(size, gfp_mask);
+		if (bp->b_addr)
+			break;
+		if (flags & XBF_READ_AHEAD)
+			return -ENOMEM;
+		XFS_STATS_INC(bp->b_mount, xb_page_retries);
+		memalloc_retry_wait(gfp_mask);
+	}
+
+	trace_xfs_buf_backing_vmalloc(bp, _RET_IP_);
+	return 0;
+}
+
 /*
  * Allocate backing memory for a buffer.
  *
@@ -175,7 +212,6 @@ xfs_buf_alloc_backing_mem(
 {
 	size_t		size = BBTOB(bp->b_length);
 	gfp_t		gfp_mask = GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOWARN;
-	struct folio	*folio;
 
 	if (xfs_buftarg_is_mem(bp->b_target))
 		return xmbuf_map_backing_mem(bp);
@@ -216,33 +252,16 @@ xfs_buf_alloc_backing_mem(
 	 */
 	if (size > PAGE_SIZE) {
 		if (!is_power_of_2(size))
-			goto fallback;
+			return xfs_buf_alloc_vmalloc(bp, size, gfp_mask, flags);
 		gfp_mask &= ~__GFP_DIRECT_RECLAIM;
 		gfp_mask |= __GFP_NORETRY;
 	}
-	folio = folio_alloc(gfp_mask, get_order(size));
-	if (!folio) {
+	if (xfs_buf_alloc_folio(bp, size, gfp_mask) < 0) {
 		if (size <= PAGE_SIZE)
 			return -ENOMEM;
 		trace_xfs_buf_backing_fallback(bp, _RET_IP_);
-		goto fallback;
-	}
-	bp->b_addr = folio_address(folio);
-	trace_xfs_buf_backing_folio(bp, _RET_IP_);
-	return 0;
-
-fallback:
-	for (;;) {
-		bp->b_addr = __vmalloc(size, gfp_mask);
-		if (bp->b_addr)
-			break;
-		if (flags & XBF_READ_AHEAD)
-			return -ENOMEM;
-		XFS_STATS_INC(bp->b_mount, xb_page_retries);
-		memalloc_retry_wait(gfp_mask);
+		return xfs_buf_alloc_vmalloc(bp, size, gfp_mask, flags);
 	}
-
-	trace_xfs_buf_backing_vmalloc(bp, _RET_IP_);
 	return 0;
 }
 
-- 
2.53.0

[PATCH 2/4] xfs: lift setting __GFP_NOFAIL from xfs_buf_alloc_kmem to the caller

[Christoph Hellwig]

The current __GFP_NOFAIL setting is wrong in some cases.  Prepare
for fixing that by giving control to the caller.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index d3d44e3ff001..dce398337ad0 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -145,7 +145,7 @@ xfs_buf_alloc_kmem(
 	ASSERT(is_power_of_2(size));
 	ASSERT(size < PAGE_SIZE);
 
-	bp->b_addr = kmalloc(size, gfp_mask | __GFP_NOFAIL);
+	bp->b_addr = kmalloc(size, gfp_mask);
 	if (!bp->b_addr)
 		return -ENOMEM;
 
@@ -230,7 +230,7 @@ xfs_buf_alloc_backing_mem(
 	 * smaller than PAGE_SIZE buffers used by XFS.
 	 */
 	if (size < PAGE_SIZE && is_power_of_2(size))
-		return xfs_buf_alloc_kmem(bp, size, gfp_mask);
+		return xfs_buf_alloc_kmem(bp, size, gfp_mask | __GFP_NOFAIL);
 
 	/*
 	 * Don't bother with the retry loop for single PAGE allocations: vmalloc
-- 
2.53.0

[PATCH 3/4] xfs: fix incorrect use of gfp flags in xfs_buf_alloc_backing_mem

[Christoph Hellwig]

xfs_buf_alloc_backing_mem currently has two issues with how the GFP_
flags are set:

  - when aiming for a large folio allocation, the gfp mask is adjusted
    to try less hard, but these flags then persist for the vmalloc
    allocation, which is bogus.
  - the __GFP_NOFAIL for small allocations is also applied when readahead
    force __GFP_NORETRY which doesn't make any sense.

Fix this by only applying __GFP_NOFAIL when __GFP_NORETRY is not set,
and by reordering the code so that the large folio gfp adjustments
are performed locally just for that allocation.

Fixes: 94c78cfa3bd1 ("xfs: convert buffer cache to use high order folios")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf.c | 49 +++++++++++++++++++++++-------------------------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index dce398337ad0..eea2a8757fe1 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -223,22 +223,6 @@ xfs_buf_alloc_backing_mem(
 	if (flags & XBF_READ_AHEAD)
 		gfp_mask |= __GFP_NORETRY;
 
-	/*
-	 * For buffers smaller than PAGE_SIZE use a kmalloc allocation if that
-	 * is properly aligned.  The slab allocator now guarantees an aligned
-	 * allocation for all power of two sizes, which matches most of the
-	 * smaller than PAGE_SIZE buffers used by XFS.
-	 */
-	if (size < PAGE_SIZE && is_power_of_2(size))
-		return xfs_buf_alloc_kmem(bp, size, gfp_mask | __GFP_NOFAIL);
-
-	/*
-	 * Don't bother with the retry loop for single PAGE allocations: vmalloc
-	 * won't do any better.
-	 */
-	if (size <= PAGE_SIZE)
-		gfp_mask |= __GFP_NOFAIL;
-
 	/*
 	 * Optimistically attempt a single high order folio allocation for
 	 * larger than PAGE_SIZE buffers.
@@ -251,18 +235,31 @@ xfs_buf_alloc_backing_mem(
 	 * path for them instead of wasting memory here.
 	 */
 	if (size > PAGE_SIZE) {
-		if (!is_power_of_2(size))
-			return xfs_buf_alloc_vmalloc(bp, size, gfp_mask, flags);
-		gfp_mask &= ~__GFP_DIRECT_RECLAIM;
-		gfp_mask |= __GFP_NORETRY;
-	}
-	if (xfs_buf_alloc_folio(bp, size, gfp_mask) < 0) {
-		if (size <= PAGE_SIZE)
-			return -ENOMEM;
-		trace_xfs_buf_backing_fallback(bp, _RET_IP_);
+		if (is_power_of_2(size)) {
+			gfp_t folio_gfp = gfp_mask;
+
+			folio_gfp &= ~__GFP_DIRECT_RECLAIM;
+			folio_gfp |= __GFP_NORETRY;
+			if (xfs_buf_alloc_folio(bp, size, folio_gfp) == 0)
+				return 0;
+			trace_xfs_buf_backing_fallback(bp, _RET_IP_);
+		}
 		return xfs_buf_alloc_vmalloc(bp, size, gfp_mask, flags);
 	}
-	return 0;
+
+	/*
+	 * The slab allocator now guarantees aligned allocations for all power
+	 * of two sizes.  This covers most smaller XFS buffers, so just use
+	 * kmalloc in this case.
+	 *
+	 * Don't bother with the vmalloc fallback for allocations of page size
+	 * or less: vmalloc won't do any better.
+	 */
+	if (!(gfp_mask & __GFP_NORETRY))
+		gfp_mask |= __GFP_NOFAIL;
+	if (size < PAGE_SIZE && is_power_of_2(size))
+		return xfs_buf_alloc_kmem(bp, size, gfp_mask);
+	return xfs_buf_alloc_folio(bp, size, gfp_mask);
 }
 
 static int
-- 
2.53.0

[PATCH 4/4] xfs: simplify the failure path in xfs_buf_alloc_vmalloc

[Christoph Hellwig]

Look at the __GFP_NORETRY flag set for readahead so that we don't
have to pass both the gfp_t and the flags in.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index eea2a8757fe1..1ed9f01b3275 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -168,14 +168,13 @@ static int
 xfs_buf_alloc_vmalloc(
 	struct xfs_buf		*bp,
 	size_t			size,
-	gfp_t			gfp_mask,
-	xfs_buf_flags_t		flags)
+	gfp_t			gfp_mask)
 {
 	for (;;) {
 		bp->b_addr = __vmalloc(size, gfp_mask);
 		if (bp->b_addr)
 			break;
-		if (flags & XBF_READ_AHEAD)
+		if (gfp_mask & __GFP_NORETRY)
 			return -ENOMEM;
 		XFS_STATS_INC(bp->b_mount, xb_page_retries);
 		memalloc_retry_wait(gfp_mask);
@@ -244,7 +243,7 @@ xfs_buf_alloc_backing_mem(
 				return 0;
 			trace_xfs_buf_backing_fallback(bp, _RET_IP_);
 		}
-		return xfs_buf_alloc_vmalloc(bp, size, gfp_mask, flags);
+		return xfs_buf_alloc_vmalloc(bp, size, gfp_mask);
 	}
 
 	/*
-- 
2.53.0

Re: [PATCH 1/4] xfs: split up xfs_buf_alloc_backing_mem

[Carlos Maiolino]

On Wed, Jun 17, 2026 at 07:58:02AM +0200, Christoph Hellwig wrote:
> Split out helpers for folio and vmalloc allocations to prepare for a bug
> fix.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_buf.c | 61 +++++++++++++++++++++++++++++++-----------------
>  1 file changed, 40 insertions(+), 21 deletions(-)
> 

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 0cea458f1353..d3d44e3ff001 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -120,6 +120,22 @@ xfs_buf_free(
>  	call_rcu(&bp->b_rcu, xfs_buf_free_callback);
>  }
>  
> +static int
> +xfs_buf_alloc_folio(
> +	struct xfs_buf		*bp,
> +	size_t			size,
> +	gfp_t			gfp_mask)
> +{
> +	struct folio		*folio;
> +
> +	folio = folio_alloc(gfp_mask, get_order(size));
> +	if (!folio)
> +		return -ENOMEM;
> +	bp->b_addr = folio_address(folio);
> +	trace_xfs_buf_backing_folio(bp, _RET_IP_);
> +	return 0;
> +}
> +
>  static int
>  xfs_buf_alloc_kmem(
>  	struct xfs_buf		*bp,
> @@ -148,6 +164,27 @@ xfs_buf_alloc_kmem(
>  	return 0;
>  }
>  
> +static int
> +xfs_buf_alloc_vmalloc(
> +	struct xfs_buf		*bp,
> +	size_t			size,
> +	gfp_t			gfp_mask,
> +	xfs_buf_flags_t		flags)
> +{
> +	for (;;) {
> +		bp->b_addr = __vmalloc(size, gfp_mask);
> +		if (bp->b_addr)
> +			break;
> +		if (flags & XBF_READ_AHEAD)
> +			return -ENOMEM;
> +		XFS_STATS_INC(bp->b_mount, xb_page_retries);
> +		memalloc_retry_wait(gfp_mask);
> +	}
> +
> +	trace_xfs_buf_backing_vmalloc(bp, _RET_IP_);
> +	return 0;
> +}
> +
>  /*
>   * Allocate backing memory for a buffer.
>   *
> @@ -175,7 +212,6 @@ xfs_buf_alloc_backing_mem(
>  {
>  	size_t		size = BBTOB(bp->b_length);
>  	gfp_t		gfp_mask = GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOWARN;
> -	struct folio	*folio;
>  
>  	if (xfs_buftarg_is_mem(bp->b_target))
>  		return xmbuf_map_backing_mem(bp);
> @@ -216,33 +252,16 @@ xfs_buf_alloc_backing_mem(
>  	 */
>  	if (size > PAGE_SIZE) {
>  		if (!is_power_of_2(size))
> -			goto fallback;
> +			return xfs_buf_alloc_vmalloc(bp, size, gfp_mask, flags);
>  		gfp_mask &= ~__GFP_DIRECT_RECLAIM;
>  		gfp_mask |= __GFP_NORETRY;
>  	}
> -	folio = folio_alloc(gfp_mask, get_order(size));
> -	if (!folio) {
> +	if (xfs_buf_alloc_folio(bp, size, gfp_mask) < 0) {
>  		if (size <= PAGE_SIZE)
>  			return -ENOMEM;
>  		trace_xfs_buf_backing_fallback(bp, _RET_IP_);
> -		goto fallback;
> -	}
> -	bp->b_addr = folio_address(folio);
> -	trace_xfs_buf_backing_folio(bp, _RET_IP_);
> -	return 0;
> -
> -fallback:
> -	for (;;) {
> -		bp->b_addr = __vmalloc(size, gfp_mask);
> -		if (bp->b_addr)
> -			break;
> -		if (flags & XBF_READ_AHEAD)
> -			return -ENOMEM;
> -		XFS_STATS_INC(bp->b_mount, xb_page_retries);
> -		memalloc_retry_wait(gfp_mask);
> +		return xfs_buf_alloc_vmalloc(bp, size, gfp_mask, flags);
>  	}
> -
> -	trace_xfs_buf_backing_vmalloc(bp, _RET_IP_);
>  	return 0;
>  }
>  
> -- 
> 2.53.0
> 

Re: [PATCH 2/4] xfs: lift setting __GFP_NOFAIL from xfs_buf_alloc_kmem to the caller

[Carlos Maiolino]

On Wed, Jun 17, 2026 at 07:58:03AM +0200, Christoph Hellwig wrote:
> The current __GFP_NOFAIL setting is wrong in some cases.  Prepare
> for fixing that by giving control to the caller.

Looks good, but would be nice to have at least one of those 'some cases'
documented here.

Independent on that:
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_buf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index d3d44e3ff001..dce398337ad0 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -145,7 +145,7 @@ xfs_buf_alloc_kmem(
>  	ASSERT(is_power_of_2(size));
>  	ASSERT(size < PAGE_SIZE);
>  
> -	bp->b_addr = kmalloc(size, gfp_mask | __GFP_NOFAIL);
> +	bp->b_addr = kmalloc(size, gfp_mask);
>  	if (!bp->b_addr)
>  		return -ENOMEM;
>  
> @@ -230,7 +230,7 @@ xfs_buf_alloc_backing_mem(
>  	 * smaller than PAGE_SIZE buffers used by XFS.
>  	 */
>  	if (size < PAGE_SIZE && is_power_of_2(size))
> -		return xfs_buf_alloc_kmem(bp, size, gfp_mask);
> +		return xfs_buf_alloc_kmem(bp, size, gfp_mask | __GFP_NOFAIL);
>  
>  	/*
>  	 * Don't bother with the retry loop for single PAGE allocations: vmalloc
> -- 
> 2.53.0
> 

Re: [PATCH 3/4] xfs: fix incorrect use of gfp flags in xfs_buf_alloc_backing_mem

[Carlos Maiolino]

On Wed, Jun 17, 2026 at 07:58:04AM +0200, Christoph Hellwig wrote:
> xfs_buf_alloc_backing_mem currently has two issues with how the GFP_
> flags are set:
> 
>   - when aiming for a large folio allocation, the gfp mask is adjusted
>     to try less hard, but these flags then persist for the vmalloc
>     allocation, which is bogus.
>   - the __GFP_NOFAIL for small allocations is also applied when readahead
>     force __GFP_NORETRY which doesn't make any sense.
> 
> Fix this by only applying __GFP_NOFAIL when __GFP_NORETRY is not set,
> and by reordering the code so that the large folio gfp adjustments
> are performed locally just for that allocation.

Ok, this addresses my concern on the previous patch. Ignore that.

This one looks good to.

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> 
> Fixes: 94c78cfa3bd1 ("xfs: convert buffer cache to use high order folios")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_buf.c | 49 +++++++++++++++++++++++-------------------------
>  1 file changed, 23 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index dce398337ad0..eea2a8757fe1 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -223,22 +223,6 @@ xfs_buf_alloc_backing_mem(
>  	if (flags & XBF_READ_AHEAD)
>  		gfp_mask |= __GFP_NORETRY;
>  
> -	/*
> -	 * For buffers smaller than PAGE_SIZE use a kmalloc allocation if that
> -	 * is properly aligned.  The slab allocator now guarantees an aligned
> -	 * allocation for all power of two sizes, which matches most of the
> -	 * smaller than PAGE_SIZE buffers used by XFS.
> -	 */
> -	if (size < PAGE_SIZE && is_power_of_2(size))
> -		return xfs_buf_alloc_kmem(bp, size, gfp_mask | __GFP_NOFAIL);
> -
> -	/*
> -	 * Don't bother with the retry loop for single PAGE allocations: vmalloc
> -	 * won't do any better.
> -	 */
> -	if (size <= PAGE_SIZE)
> -		gfp_mask |= __GFP_NOFAIL;
> -
>  	/*
>  	 * Optimistically attempt a single high order folio allocation for
>  	 * larger than PAGE_SIZE buffers.
> @@ -251,18 +235,31 @@ xfs_buf_alloc_backing_mem(
>  	 * path for them instead of wasting memory here.
>  	 */
>  	if (size > PAGE_SIZE) {
> -		if (!is_power_of_2(size))
> -			return xfs_buf_alloc_vmalloc(bp, size, gfp_mask, flags);
> -		gfp_mask &= ~__GFP_DIRECT_RECLAIM;
> -		gfp_mask |= __GFP_NORETRY;
> -	}
> -	if (xfs_buf_alloc_folio(bp, size, gfp_mask) < 0) {
> -		if (size <= PAGE_SIZE)
> -			return -ENOMEM;
> -		trace_xfs_buf_backing_fallback(bp, _RET_IP_);
> +		if (is_power_of_2(size)) {
> +			gfp_t folio_gfp = gfp_mask;
> +
> +			folio_gfp &= ~__GFP_DIRECT_RECLAIM;
> +			folio_gfp |= __GFP_NORETRY;
> +			if (xfs_buf_alloc_folio(bp, size, folio_gfp) == 0)
> +				return 0;
> +			trace_xfs_buf_backing_fallback(bp, _RET_IP_);
> +		}
>  		return xfs_buf_alloc_vmalloc(bp, size, gfp_mask, flags);
>  	}
> -	return 0;
> +
> +	/*
> +	 * The slab allocator now guarantees aligned allocations for all power
> +	 * of two sizes.  This covers most smaller XFS buffers, so just use
> +	 * kmalloc in this case.
> +	 *
> +	 * Don't bother with the vmalloc fallback for allocations of page size
> +	 * or less: vmalloc won't do any better.
> +	 */
> +	if (!(gfp_mask & __GFP_NORETRY))
> +		gfp_mask |= __GFP_NOFAIL;
> +	if (size < PAGE_SIZE && is_power_of_2(size))
> +		return xfs_buf_alloc_kmem(bp, size, gfp_mask);
> +	return xfs_buf_alloc_folio(bp, size, gfp_mask);
>  }
>  
>  static int
> -- 
> 2.53.0
> 
> 

Re: [PATCH 4/4] xfs: simplify the failure path in xfs_buf_alloc_vmalloc

[Carlos Maiolino]

On Wed, Jun 17, 2026 at 07:58:05AM +0200, Christoph Hellwig wrote:
> Look at the __GFP_NORETRY flag set for readahead so that we don't
> have to pass both the gfp_t and the flags in.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_buf.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index eea2a8757fe1..1ed9f01b3275 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -168,14 +168,13 @@ static int
>  xfs_buf_alloc_vmalloc(
>  	struct xfs_buf		*bp,
>  	size_t			size,
> -	gfp_t			gfp_mask,
> -	xfs_buf_flags_t		flags)
> +	gfp_t			gfp_mask)
>  {
>  	for (;;) {
>  		bp->b_addr = __vmalloc(size, gfp_mask);
>  		if (bp->b_addr)
>  			break;
> -		if (flags & XBF_READ_AHEAD)

My only concern is that looking at it now, isn't obvious why GRP_NORETRY
has been set in the first place, perhaps worth adding a comment above
it? Like:
		/* Set during readahead */
> +		if (gfp_mask & __GFP_NORETRY)
>  			return -ENOMEM;
>  		XFS_STATS_INC(bp->b_mount, xb_page_retries);
>  		memalloc_retry_wait(gfp_mask);
> @@ -244,7 +243,7 @@ xfs_buf_alloc_backing_mem(
>  				return 0;
>  			trace_xfs_buf_backing_fallback(bp, _RET_IP_);
>  		}
> -		return xfs_buf_alloc_vmalloc(bp, size, gfp_mask, flags);
> +		return xfs_buf_alloc_vmalloc(bp, size, gfp_mask);
>  	}

Other than the above caveat:
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

>  
>  	/*
> -- 
> 2.53.0
> 
> 

Re: [PATCH 4/4] xfs: simplify the failure path in xfs_buf_alloc_vmalloc

[Christoph Hellwig]

On Mon, Jun 22, 2026 at 02:34:10PM +0200, Carlos Maiolino wrote:
> >  	for (;;) {
> >  		bp->b_addr = __vmalloc(size, gfp_mask);
> >  		if (bp->b_addr)
> >  			break;
> > -		if (flags & XBF_READ_AHEAD)
> 
> My only concern is that looking at it now, isn't obvious why GRP_NORETRY
> has been set in the first place, perhaps worth adding a comment above
> it? Like:
> 		/* Set during readahead */

Maybe.  But in the end we don't really care why it was set, as it clearly
tells us not to retry, right?

Re: [PATCH 4/4] xfs: simplify the failure path in xfs_buf_alloc_vmalloc

[Carlos Maiolino]

On Wed, Jun 24, 2026 at 09:41:35AM +0200, Christoph Hellwig wrote:
> On Mon, Jun 22, 2026 at 02:34:10PM +0200, Carlos Maiolino wrote:
> > >  	for (;;) {
> > >  		bp->b_addr = __vmalloc(size, gfp_mask);
> > >  		if (bp->b_addr)
> > >  			break;
> > > -		if (flags & XBF_READ_AHEAD)
> > 
> > My only concern is that looking at it now, isn't obvious why GRP_NORETRY
> > has been set in the first place, perhaps worth adding a comment above
> > it? Like:
> > 		/* Set during readahead */
> 
> Maybe.  But in the end we don't really care why it was set, as it clearly
> tells us not to retry, right?
> 
> 

Fair enough

Re: [PATCH 1/4] xfs: split up xfs_buf_alloc_backing_mem

[Darrick J. Wong]

On Wed, Jun 17, 2026 at 07:58:02AM +0200, Christoph Hellwig wrote:
> Split out helpers for folio and vmalloc allocations to prepare for a bug
> fix.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Ok, a simple hoist; so far so good

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_buf.c | 61 +++++++++++++++++++++++++++++++-----------------
>  1 file changed, 40 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 0cea458f1353..d3d44e3ff001 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -120,6 +120,22 @@ xfs_buf_free(
>  	call_rcu(&bp->b_rcu, xfs_buf_free_callback);
>  }
>  
> +static int
> +xfs_buf_alloc_folio(
> +	struct xfs_buf		*bp,
> +	size_t			size,
> +	gfp_t			gfp_mask)
> +{
> +	struct folio		*folio;
> +
> +	folio = folio_alloc(gfp_mask, get_order(size));
> +	if (!folio)
> +		return -ENOMEM;
> +	bp->b_addr = folio_address(folio);
> +	trace_xfs_buf_backing_folio(bp, _RET_IP_);
> +	return 0;
> +}
> +
>  static int
>  xfs_buf_alloc_kmem(
>  	struct xfs_buf		*bp,
> @@ -148,6 +164,27 @@ xfs_buf_alloc_kmem(
>  	return 0;
>  }
>  
> +static int
> +xfs_buf_alloc_vmalloc(
> +	struct xfs_buf		*bp,
> +	size_t			size,
> +	gfp_t			gfp_mask,
> +	xfs_buf_flags_t		flags)
> +{
> +	for (;;) {
> +		bp->b_addr = __vmalloc(size, gfp_mask);
> +		if (bp->b_addr)
> +			break;
> +		if (flags & XBF_READ_AHEAD)
> +			return -ENOMEM;
> +		XFS_STATS_INC(bp->b_mount, xb_page_retries);
> +		memalloc_retry_wait(gfp_mask);
> +	}
> +
> +	trace_xfs_buf_backing_vmalloc(bp, _RET_IP_);
> +	return 0;
> +}
> +
>  /*
>   * Allocate backing memory for a buffer.
>   *
> @@ -175,7 +212,6 @@ xfs_buf_alloc_backing_mem(
>  {
>  	size_t		size = BBTOB(bp->b_length);
>  	gfp_t		gfp_mask = GFP_KERNEL | __GFP_NOLOCKDEP | __GFP_NOWARN;
> -	struct folio	*folio;
>  
>  	if (xfs_buftarg_is_mem(bp->b_target))
>  		return xmbuf_map_backing_mem(bp);
> @@ -216,33 +252,16 @@ xfs_buf_alloc_backing_mem(
>  	 */
>  	if (size > PAGE_SIZE) {
>  		if (!is_power_of_2(size))
> -			goto fallback;
> +			return xfs_buf_alloc_vmalloc(bp, size, gfp_mask, flags);
>  		gfp_mask &= ~__GFP_DIRECT_RECLAIM;
>  		gfp_mask |= __GFP_NORETRY;
>  	}
> -	folio = folio_alloc(gfp_mask, get_order(size));
> -	if (!folio) {
> +	if (xfs_buf_alloc_folio(bp, size, gfp_mask) < 0) {
>  		if (size <= PAGE_SIZE)
>  			return -ENOMEM;
>  		trace_xfs_buf_backing_fallback(bp, _RET_IP_);
> -		goto fallback;
> -	}
> -	bp->b_addr = folio_address(folio);
> -	trace_xfs_buf_backing_folio(bp, _RET_IP_);
> -	return 0;
> -
> -fallback:
> -	for (;;) {
> -		bp->b_addr = __vmalloc(size, gfp_mask);
> -		if (bp->b_addr)
> -			break;
> -		if (flags & XBF_READ_AHEAD)
> -			return -ENOMEM;
> -		XFS_STATS_INC(bp->b_mount, xb_page_retries);
> -		memalloc_retry_wait(gfp_mask);
> +		return xfs_buf_alloc_vmalloc(bp, size, gfp_mask, flags);
>  	}
> -
> -	trace_xfs_buf_backing_vmalloc(bp, _RET_IP_);
>  	return 0;
>  }
>  
> -- 
> 2.53.0
> 
> 

Re: [PATCH 2/4] xfs: lift setting __GFP_NOFAIL from xfs_buf_alloc_kmem to the caller

[Darrick J. Wong]

On Wed, Jun 17, 2026 at 07:58:03AM +0200, Christoph Hellwig wrote:
> The current __GFP_NOFAIL setting is wrong in some cases.  Prepare
> for fixing that by giving control to the caller.

Simple enough...

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_buf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index d3d44e3ff001..dce398337ad0 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -145,7 +145,7 @@ xfs_buf_alloc_kmem(
>  	ASSERT(is_power_of_2(size));
>  	ASSERT(size < PAGE_SIZE);
>  
> -	bp->b_addr = kmalloc(size, gfp_mask | __GFP_NOFAIL);
> +	bp->b_addr = kmalloc(size, gfp_mask);
>  	if (!bp->b_addr)
>  		return -ENOMEM;
>  
> @@ -230,7 +230,7 @@ xfs_buf_alloc_backing_mem(
>  	 * smaller than PAGE_SIZE buffers used by XFS.
>  	 */
>  	if (size < PAGE_SIZE && is_power_of_2(size))
> -		return xfs_buf_alloc_kmem(bp, size, gfp_mask);
> +		return xfs_buf_alloc_kmem(bp, size, gfp_mask | __GFP_NOFAIL);
>  
>  	/*
>  	 * Don't bother with the retry loop for single PAGE allocations: vmalloc
> -- 
> 2.53.0
> 
> 

Re: [PATCH 3/4] xfs: fix incorrect use of gfp flags in xfs_buf_alloc_backing_mem

[Darrick J. Wong]

On Wed, Jun 17, 2026 at 07:58:04AM +0200, Christoph Hellwig wrote:
> xfs_buf_alloc_backing_mem currently has two issues with how the GFP_
> flags are set:
> 
>   - when aiming for a large folio allocation, the gfp mask is adjusted
>     to try less hard, but these flags then persist for the vmalloc
>     allocation, which is bogus.
>   - the __GFP_NOFAIL for small allocations is also applied when readahead
>     force __GFP_NORETRY which doesn't make any sense.
> 
> Fix this by only applying __GFP_NOFAIL when __GFP_NORETRY is not set,
> and by reordering the code so that the large folio gfp adjustments
> are performed locally just for that allocation.
> 
> Fixes: 94c78cfa3bd1 ("xfs: convert buffer cache to use high order folios")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_buf.c | 49 +++++++++++++++++++++++-------------------------
>  1 file changed, 23 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index dce398337ad0..eea2a8757fe1 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -223,22 +223,6 @@ xfs_buf_alloc_backing_mem(
>  	if (flags & XBF_READ_AHEAD)
>  		gfp_mask |= __GFP_NORETRY;
>  
> -	/*
> -	 * For buffers smaller than PAGE_SIZE use a kmalloc allocation if that
> -	 * is properly aligned.  The slab allocator now guarantees an aligned
> -	 * allocation for all power of two sizes, which matches most of the
> -	 * smaller than PAGE_SIZE buffers used by XFS.
> -	 */
> -	if (size < PAGE_SIZE && is_power_of_2(size))
> -		return xfs_buf_alloc_kmem(bp, size, gfp_mask | __GFP_NOFAIL);
> -
> -	/*
> -	 * Don't bother with the retry loop for single PAGE allocations: vmalloc
> -	 * won't do any better.
> -	 */
> -	if (size <= PAGE_SIZE)
> -		gfp_mask |= __GFP_NOFAIL;
> -
>  	/*
>  	 * Optimistically attempt a single high order folio allocation for
>  	 * larger than PAGE_SIZE buffers.
> @@ -251,18 +235,31 @@ xfs_buf_alloc_backing_mem(
>  	 * path for them instead of wasting memory here.
>  	 */
>  	if (size > PAGE_SIZE) {
> -		if (!is_power_of_2(size))
> -			return xfs_buf_alloc_vmalloc(bp, size, gfp_mask, flags);
> -		gfp_mask &= ~__GFP_DIRECT_RECLAIM;
> -		gfp_mask |= __GFP_NORETRY;
> -	}
> -	if (xfs_buf_alloc_folio(bp, size, gfp_mask) < 0) {
> -		if (size <= PAGE_SIZE)
> -			return -ENOMEM;
> -		trace_xfs_buf_backing_fallback(bp, _RET_IP_);
> +		if (is_power_of_2(size)) {
> +			gfp_t folio_gfp = gfp_mask;
> +
> +			folio_gfp &= ~__GFP_DIRECT_RECLAIM;
> +			folio_gfp |= __GFP_NORETRY;
> +			if (xfs_buf_alloc_folio(bp, size, folio_gfp) == 0)
> +				return 0;
> +			trace_xfs_buf_backing_fallback(bp, _RET_IP_);
> +		}
>  		return xfs_buf_alloc_vmalloc(bp, size, gfp_mask, flags);
>  	}
> -	return 0;
> +
> +	/*
> +	 * The slab allocator now guarantees aligned allocations for all power
> +	 * of two sizes.  This covers most smaller XFS buffers, so just use
> +	 * kmalloc in this case.
> +	 *
> +	 * Don't bother with the vmalloc fallback for allocations of page size
> +	 * or less: vmalloc won't do any better.
> +	 */
> +	if (!(gfp_mask & __GFP_NORETRY))
> +		gfp_mask |= __GFP_NOFAIL;
> +	if (size < PAGE_SIZE && is_power_of_2(size))
> +		return xfs_buf_alloc_kmem(bp, size, gfp_mask);
> +	return xfs_buf_alloc_folio(bp, size, gfp_mask);

Hrmm, ok.  So first we special-case buffers > PAGE_SIZE -- if they're a
power of two, we try (not very hard) to allocate a single large folio.
If that fails or it's not a power-of-two, then we just do vmalloc, which
allows direct reclaim and retries.

For smaller than PAGE_SIZE buffers that are powers of two, I guess we
use the slab allocator, otherwise a full folio.  That part strikes me as
a little strange (efficiency, I guess?), but that's what the code did
before so I guess it's ok.

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

>  }
>  
>  static int
> -- 
> 2.53.0
> 
> 

Re: [PATCH 4/4] xfs: simplify the failure path in xfs_buf_alloc_vmalloc

[Darrick J. Wong]

On Mon, Jun 22, 2026 at 02:34:10PM +0200, Carlos Maiolino wrote:
> On Wed, Jun 17, 2026 at 07:58:05AM +0200, Christoph Hellwig wrote:
> > Look at the __GFP_NORETRY flag set for readahead so that we don't
> > have to pass both the gfp_t and the flags in.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  fs/xfs/xfs_buf.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> > index eea2a8757fe1..1ed9f01b3275 100644
> > --- a/fs/xfs/xfs_buf.c
> > +++ b/fs/xfs/xfs_buf.c
> > @@ -168,14 +168,13 @@ static int
> >  xfs_buf_alloc_vmalloc(
> >  	struct xfs_buf		*bp,
> >  	size_t			size,
> > -	gfp_t			gfp_mask,
> > -	xfs_buf_flags_t		flags)
> > +	gfp_t			gfp_mask)
> >  {
> >  	for (;;) {
> >  		bp->b_addr = __vmalloc(size, gfp_mask);
> >  		if (bp->b_addr)
> >  			break;
> > -		if (flags & XBF_READ_AHEAD)
> 
> My only concern is that looking at it now, isn't obvious why GRP_NORETRY
> has been set in the first place, perhaps worth adding a comment above
> it? Like:
> 		/* Set during readahead */

I think it's fine as-is, because the state of being in readahead is
something that the caller of xfs_buf_alloc_vmalloc knows, and doesn't
need to share with xfs_buf_alloc_vmalloc itself.

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D


> > +		if (gfp_mask & __GFP_NORETRY)
> >  			return -ENOMEM;
> >  		XFS_STATS_INC(bp->b_mount, xb_page_retries);
> >  		memalloc_retry_wait(gfp_mask);
> > @@ -244,7 +243,7 @@ xfs_buf_alloc_backing_mem(
> >  				return 0;
> >  			trace_xfs_buf_backing_fallback(bp, _RET_IP_);
> >  		}
> > -		return xfs_buf_alloc_vmalloc(bp, size, gfp_mask, flags);
> > +		return xfs_buf_alloc_vmalloc(bp, size, gfp_mask);
> >  	}
> 
> Other than the above caveat:
> Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
> 
> >  
> >  	/*
> > -- 
> > 2.53.0
> > 
> > 
> 

Re: [PATCH 3/4] xfs: fix incorrect use of gfp flags in xfs_buf_alloc_backing_mem

[Christoph Hellwig]

On Mon, Jun 29, 2026 at 06:49:35PM -0700, Darrick J. Wong wrote:
> > +	return xfs_buf_alloc_folio(bp, size, gfp_mask);
> 
> Hrmm, ok.  So first we special-case buffers > PAGE_SIZE -- if they're a
> power of two, we try (not very hard) to allocate a single large folio.
> If that fails or it's not a power-of-two, then we just do vmalloc, which
> allows direct reclaim and retries.

Yes.

> For smaller than PAGE_SIZE buffers that are powers of two, I guess we
> use the slab allocator, otherwise a full folio.  That part strikes me as
> a little strange (efficiency, I guess?), but that's what the code did
> before so I guess it's ok.

That's because slab doesn't guarantee alignent for non-power-of two
allocations, and we need that for the block layer.  There is a comment
in the code explaining that.

Re: [PATCH 2/4] xfs: lift setting __GFP_NOFAIL from xfs_buf_alloc_kmem to the caller

[Carlos Maiolino]

On Mon, Jun 29, 2026 at 05:52:54PM -0700, Darrick J. Wong wrote:
> On Wed, Jun 17, 2026 at 07:58:03AM +0200, Christoph Hellwig wrote:
> > The current __GFP_NOFAIL setting is wrong in some cases.  Prepare
> > for fixing that by giving control to the caller.
> 
> Simple enough...

/me interprets it as a RwB

> 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  fs/xfs/xfs_buf.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> > index d3d44e3ff001..dce398337ad0 100644
> > --- a/fs/xfs/xfs_buf.c
> > +++ b/fs/xfs/xfs_buf.c
> > @@ -145,7 +145,7 @@ xfs_buf_alloc_kmem(
> >  	ASSERT(is_power_of_2(size));
> >  	ASSERT(size < PAGE_SIZE);
> >  
> > -	bp->b_addr = kmalloc(size, gfp_mask | __GFP_NOFAIL);
> > +	bp->b_addr = kmalloc(size, gfp_mask);
> >  	if (!bp->b_addr)
> >  		return -ENOMEM;
> >  
> > @@ -230,7 +230,7 @@ xfs_buf_alloc_backing_mem(
> >  	 * smaller than PAGE_SIZE buffers used by XFS.
> >  	 */
> >  	if (size < PAGE_SIZE && is_power_of_2(size))
> > -		return xfs_buf_alloc_kmem(bp, size, gfp_mask);
> > +		return xfs_buf_alloc_kmem(bp, size, gfp_mask | __GFP_NOFAIL);
> >  
> >  	/*
> >  	 * Don't bother with the retry loop for single PAGE allocations: vmalloc
> > -- 
> > 2.53.0
> > 
> > 
> 

Re: fix GFP_ flag use in the buffer cache

[Carlos Maiolino]

On Wed, 17 Jun 2026 07:58:01 +0200, Christoph Hellwig wrote:
> usage of the GFP_ flags in the buffer cache, including mixing
> __GFP_NOFAIL and __GFP_NORETRY which is nonsensical, and passing
> __GFP_NORETRY to the dedicated fallback.
> 
> Diffstat:
>  xfs_buf.c |  105 +++++++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 60 insertions(+), 45 deletions(-)
> 
> [...]

Applied to for-next, thanks!

[1/4] xfs: split up xfs_buf_alloc_backing_mem
      commit: c752838874578b7f0267d9f3e73892b1c7a0d9cc
[2/4] xfs: lift setting __GFP_NOFAIL from xfs_buf_alloc_kmem to the caller
      commit: 27d7d15184d701b6e4db2b7431e4cee67a041a1c
[3/4] xfs: fix incorrect use of gfp flags in xfs_buf_alloc_backing_mem
      commit: 0144bcf619602e82843a514431d3cf2cd13ec08a
[4/4] xfs: simplify the failure path in xfs_buf_alloc_vmalloc
      commit: 19dc95d4b6cf81e1878a3abd587afc967b75d5ce

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>

Re: [PATCH 2/4] xfs: lift setting __GFP_NOFAIL from xfs_buf_alloc_kmem to the caller

[Darrick J. Wong]

On Wed, Jul 01, 2026 at 11:31:27AM +0200, Carlos Maiolino wrote:
> On Mon, Jun 29, 2026 at 05:52:54PM -0700, Darrick J. Wong wrote:
> > On Wed, Jun 17, 2026 at 07:58:03AM +0200, Christoph Hellwig wrote:
> > > The current __GFP_NOFAIL setting is wrong in some cases.  Prepare
> > > for fixing that by giving control to the caller.
> > 
> > Simple enough...
> 
> /me interprets it as a RwB

Yeah.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> 
> > 
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > >  fs/xfs/xfs_buf.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> > > index d3d44e3ff001..dce398337ad0 100644
> > > --- a/fs/xfs/xfs_buf.c
> > > +++ b/fs/xfs/xfs_buf.c
> > > @@ -145,7 +145,7 @@ xfs_buf_alloc_kmem(
> > >  	ASSERT(is_power_of_2(size));
> > >  	ASSERT(size < PAGE_SIZE);
> > >  
> > > -	bp->b_addr = kmalloc(size, gfp_mask | __GFP_NOFAIL);
> > > +	bp->b_addr = kmalloc(size, gfp_mask);
> > >  	if (!bp->b_addr)
> > >  		return -ENOMEM;
> > >  
> > > @@ -230,7 +230,7 @@ xfs_buf_alloc_backing_mem(
> > >  	 * smaller than PAGE_SIZE buffers used by XFS.
> > >  	 */
> > >  	if (size < PAGE_SIZE && is_power_of_2(size))
> > > -		return xfs_buf_alloc_kmem(bp, size, gfp_mask);
> > > +		return xfs_buf_alloc_kmem(bp, size, gfp_mask | __GFP_NOFAIL);
> > >  
> > >  	/*
> > >  	 * Don't bother with the retry loop for single PAGE allocations: vmalloc
> > > -- 
> > > 2.53.0
> > > 
> > > 
> >