Re: [PATCH 1/3] slab, block: generalize bvec_alloc_gfp

[Vlastimil Babka <vbabka@suse.cz>]

On 10/23/25 10:08, Christoph Hellwig wrote:
> bvec_alloc_gfp is useful for any place that tries to kmalloc first and
> then fall back to a mempool.  Rename it and move it to blk.h to prepare

I wonder if such fall backs are necessary because IIRC mempools try to
allocate from the underlying provider (i.e. kmalloc caches first), and only
give out the reserves when that fails. Is it done for less overhead or
something?

> for using it to allocate the default integrity buffer.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

That says blk.h but you move it to slab.h? Assuming you intended slab.h.
However gfp flags are not slab only so it should be rather
include/linux/gfp.h - added maintainers of that to Cc.

We do have gfp_nested_mask() there which is quite similar but not exactly.
Maybe a canonical macro not for nested, but for opportunistic allocations
(if a one size fits all solution can be found) would be useful too, as
people indeed reinvent those manually in various places with subtle differences.

> ---
>  block/bio.c          | 13 ++-----------
>  include/linux/slab.h | 10 ++++++++++
>  2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index b3a79285c278..4ea5833a7637 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -169,16 +169,6 @@ void bvec_free(mempool_t *pool, struct bio_vec *bv, unsigned short nr_vecs)
>  		kmem_cache_free(biovec_slab(nr_vecs)->slab, bv);
>  }
>  
> -/*
> - * Make the first allocation restricted and don't dump info on allocation
> - * failures, since we'll fall back to the mempool in case of failure.
> - */
> -static inline gfp_t bvec_alloc_gfp(gfp_t gfp)
> -{
> -	return (gfp & ~(__GFP_DIRECT_RECLAIM | __GFP_IO)) |
> -		__GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN;
> -}
> -
>  struct bio_vec *bvec_alloc(mempool_t *pool, unsigned short *nr_vecs,
>  		gfp_t gfp_mask)
>  {
> @@ -201,7 +191,8 @@ struct bio_vec *bvec_alloc(mempool_t *pool, unsigned short *nr_vecs,
>  	if (*nr_vecs < BIO_MAX_VECS) {
>  		struct bio_vec *bvl;
>  
> -		bvl = kmem_cache_alloc(bvs->slab, bvec_alloc_gfp(gfp_mask));
> +		bvl = kmem_cache_alloc(bvs->slab,
> +				try_alloc_gfp(gfp_mask & ~__GFP_IO));
>  		if (likely(bvl) || !(gfp_mask & __GFP_DIRECT_RECLAIM))
>  			return bvl;
>  		*nr_vecs = BIO_MAX_VECS;
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index d5a8ab98035c..a6672cead03e 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -1113,6 +1113,16 @@ void kfree_rcu_scheduler_running(void);
>   */
>  size_t kmalloc_size_roundup(size_t size);
>  
> +/*
> + * Make the first allocation restricted and don't dump info on allocation
> + * failures, for callers that will fall back to a mempool in case of failure.
> + */
> +static inline gfp_t try_alloc_gfp(gfp_t gfp)
> +{
> +	return (gfp & ~__GFP_DIRECT_RECLAIM) |
> +		__GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN;

Note without __GFP_DIRECT_RECLAIM the __GFP_NORETRY is most likely redundant
(but doesn't hurt).

> +}
> +
>  void __init kmem_cache_init_late(void);
>  void __init kvfree_rcu_init(void);
>