Linux XFS filesystem development
 help / color / mirror / Atom feed
* fix GFP_ flag use in the buffer cache
@ 2026-06-17  5:58 Christoph Hellwig
  2026-06-17  5:58 ` [PATCH 1/4] xfs: split up xfs_buf_alloc_backing_mem Christoph Hellwig
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Christoph Hellwig @ 2026-06-17  5:58 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: linux-xfs

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(-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/4] xfs: split up xfs_buf_alloc_backing_mem
  2026-06-17  5:58 fix GFP_ flag use in the buffer cache Christoph Hellwig
@ 2026-06-17  5:58 ` Christoph Hellwig
  2026-06-22 12:17   ` Carlos Maiolino
  2026-06-17  5:58 ` [PATCH 2/4] xfs: lift setting __GFP_NOFAIL from xfs_buf_alloc_kmem to the caller Christoph Hellwig
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2026-06-17  5:58 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: linux-xfs

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] xfs: lift setting __GFP_NOFAIL from xfs_buf_alloc_kmem to the caller
  2026-06-17  5:58 fix GFP_ flag use in the buffer cache Christoph Hellwig
  2026-06-17  5:58 ` [PATCH 1/4] xfs: split up xfs_buf_alloc_backing_mem Christoph Hellwig
@ 2026-06-17  5:58 ` Christoph Hellwig
  2026-06-22 12:22   ` Carlos Maiolino
  2026-06-17  5:58 ` [PATCH 3/4] xfs: fix incorrect use of gfp flags in xfs_buf_alloc_backing_mem Christoph Hellwig
  2026-06-17  5:58 ` [PATCH 4/4] xfs: simplify the failure path in xfs_buf_alloc_vmalloc Christoph Hellwig
  3 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2026-06-17  5:58 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: linux-xfs

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] xfs: fix incorrect use of gfp flags in xfs_buf_alloc_backing_mem
  2026-06-17  5:58 fix GFP_ flag use in the buffer cache Christoph Hellwig
  2026-06-17  5:58 ` [PATCH 1/4] xfs: split up xfs_buf_alloc_backing_mem Christoph Hellwig
  2026-06-17  5:58 ` [PATCH 2/4] xfs: lift setting __GFP_NOFAIL from xfs_buf_alloc_kmem to the caller Christoph Hellwig
@ 2026-06-17  5:58 ` Christoph Hellwig
  2026-06-22 12:31   ` Carlos Maiolino
  2026-06-17  5:58 ` [PATCH 4/4] xfs: simplify the failure path in xfs_buf_alloc_vmalloc Christoph Hellwig
  3 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2026-06-17  5:58 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: linux-xfs

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] xfs: simplify the failure path in xfs_buf_alloc_vmalloc
  2026-06-17  5:58 fix GFP_ flag use in the buffer cache Christoph Hellwig
                   ` (2 preceding siblings ...)
  2026-06-17  5:58 ` [PATCH 3/4] xfs: fix incorrect use of gfp flags in xfs_buf_alloc_backing_mem Christoph Hellwig
@ 2026-06-17  5:58 ` Christoph Hellwig
  2026-06-22 12:34   ` Carlos Maiolino
  3 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2026-06-17  5:58 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: linux-xfs

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4] xfs: split up xfs_buf_alloc_backing_mem
  2026-06-17  5:58 ` [PATCH 1/4] xfs: split up xfs_buf_alloc_backing_mem Christoph Hellwig
@ 2026-06-22 12:17   ` Carlos Maiolino
  0 siblings, 0 replies; 11+ messages in thread
From: Carlos Maiolino @ 2026-06-22 12:17 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

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
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] xfs: lift setting __GFP_NOFAIL from xfs_buf_alloc_kmem to the caller
  2026-06-17  5:58 ` [PATCH 2/4] xfs: lift setting __GFP_NOFAIL from xfs_buf_alloc_kmem to the caller Christoph Hellwig
@ 2026-06-22 12:22   ` Carlos Maiolino
  0 siblings, 0 replies; 11+ messages in thread
From: Carlos Maiolino @ 2026-06-22 12:22 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

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
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] xfs: fix incorrect use of gfp flags in xfs_buf_alloc_backing_mem
  2026-06-17  5:58 ` [PATCH 3/4] xfs: fix incorrect use of gfp flags in xfs_buf_alloc_backing_mem Christoph Hellwig
@ 2026-06-22 12:31   ` Carlos Maiolino
  0 siblings, 0 replies; 11+ messages in thread
From: Carlos Maiolino @ 2026-06-22 12:31 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

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
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] xfs: simplify the failure path in xfs_buf_alloc_vmalloc
  2026-06-17  5:58 ` [PATCH 4/4] xfs: simplify the failure path in xfs_buf_alloc_vmalloc Christoph Hellwig
@ 2026-06-22 12:34   ` Carlos Maiolino
  2026-06-24  7:41     ` Christoph Hellwig
  0 siblings, 1 reply; 11+ messages in thread
From: Carlos Maiolino @ 2026-06-22 12:34 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

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
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] xfs: simplify the failure path in xfs_buf_alloc_vmalloc
  2026-06-22 12:34   ` Carlos Maiolino
@ 2026-06-24  7:41     ` Christoph Hellwig
  2026-06-24  8:25       ` Carlos Maiolino
  0 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2026-06-24  7:41 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: Christoph Hellwig, linux-xfs

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?


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] xfs: simplify the failure path in xfs_buf_alloc_vmalloc
  2026-06-24  7:41     ` Christoph Hellwig
@ 2026-06-24  8:25       ` Carlos Maiolino
  0 siblings, 0 replies; 11+ messages in thread
From: Carlos Maiolino @ 2026-06-24  8:25 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-06-24  8:25 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-17  5:58 fix GFP_ flag use in the buffer cache Christoph Hellwig
2026-06-17  5:58 ` [PATCH 1/4] xfs: split up xfs_buf_alloc_backing_mem Christoph Hellwig
2026-06-22 12:17   ` Carlos Maiolino
2026-06-17  5:58 ` [PATCH 2/4] xfs: lift setting __GFP_NOFAIL from xfs_buf_alloc_kmem to the caller Christoph Hellwig
2026-06-22 12:22   ` Carlos Maiolino
2026-06-17  5:58 ` [PATCH 3/4] xfs: fix incorrect use of gfp flags in xfs_buf_alloc_backing_mem Christoph Hellwig
2026-06-22 12:31   ` Carlos Maiolino
2026-06-17  5:58 ` [PATCH 4/4] xfs: simplify the failure path in xfs_buf_alloc_vmalloc Christoph Hellwig
2026-06-22 12:34   ` Carlos Maiolino
2026-06-24  7:41     ` Christoph Hellwig
2026-06-24  8:25       ` Carlos Maiolino

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox