Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk
Jesper Dangaard Brouer <[email protected]>
| Newsgroups | org.kernel.vger.io-uring,org.freedesktop.lists.dri-devel,org.kernel.vger.bpf,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel,org.kernel.vger.netdev,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
On 27/05/2026 09.02, Christoph Hellwig wrote: > The kmem_cache_alloc_bulk return value is weird. It returns the number > of allocated objects, but that must always be 0 or the requested number > based on the implementations and the handling in the callers, but that > assumption is not actually documented anywhere, which confuses automated > review tools. > I remember, this API behavior was requested by AKPM when I developed kmem_cache_alloc_bulk. I trusted AKPM's decision, but I cannot explain why this choice was made. I kept the netdev code usage below. The current napi_skb_cache_get_bulk have a retry logic that assumes that a partial bulk number can be returned (which it cannot as Hellwig explains). Cc Alex/Olek please review the changes below as you added this retry logic. > Fix this by returning a bool if the allocation succeeded and adding a > kerneldoc comment explaining the API. > > Signed-off-by: Christoph Hellwig <[email protected]> > --- > drivers/gpu/drm/msm/msm_iommu.c | 6 +-- > drivers/gpu/drm/panthor/panthor_mmu.c | 12 +++--- > include/linux/slab.h | 6 ++- > io_uring/io_uring.c | 23 +++++------ > lib/test_meminit.c | 19 +++++---- > mm/kasan/kasan_test_c.c | 5 +-- > mm/kfence/kfence_test.c | 9 +++-- > mm/slub.c | 58 +++++++++++++++------------ > net/bpf/test_run.c | 7 ++-- > net/core/skbuff.c | 24 ++++++----- > tools/include/linux/slab.h | 2 +- > tools/testing/shared/linux.c | 19 ++++----- > 12 files changed, 93 insertions(+), 97 deletions(-) > [...] > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 44ac121cfccb..73045b688385 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -288,11 +288,11 @@ static inline struct sk_buff *napi_skb_cache_get(bool alloc) > > local_lock_nested_bh(&napi_alloc_cache.bh_lock); > if (unlikely(!nc->skb_count)) { > - if (alloc) > - nc->skb_count = kmem_cache_alloc_bulk(net_hotdata.skbuff_cache, > - GFP_ATOMIC | __GFP_NOWARN, > - NAPI_SKB_CACHE_BULK, > - nc->skb_cache); > + if (alloc && kmem_cache_alloc_bulk(net_hotdata.skbuff_cache, > + GFP_ATOMIC | __GFP_NOWARN, > + NAPI_SKB_CACHE_BULK, > + nc->skb_cache)) > + nc->skb_count = NAPI_SKB_CACHE_BULK; > if (unlikely(!nc->skb_count)) { > local_unlock_nested_bh(&napi_alloc_cache.bh_lock); > return NULL; > @@ -353,16 +353,18 @@ u32 napi_skb_cache_get_bulk(void **skbs, u32 n) > > /* No enough cached skbs. Try refilling the cache first */ > bulk = min(NAPI_SKB_CACHE_SIZE - nc->skb_count, NAPI_SKB_CACHE_BULK); > - nc->skb_count += kmem_cache_alloc_bulk(net_hotdata.skbuff_cache, > - GFP_ATOMIC | __GFP_NOWARN, bulk, > - &nc->skb_cache[nc->skb_count]); > + if (kmem_cache_alloc_bulk(net_hotdata.skbuff_cache, > + GFP_ATOMIC | __GFP_NOWARN, bulk, > + &nc->skb_cache[nc->skb_count])) > + nc->skb_count += bulk; > if (likely(nc->skb_count >= n)) > goto get; > > /* Still not enough. Bulk-allocate the missing part directly, zeroed */ > - n -= kmem_cache_alloc_bulk(net_hotdata.skbuff_cache, > - GFP_ATOMIC | __GFP_ZERO | __GFP_NOWARN, > - n - nc->skb_count, &skbs[nc->skb_count]); > + if (kmem_cache_alloc_bulk(net_hotdata.skbuff_cache, > + GFP_ATOMIC | __GFP_ZERO | __GFP_NOWARN, > + n - nc->skb_count, &skbs[nc->skb_count])) > + n = nc->skb_count; > if (likely(nc->skb_count >= n)) > goto get; >