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

["Darrick J. Wong" <djwong@kernel.org>]

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