improve the kmem_cache_alloc_bulk API v2

[Christoph Hellwig]

Hi all,

kmem_cache_alloc_bulk has a very unintuitive and undocumented return
value convention. Fix that and add documentation.

Changes since v1:
- fix compile in drm and test_meminit
- document that 0-sized allocations fail

Diffstat:
drivers/gpu/drm/msm/msm_iommu.c | 6 +--
drivers/gpu/drm/panthor/panthor_mmu.c | 13 +++----
include/linux/slab.h | 6 ++-
io_uring/io_uring.c | 23 +++++--------
lib/test_meminit.c | 23 ++++++-------
mm/kasan/kasan_test_c.c | 5 +-
mm/kfence/kfence_test.c | 9 ++---
mm/slub.c | 59 +++++++++++++++++++---------------
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, 97 insertions(+), 99 deletions(-)

[Christoph Hellwig]

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.

Fix this by returning a bool if the allocation succeeded and adding a
kerneldoc comment explaining the API.

Signed-off-by: Christoph Hellwig <h...@lst.de>
Reviewed-by: Alexander Lobakin <aleksande...@intel.com> # skbuff
---

drivers/gpu/drm/msm/msm_iommu.c | 6 +--
drivers/gpu/drm/panthor/panthor_mmu.c | 13 +++---

include/linux/slab.h | 6 ++-
io_uring/io_uring.c | 23 ++++-------
lib/test_meminit.c | 23 +++++------
mm/kasan/kasan_test_c.c | 5 +--
mm/kfence/kfence_test.c | 9 ++--
mm/slub.c | 59 +++++++++++++++------------
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, 97 insertions(+), 99 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index 058c71c82cf5..533104d71f6c 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -330,17 +330,15 @@ static int
msm_iommu_pagetable_prealloc_allocate(struct msm_mmu *mmu, struct msm_mmu_prealloc *p)
{
struct kmem_cache *pt_cache = get_pt_cache(mmu);
- int ret;

p->pages = kvmalloc_objs(*p->pages, p->count);
if (!p->pages)
return -ENOMEM;

- ret = kmem_cache_alloc_bulk(pt_cache, GFP_KERNEL, p->count, p->pages);
- if (ret != p->count) {
+ if (!kmem_cache_alloc_bulk(pt_cache, GFP_KERNEL, p->count, p->pages)) {
kfree(p->pages);
p->pages = NULL;
- p->count = ret;
+ p->count = 0;
return -ENOMEM;
}

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 75d98dad7b1d..b12c641af46c 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -1274,13 +1274,13 @@ static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx,
goto err_cleanup;
}

- ret = kmem_cache_alloc_bulk(pt_cache, GFP_KERNEL, pt_count,
- op_ctx->rsvd_page_tables.pages);
- op_ctx->rsvd_page_tables.count = ret;
- if (ret != pt_count) {
+ if (!kmem_cache_alloc_bulk(pt_cache, GFP_KERNEL, pt_count,
+ op_ctx->rsvd_page_tables.pages)) {
+ op_ctx->rsvd_page_tables.count = 0;
ret = -ENOMEM;
goto err_cleanup;
}
+ op_ctx->rsvd_page_tables.count = pt_count;

/* Insert BO into the extobj list last, when we know nothing can fail. */
dma_resv_lock(panthor_vm_resv(vm), NULL);
@@ -1328,9 +1328,8 @@ static int panthor_vm_prepare_unmap_op_ctx(struct panthor_vm_op_ctx *op_ctx,
goto err_cleanup;
}

- ret = kmem_cache_alloc_bulk(pt_cache, GFP_KERNEL, pt_count,
- op_ctx->rsvd_page_tables.pages);
- if (ret != pt_count) {
+ if (!kmem_cache_alloc_bulk(pt_cache, GFP_KERNEL, pt_count,
+ op_ctx->rsvd_page_tables.pages)) {
ret = -ENOMEM;
goto err_cleanup;
}
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 2b5ab488e96b..6a7b452d43a0 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -815,8 +815,10 @@ kmem_buckets *kmem_buckets_create(const char *name, slab_flags_t flags,
*/
void kmem_cache_free_bulk(struct kmem_cache *s, size_t size, void **p);

-int kmem_cache_alloc_bulk_noprof(struct kmem_cache *s, gfp_t flags, size_t size, void **p);
-#define kmem_cache_alloc_bulk(...) alloc_hooks(kmem_cache_alloc_bulk_noprof(__VA_ARGS__))
+bool kmem_cache_alloc_bulk_noprof(struct kmem_cache *s, gfp_t flags,
+ size_t size, void **p);
+#define kmem_cache_alloc_bulk(...) \
+ alloc_hooks(kmem_cache_alloc_bulk_noprof(__VA_ARGS__))

static __always_inline void kfree_bulk(size_t size, void **p)
{
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 103b6c88f252..bf847ca823f7 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -978,29 +978,24 @@ __cold bool __io_alloc_req_refill(struct io_ring_ctx *ctx)
{
gfp_t gfp = GFP_KERNEL | __GFP_NOWARN | __GFP_ZERO;
void *reqs[IO_REQ_ALLOC_BATCH];
- int ret;
-
- ret = kmem_cache_alloc_bulk(req_cachep, gfp, ARRAY_SIZE(reqs), reqs);
+ int nr_reqs = ARRAY_SIZE(reqs);

/*
- * Bulk alloc is all-or-nothing. If we fail to get a batch,
- * retry single alloc to be on the safe side.
+ * Bulk alloc is all-or-nothing. If we fail to get a batch, retry a
+ * single allocation to be on the safe side.
*/
- if (unlikely(ret <= 0)) {
+ if (!kmem_cache_alloc_bulk(req_cachep, gfp, nr_reqs, reqs)) {
reqs[0] = kmem_cache_alloc(req_cachep, gfp);
if (!reqs[0])
return false;
- ret = 1;
+ nr_reqs = 1;
}

- percpu_ref_get_many(&ctx->refs, ret);
- ctx->nr_req_allocated += ret;
-
- while (ret--) {
- struct io_kiocb *req = reqs[ret];
+ percpu_ref_get_many(&ctx->refs, nr_reqs);
+ ctx->nr_req_allocated += nr_reqs;

- io_req_add_to_cache(req, ctx);
- }
+ while (nr_reqs--)
+ io_req_add_to_cache(reqs[nr_reqs], ctx);
return true;
}

diff --git a/lib/test_meminit.c b/lib/test_meminit.c
index d028a6552cd6..68c3b9da090e 100644
--- a/lib/test_meminit.c
+++ b/lib/test_meminit.c
@@ -229,16 +229,14 @@ static int __init do_kmem_cache_size(size_t size, bool want_ctor,
for (iter = 0; iter < 10; iter++) {
/* Do a test of bulk allocations */
if (!want_rcu && !want_ctor) {
- int ret;
-
- ret = kmem_cache_alloc_bulk(c, alloc_mask, BULK_SIZE, bulk_array);
- if (!ret) {
+ if (!kmem_cache_alloc_bulk(c, alloc_mask, BULK_SIZE,
+ bulk_array)) {
fail = true;
} else {
int i;
- for (i = 0; i < ret; i++)
+ for (i = 0; i < BULK_SIZE; i++)
fail |= check_buf(bulk_array[i], size, want_ctor, want_rcu, want_zero);
- kmem_cache_free_bulk(c, ret, bulk_array);
+ kmem_cache_free_bulk(c, BULK_SIZE, bulk_array);
}
}

@@ -348,23 +346,24 @@ static int __init do_kmem_cache_size_bulk(int size, int *total_failures)
{
struct kmem_cache *c;
int i, iter, maxiter = 1024;
- int num, bytes;
+ int bytes;
bool fail = false;
void *objects[10];

c = kmem_cache_create("test_cache", size, size, 0, NULL);
for (iter = 0; (iter < maxiter) && !fail; iter++) {
- num = kmem_cache_alloc_bulk(c, GFP_KERNEL, ARRAY_SIZE(objects),
- objects);
- for (i = 0; i < num; i++) {
+ if (!kmem_cache_alloc_bulk(c, GFP_KERNEL, ARRAY_SIZE(objects),
+ objects))
+ continue;
+
+ for (i = 0; i < ARRAY_SIZE(objects); i++) {
bytes = count_nonzero_bytes(objects[i], size);
if (bytes)
fail = true;
fill_with_garbage(objects[i], size);
}

- if (num)
- kmem_cache_free_bulk(c, num, objects);
+ kmem_cache_free_bulk(c, ARRAY_SIZE(objects), objects);
}
kmem_cache_destroy(c);
*total_failures += fail;
diff --git a/mm/kasan/kasan_test_c.c b/mm/kasan/kasan_test_c.c
index 3f4ed29178b3..b9e167ed5be3 100644
--- a/mm/kasan/kasan_test_c.c
+++ b/mm/kasan/kasan_test_c.c
@@ -1225,14 +1225,13 @@ static void kmem_cache_bulk(struct kunit *test)
struct kmem_cache *cache;
size_t size = 200;
char *p[10];
- bool ret;
int i;

cache = kmem_cache_create("test_cache", size, 0, 0, NULL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, cache);

- ret = kmem_cache_alloc_bulk(cache, GFP_KERNEL, ARRAY_SIZE(p), (void **)&p);
- if (!ret) {
+ if (!kmem_cache_alloc_bulk(cache, GFP_KERNEL, ARRAY_SIZE(p),
+ (void **)&p)) {
kunit_err(test, "Allocation failed: %s\n", __func__);
kmem_cache_destroy(cache);
return;
diff --git a/mm/kfence/kfence_test.c b/mm/kfence/kfence_test.c
index 10424cd25e5a..c472e66e7242 100644
--- a/mm/kfence/kfence_test.c
+++ b/mm/kfence/kfence_test.c
@@ -761,9 +761,10 @@ static void test_memcache_alloc_bulk(struct kunit *test)
timeout = jiffies + msecs_to_jiffies(100 * kfence_sample_interval);
do {
void *objects[100];
- int i, num = kmem_cache_alloc_bulk(test_cache, GFP_ATOMIC, ARRAY_SIZE(objects),
- objects);
- if (!num)
+ int i;
+
+ if (!kmem_cache_alloc_bulk(test_cache, GFP_ATOMIC,
+ ARRAY_SIZE(objects), objects))
continue;
for (i = 0; i < ARRAY_SIZE(objects); i++) {
if (is_kfence_address(objects[i])) {
@@ -771,7 +772,7 @@ static void test_memcache_alloc_bulk(struct kunit *test)
break;
}
}
- kmem_cache_free_bulk(test_cache, num, objects);
+ kmem_cache_free_bulk(test_cache, ARRAY_SIZE(objects), objects);
/*
* kmem_cache_alloc_bulk() disables interrupts, and calling it
* in a tight loop may not give KFENCE a chance to switch the
diff --git a/mm/slub.c b/mm/slub.c
index a2bf3756ca7d..57b2af707e95 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4981,8 +4981,8 @@ static int __prefill_sheaf_pfmemalloc(struct kmem_cache *s,
return ret;
}

-static int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags,
- size_t size, void **p);
+static bool __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags,
+ size_t size, void **p);

/*
* returns a sheaf that has at least the requested size
@@ -5154,9 +5154,8 @@ int kmem_cache_refill_sheaf(struct kmem_cache *s, gfp_t gfp,
return __prefill_sheaf_pfmemalloc(s, sheaf, gfp);

if (!__kmem_cache_alloc_bulk(s, gfp, sheaf->capacity - sheaf->size,
- &sheaf->objects[sheaf->size])) {
+ &sheaf->objects[sheaf->size]))
return -ENOMEM;
- }
sheaf->size = sheaf->capacity;

return 0;
@@ -7289,9 +7288,8 @@ refill_objects(struct kmem_cache *s, void **p, gfp_t gfp, unsigned int min,
return refilled;
}

-static inline
-int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
- void **p)
+static bool __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags,
+ size_t size, void **p)
{
int i;

@@ -7312,30 +7310,43 @@ int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
stat_add(s, ALLOC_SLOWPATH, i);
}

- return i;
+ return true;

error:
__kmem_cache_free_bulk(s, i, p);
- return 0;
-
+ return false;
}

-/*
- * Note that interrupts must be enabled when calling this function and gfp
- * flags must allow spinning.
+/**
+ * kmem_cache_alloc_bulk - Allocate multiple objects
+ * @s: The cache to allocate from
+ * @flags: GFP_* flags. See kmalloc().
+ * @size: Number of objects to allocate
+ * @p: Array of allocated objects
+ *
+ * Allocate @size objects from @s and places them into @p. @size must be larger
+ * than 0.
+ *
+ * Interrupts must be enabled when calling this function and @flags must allow
+ * spinning.
+ *
+ * Unlike alloc_pages_bulk(), this function does not check for already allocated
+ * objects in @p, and thus the caller does not need to zero it.
+ *
+ * Return: %true if the allocation succeeded, or %false if it failed.
*/
-int kmem_cache_alloc_bulk_noprof(struct kmem_cache *s, gfp_t flags, size_t size,
- void **p)
+bool kmem_cache_alloc_bulk_noprof(struct kmem_cache *s, gfp_t flags,
+ size_t size, void **p)
{
unsigned int i = 0;
void *kfence_obj;

if (!size)
- return 0;
+ return false;

s = slab_pre_alloc_hook(s, flags);
if (unlikely(!s))
- return 0;
+ return false;

/*
* to make things simpler, only assume at most once kfence allocated
@@ -7352,18 +7363,18 @@ int kmem_cache_alloc_bulk_noprof(struct kmem_cache *s, gfp_t flags, size_t size,
}

i = alloc_from_pcs_bulk(s, flags, size, p);
-
if (i < size) {
/*
* If we ran out of memory, don't bother with freeing back to
* the percpu sheaves, we have bigger problems.
*/
- if (unlikely(__kmem_cache_alloc_bulk(s, flags, size - i, p + i) == 0)) {
+ if (unlikely(!__kmem_cache_alloc_bulk(s, flags, size - i,
+ p + i))) {
if (i > 0)
__kmem_cache_free_bulk(s, i, p);
if (kfence_obj)
__kfence_free(kfence_obj);
- return 0;
+ return false;
}
}

@@ -7382,12 +7393,8 @@ int kmem_cache_alloc_bulk_noprof(struct kmem_cache *s, gfp_t flags, size_t size,
* memcg and kmem_cache debug support and memory initialization.
* Done outside of the IRQ disabled fastpath loop.
*/
- if (unlikely(!slab_post_alloc_hook(s, NULL, flags, size, p,
- slab_want_init_on_alloc(flags, s), s->object_size))) {
- return 0;
- }
-
- return size;
+ return likely(slab_post_alloc_hook(s, NULL, flags, size, p,
+ slab_want_init_on_alloc(flags, s), s->object_size));
}
EXPORT_SYMBOL(kmem_cache_alloc_bulk_noprof);

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 2bc04feadfab..99ab9ddb05e3 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -243,12 +243,11 @@ static int xdp_recv_frames(struct xdp_frame **frames, int nframes,
struct net_device *dev)
{
gfp_t gfp = __GFP_ZERO | GFP_ATOMIC;
- int i, n;
+ int i;
LIST_HEAD(list);

- n = kmem_cache_alloc_bulk(net_hotdata.skbuff_cache, gfp, nframes,
- (void **)skbs);
- if (unlikely(n == 0)) {
+ if (!kmem_cache_alloc_bulk(net_hotdata.skbuff_cache, gfp, nframes,
+ (void **)skbs)) {
for (i = 0; i < nframes; i++)
xdp_return_frame(frames[i]);
return -ENOMEM;
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;

diff --git a/tools/include/linux/slab.h b/tools/include/linux/slab.h
index 6d8e9413d5a4..2e63c2e726aa 100644
--- a/tools/include/linux/slab.h
+++ b/tools/include/linux/slab.h
@@ -183,7 +183,7 @@ __kmem_cache_create(const char *name, unsigned int size, unsigned int align,
default: __kmem_cache_create)(__name, __object_size, __args, __VA_ARGS__)

void kmem_cache_free_bulk(struct kmem_cache *cachep, size_t size, void **list);
-int kmem_cache_alloc_bulk(struct kmem_cache *cachep, gfp_t gfp, size_t size,
+bool kmem_cache_alloc_bulk(struct kmem_cache *cachep, gfp_t gfp, size_t size,
void **list);
struct slab_sheaf *
kmem_cache_prefill_sheaf(struct kmem_cache *s, gfp_t gfp, unsigned int size);
diff --git a/tools/testing/shared/linux.c b/tools/testing/shared/linux.c
index 8c7257155958..e9c3bc9b3272 100644
--- a/tools/testing/shared/linux.c
+++ b/tools/testing/shared/linux.c
@@ -154,7 +154,7 @@ void kmem_cache_shrink(struct kmem_cache *cachep)
{
}

-int kmem_cache_alloc_bulk(struct kmem_cache *cachep, gfp_t gfp, size_t size,
+bool kmem_cache_alloc_bulk(struct kmem_cache *cachep, gfp_t gfp, size_t size,
void **p)
{
size_t i;
@@ -213,7 +213,7 @@ int kmem_cache_alloc_bulk(struct kmem_cache *cachep, gfp_t gfp, size_t size,
pthread_mutex_unlock(&cachep->lock);
if (cachep->callback)
cachep->exec_callback = true;
- return 0;
+ return false;
}

for (i = 0; i < size; i++) {
@@ -224,7 +224,7 @@ int kmem_cache_alloc_bulk(struct kmem_cache *cachep, gfp_t gfp, size_t size,
printf("Allocating %p from slab\n", p[i]);
}

- return size;
+ return true;
}

struct kmem_cache *
@@ -271,8 +271,8 @@ kmem_cache_prefill_sheaf(struct kmem_cache *s, gfp_t gfp, unsigned int size)

sheaf->cache = s;
sheaf->capacity = capacity;
- sheaf->size = kmem_cache_alloc_bulk(s, gfp, size, sheaf->objects);
- if (!sheaf->size) {
+ sheaf->size = size;
+ if (!kmem_cache_alloc_bulk(s, gfp, size, sheaf->objects)) {
free(sheaf);
return NULL;
}
@@ -284,7 +284,6 @@ int kmem_cache_refill_sheaf(struct kmem_cache *s, gfp_t gfp,
struct slab_sheaf **sheafp, unsigned int size)
{
struct slab_sheaf *sheaf = *sheafp;
- int refill;

if (sheaf->size >= size)
return 0;
@@ -299,12 +298,10 @@ int kmem_cache_refill_sheaf(struct kmem_cache *s, gfp_t gfp,
return 0;
}

- refill = kmem_cache_alloc_bulk(s, gfp, size - sheaf->size,
- &sheaf->objects[sheaf->size]);
- if (!refill)
+ if (!kmem_cache_alloc_bulk(s, gfp, size - sheaf->size,
+ &sheaf->objects[sheaf->size]))
return -ENOMEM;
-
- sheaf->size += refill;
+ sheaf->size += (size - sheaf->size);
return 0;
}

--
2.53.0

[Vlastimil Babka (SUSE)]

On 5/28/26 11:34, 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.
>
> Fix this by returning a bool if the allocation succeeded and adding a
> kerneldoc comment explaining the API.
>
> Signed-off-by: Christoph Hellwig <h...@lst.de>
> Reviewed-by: Alexander Lobakin <aleksande...@intel.com> # skbuff
> ---
> drivers/gpu/drm/msm/msm_iommu.c | 6 +--
> drivers/gpu/drm/panthor/panthor_mmu.c | 13 +++---
> include/linux/slab.h | 6 ++-
> io_uring/io_uring.c | 23 ++++-------
> lib/test_meminit.c | 23 +++++------
> mm/kasan/kasan_test_c.c | 5 +--
> mm/kfence/kfence_test.c | 9 ++--
> mm/slub.c | 59 +++++++++++++++------------
> 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, 97 insertions(+), 99 deletions(-)

Thanks, I applied it to slab/for-7.2/alloc_bulk and merged to slab/for-next
(it's still yankable in case of issues)

Did some fixups below (the comment was stale prior to the patch; restored
unlikely(), simplified one line).

A test merge into yesterday's -next found a conflict in drivers/gpu/drm/
panthor/panthor_mmu.c. Commit 1013bf53650e ("drm/panthor: Split
panthor_vm_prepare_map_op_ctx() to prepare for reclaim") moved the changed
codeto a new function panthor_vm_op_ctx_prealloc_pts().
But it's solvable so no need for a complicated coordination I think.

diff --git a/mm/slub.c b/mm/slub.c
index 6caf6f3ceeed..711df528c9a6 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -7372,10 +7372,7 @@ bool kmem_cache_alloc_bulk_noprof(struct kmem_cache *s, gfp_t flags,
}

out:
- /*
- * memcg and kmem_cache debug support and memory initialization.
- * Done outside of the IRQ disabled fastpath loop.
- */
+ /* memcg and kmem_cache debug support and memory initialization */

return likely(slab_post_alloc_hook(s, NULL, flags, size, p,

slab_want_init_on_alloc(flags, s), s->object_size));
}
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 99ab9ddb05e3..dbf0d8eae8d8 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -246,8 +246,8 @@ static int xdp_recv_frames(struct xdp_frame **frames, int nframes,
int i;
LIST_HEAD(list);

- if (!kmem_cache_alloc_bulk(net_hotdata.skbuff_cache, gfp, nframes,
- (void **)skbs)) {
+ if (unlikely(!kmem_cache_alloc_bulk(net_hotdata.skbuff_cache, gfp,
+ nframes, (void **)skbs))) {

for (i = 0; i < nframes; i++)
xdp_return_frame(frames[i]);
return -ENOMEM;

diff --git a/tools/testing/shared/linux.c b/tools/testing/shared/linux.c
index e9c3bc9b3272..e0a0693df08f 100644
--- a/tools/testing/shared/linux.c
+++ b/tools/testing/shared/linux.c
@@ -301,7 +301,7 @@ int kmem_cache_refill_sheaf(struct kmem_cache *s, gfp_t gfp,

if (!kmem_cache_alloc_bulk(s, gfp, size - sheaf->size,

&sheaf->objects[sheaf->size]))
return -ENOMEM;

- sheaf->size += (size - sheaf->size);

+ sheaf->size = size;

return 0;
}

[Christoph Hellwig]

On Fri, May 29, 2026 at 01:54:48PM +0200, Vlastimil Babka (SUSE) wrote:
> Thanks, I applied it to slab/for-7.2/alloc_bulk and merged to slab/for-next
> (it's still yankable in case of issues)
>
> Did some fixups below (the comment was stale prior to the patch; restored
> unlikely(), simplified one line).
>
> A test merge into yesterday's -next found a conflict in drivers/gpu/drm/
> panthor/panthor_mmu.c. Commit 1013bf53650e ("drm/panthor: Split
> panthor_vm_prepare_map_op_ctx() to prepare for reclaim") moved the changed
> codeto a new function panthor_vm_op_ctx_prealloc_pts().
> But it's solvable so no need for a complicated coordination I think.

Ok, thanks. The two Sashiko complains also look like they had merrits,
but I won't get to looking into them until Monday.

Your fixups look good to me.

[Harry Yoo]

The review:
https://sashiko.dev/#/patchset/20260528093437.2519248-2-hch%40lst.de

So there is a user who might call kmem_cache_alloc_bulk() with size = 0
(although the comment says @size must be larger than 0!) and
kmem_cache_alloc_bulk() returning 0 was considered a success in that case.

Either fixing kmem_cache_alloc_bulk() (and the comment) or fixing the
user sounds fine to me.

And yeah freeing an object via kfree() allocated via kvmalloc is a bug...

--
Cheers,
Harry / Hyeonggon

[Christoph Hellwig]

On Mon, Jun 01, 2026 at 03:39:06PM +0900, Harry Yoo wrote:
> > Ok, thanks. The two Sashiko complains also look like they had merrits,
> > but I won't get to looking into them until Monday.
> The review:
> https://sashiko.dev/#/patchset/20260528093437.2519248-2-hch%40lst.de
>
> So there is a user who might call kmem_cache_alloc_bulk() with size = 0
> (although the comment says @size must be larger than 0!) and
> kmem_cache_alloc_bulk() returning 0 was considered a success in that case.
>
> Either fixing kmem_cache_alloc_bulk() (and the comment) or fixing the
> user sounds fine to me.
>
> And yeah freeing an object via kfree() allocated via kvmalloc is a bug...

I think the right fix here is to check p->count somewhere in the msm
code and never get here. And the kfree/kvfree fix of course.

І.e. nothing to change in this patch, but a headsup for the MSM
maintainers.

[Vlastimil Babka (SUSE)]

On 6/1/26 08:39, Harry Yoo wrote:
>
>
> On 5/29/26 10:50 PM, Christoph Hellwig wrote:
>> On Fri, May 29, 2026 at 01:54:48PM +0200, Vlastimil Babka (SUSE) wrote:
>>> Thanks, I applied it to slab/for-7.2/alloc_bulk and merged to slab/for-next
>>> (it's still yankable in case of issues)
>>>
>>> Did some fixups below (the comment was stale prior to the patch; restored
>>> unlikely(), simplified one line).
>>>
>>> A test merge into yesterday's -next found a conflict in drivers/gpu/drm/
>>> panthor/panthor_mmu.c. Commit 1013bf53650e ("drm/panthor: Split
>>> panthor_vm_prepare_map_op_ctx() to prepare for reclaim") moved the changed
>>> codeto a new function panthor_vm_op_ctx_prealloc_pts().
>>> But it's solvable so no need for a complicated coordination I think.
>>
>> Ok, thanks. The two Sashiko complains also look like they had merrits,
>> but I won't get to looking into them until Monday.
> The review:
> https://sashiko.dev/#/patchset/20260528093437.2519248-2-hch%40lst.de
>
> So there is a user who might call kmem_cache_alloc_bulk() with size = 0

I don't know if it can really happen there, but maybe DRM folks can tell us.

> (although the comment says @size must be larger than 0!) and

The comment is however new, the caller existed when there was no comment and
the return value 0 when asked for 0 was working there.

> kmem_cache_alloc_bulk() returning 0 was considered a success in that case.
>
> Either fixing kmem_cache_alloc_bulk() (and the comment) or fixing the
> user sounds fine to me.

Would it be wrong if we just returned true for size of 0? Would something
else break?

> And yeah freeing an object via kfree() allocated via kvmalloc is a bug...

Yep.

[Christoph Hellwig]

On Mon, Jun 01, 2026 at 10:16:30AM +0200, Vlastimil Babka (SUSE) wrote:
> > kmem_cache_alloc_bulk() returning 0 was considered a success in that case.
> >
> > Either fixing kmem_cache_alloc_bulk() (and the comment) or fixing the
> > user sounds fine to me.
>
> Would it be wrong if we just returned true for size of 0? Would something
> else break?

I don't think it is wrong per se, but it feels like the wrong kind of
API. I.e. I don't think the MSM caller actually wants this, as they'd
also do a zero-sized kvmalloc.

[Harry Yoo]

On 6/1/26 5:16 PM, Vlastimil Babka (SUSE) wrote:
> On 6/1/26 08:39, Harry Yoo wrote:
>>
>>
>> On 5/29/26 10:50 PM, Christoph Hellwig wrote:
>>> On Fri, May 29, 2026 at 01:54:48PM +0200, Vlastimil Babka (SUSE) wrote:
>>>> Thanks, I applied it to slab/for-7.2/alloc_bulk and merged to slab/for-next
>>>> (it's still yankable in case of issues)
>>>>
>>>> Did some fixups below (the comment was stale prior to the patch; restored
>>>> unlikely(), simplified one line).
>>>>
>>>> A test merge into yesterday's -next found a conflict in drivers/gpu/drm/
>>>> panthor/panthor_mmu.c. Commit 1013bf53650e ("drm/panthor: Split
>>>> panthor_vm_prepare_map_op_ctx() to prepare for reclaim") moved the changed
>>>> codeto a new function panthor_vm_op_ctx_prealloc_pts().
>>>> But it's solvable so no need for a complicated coordination I think.
>>>
>>> Ok, thanks. The two Sashiko complains also look like they had merrits,
>>> but I won't get to looking into them until Monday.
>> The review:
>> https://sashiko.dev/#/patchset/20260528093437.2519248-2-hch%40lst.de
>>
>> So there is a user who might call kmem_cache_alloc_bulk() with size = 0
>
> I don't know if it can really happen there, but maybe DRM folks can tell us.

Yeah beyond my area :)

>> (although the comment says @size must be larger than 0!) and
>
> The comment is however new, the caller existed when there was no comment and
> the return value 0 when asked for 0 was working there.

Haha, you're right. Didn't notice that...

>> kmem_cache_alloc_bulk() returning 0 was considered a success in that case.
>>
>> Either fixing kmem_cache_alloc_bulk() (and the comment) or fixing the
>> user sounds fine to me.
>
> Would it be wrong if we just returned true for size of 0? Would something
> else break?

I think it won't break as nobody should be accessing an array of size 0,
and it matches the previous behavior.

[Vlastimil Babka (SUSE)]

If p->count is 0 then indeed there's a zero-sized kvmalloc so p->pages ==
ZERO_SIZE_PTR but then nothing breaks because nothing tries to dereference it?

msm_iommu_pagetable_prealloc_cleanup() has a "if (p->count > 0)" branch so
it seems it's considered possible. But then the rest of the functions also
seems working fine, i.e. kmem_cache_free_bulk() of zero size does nothing,
kvfree() of ZERO_SIZE_PTR does nothing.

It seems to me kmem_cache_alloc_bulk() returning true for size == 0 fits
naturally in this world and is less likely to result in a gotcha?

[Rob Clark]

I think I was probably expecting kvmalloc(0) => NULL ... but it
happened to work out before

Adding an "if (!p->count) return 0;" at the top of
msm_iommu_pagetable_prealloc_allocate() seems like the thing to do..
if you want, I can send that patch (but traveling this week... so
let's see what I can do)

BR,
-R

[Rob Clark]

Aaaaaand.. sending patch from hotel wifi doesn't seem to be a thing
that works.. but I've tested the following w/ deqp-vk cts, and works
as expected

----------------

diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index 7d449e5202c5..ef744d154bbe 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -332,13 +332,16 @@ msm_iommu_pagetable_prealloc_allocate(struct
msm_mmu *mmu, struct msm_mmu_preall

struct kmem_cache *pt_cache = get_pt_cache(mmu);

int ret;

+ if (!p->count)
+ return 0;
+

p->pages = kvmalloc_objs(*p->pages, p->count);
if (!p->pages)
return -ENOMEM;

ret = kmem_cache_alloc_bulk(pt_cache, GFP_KERNEL, p->count, p->pages);

if (ret != p->count) {

- kfree(p->pages);
+ kvfree(p->pages);
p->pages = NULL;
p->count = ret;
return -ENOMEM;

[Vlastimil Babka (SUSE)]

Thanks, I will amend the commit in slab tree with this as the easiest way to
go foward.
Just a quick question though:

> ----------------
>
> diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
> index 7d449e5202c5..ef744d154bbe 100644
> --- a/drivers/gpu/drm/msm/msm_iommu.c
> +++ b/drivers/gpu/drm/msm/msm_iommu.c
> @@ -332,13 +332,16 @@ msm_iommu_pagetable_prealloc_allocate(struct
> msm_mmu *mmu, struct msm_mmu_preall
> struct kmem_cache *pt_cache = get_pt_cache(mmu);
> int ret;
>
> + if (!p->count)
> + return 0;

We know p->pages is NULL in this case, right? Because it was allocated by
vm_bind_job_create() using kzalloc().
And the job can't be reused with a leftover value?
(msm_iommu_pagetable_prealloc_cleanup doesn't set p->pages to zero).
Or should we set p->pages to NULL here.

[Rob Clark]

Thanks

> Just a quick question though:
>
> > ----------------
> >
> > diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
> > index 7d449e5202c5..ef744d154bbe 100644
> > --- a/drivers/gpu/drm/msm/msm_iommu.c
> > +++ b/drivers/gpu/drm/msm/msm_iommu.c
> > @@ -332,13 +332,16 @@ msm_iommu_pagetable_prealloc_allocate(struct
> > msm_mmu *mmu, struct msm_mmu_preall
> > struct kmem_cache *pt_cache = get_pt_cache(mmu);
> > int ret;
> >
> > + if (!p->count)
> > + return 0;
>
> We know p->pages is NULL in this case, right? Because it was allocated by
> vm_bind_job_create() using kzalloc().
> And the job can't be reused with a leftover value?
> (msm_iommu_pagetable_prealloc_cleanup doesn't set p->pages to zero).
> Or should we set p->pages to NULL here.

Correct, the job is not reused. But I suppose setting p->pages to
NULL would make things more obvious, so no objection to that.

BR,
-R

[Vlastimil Babka (SUSE)]

OK, did that, just in case. Thanks.

[Harry Yoo]

On 6/4/26 1:22 AM, Vlastimil Babka (SUSE) wrote:
> On 6/3/26 13:13, Rob Clark wrote:
>> On Wed, Jun 3, 2026 at 2:17 AM Vlastimil Babka (SUSE) <vba...@kernel.org> wrote:
>>>
>>> We know p->pages is NULL in this case, right? Because it was allocated by
>>> vm_bind_job_create() using kzalloc().
>>> And the job can't be reused with a leftover value?
>>> (msm_iommu_pagetable_prealloc_cleanup doesn't set p->pages to zero).
>>> Or should we set p->pages to NULL here.
>>
>> Correct, the job is not reused. But I suppose setting p->pages to
>> NULL would make things more obvious, so no objection to that.
>
> OK, did that, just in case. Thanks.

The kvfree() -> kfree() part should probably be a separate patch
with Fixes: 830d68f2cb8a ("drm/msm: Fix pgtable prealloc error
path") and Cc: stable?

...as the commit landed v6.18.

>> BR,
>> -R
>>
>>>> +
>>>> p->pages = kvmalloc_objs(*p->pages, p->count);
>>>> if (!p->pages)
>>>> return -ENOMEM;
>>>>
>>>> ret = kmem_cache_alloc_bulk(pt_cache, GFP_KERNEL, p->count, p->pages);
>>>> if (ret != p->count) {
>>>> - kfree(p->pages);
>>>> + kvfree(p->pages);
>>>> p->pages = NULL;
>>>> p->count = ret;
>>>> return -ENOMEM;

[Vlastimil Babka (SUSE)]

On 6/4/26 09:10, Harry Yoo wrote:
>
>
> On 6/4/26 1:22 AM, Vlastimil Babka (SUSE) wrote:
>> On 6/3/26 13:13, Rob Clark wrote:
>>> On Wed, Jun 3, 2026 at 2:17 AM Vlastimil Babka (SUSE) <vba...@kernel.org> wrote:
>>>>
>>>> We know p->pages is NULL in this case, right? Because it was allocated by
>>>> vm_bind_job_create() using kzalloc().
>>>> And the job can't be reused with a leftover value?
>>>> (msm_iommu_pagetable_prealloc_cleanup doesn't set p->pages to zero).
>>>> Or should we set p->pages to NULL here.
>>>
>>> Correct, the job is not reused. But I suppose setting p->pages to
>>> NULL would make things more obvious, so no objection to that.
>>
>> OK, did that, just in case. Thanks.
>
> The kvfree() -> kfree() part should probably be a separate patch
> with Fixes: 830d68f2cb8a ("drm/msm: Fix pgtable prealloc error
> path") and Cc: stable?
>
> ...as the commit landed v6.18.

Hm right, but realistically, can there be so many pages necessary, that the
array to hold their pointers would be over what kmalloc() can provide?

[Rob Clark]

On Thu, Jun 4, 2026 at 12:35 AM Vlastimil Babka (SUSE)

<vba...@kernel.org> wrote:
>
> On 6/4/26 09:10, Harry Yoo wrote:
> >
> >
> > On 6/4/26 1:22 AM, Vlastimil Babka (SUSE) wrote:
> >> On 6/3/26 13:13, Rob Clark wrote:
> >>> On Wed, Jun 3, 2026 at 2:17 AM Vlastimil Babka (SUSE) <vba...@kernel.org> wrote:
> >>>>
> >>>> We know p->pages is NULL in this case, right? Because it was allocated by
> >>>> vm_bind_job_create() using kzalloc().
> >>>> And the job can't be reused with a leftover value?
> >>>> (msm_iommu_pagetable_prealloc_cleanup doesn't set p->pages to zero).
> >>>> Or should we set p->pages to NULL here.
> >>>
> >>> Correct, the job is not reused. But I suppose setting p->pages to
> >>> NULL would make things more obvious, so no objection to that.
> >>
> >> OK, did that, just in case. Thanks.
> >
> > The kvfree() -> kfree() part should probably be a separate patch
> > with Fixes: 830d68f2cb8a ("drm/msm: Fix pgtable prealloc error
> > path") and Cc: stable?
> >
> > ...as the commit landed v6.18.
>
> Hm right, but realistically, can there be so many pages necessary, that the
> array to hold their pointers would be over what kmalloc() can provide?

Pretty unlikely.. IIRC kvmalloc won't fallback to vmalloc for anything
under PAGE_SIZE, so count==512.. which is more than 10x what I've
seen in practice

BR,
-R