Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[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 <hch@lst.de>
> Reviewed-by: Alexander Lobakin <aleksander.lobakin@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;
 }

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[Harry Yoo]

[-- Attachment #1.1: Type: text/plain, Size: 1372 bytes --]



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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

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

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[Harry Yoo]

[-- Attachment #1.1: Type: text/plain, Size: 1959 bytes --]



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.

-- 
Cheers,
Harry / Hyeonggon


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[Vlastimil Babka (SUSE)]

On 6/1/26 13:38, Christoph Hellwig wrote:
> 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.

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?

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[Rob Clark]

On Mon, Jun 1, 2026 at 5:50 AM Vlastimil Babka (SUSE) <vbabka@kernel.org> wrote:
>
> On 6/1/26 13:38, Christoph Hellwig wrote:
> > 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.
>
> 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?

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

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[Rob Clark]

On Mon, Jun 1, 2026 at 6:32 AM Rob Clark <rob.clark@oss.qualcomm.com> wrote:
>
> On Mon, Jun 1, 2026 at 5:50 AM Vlastimil Babka (SUSE) <vbabka@kernel.org> wrote:
> >
> > On 6/1/26 13:38, Christoph Hellwig wrote:
> > > 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.
> >
> > 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?
>
> 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)

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;

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[Vlastimil Babka (SUSE)]

On 6/1/26 16:39, Rob Clark wrote:
> On Mon, Jun 1, 2026 at 6:32 AM Rob Clark <rob.clark@oss.qualcomm.com> wrote:
>>
>> On Mon, Jun 1, 2026 at 5:50 AM Vlastimil Babka (SUSE) <vbabka@kernel.org> wrote:
>> >
>> > On 6/1/26 13:38, Christoph Hellwig wrote:
>> > > 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.
>> >
>> > 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?
>>
>> 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)
> 
> 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

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.

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

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[Rob Clark]

On Wed, Jun 3, 2026 at 2:17 AM Vlastimil Babka (SUSE) <vbabka@kernel.org> wrote:
>
> On 6/1/26 16:39, Rob Clark wrote:
> > On Mon, Jun 1, 2026 at 6:32 AM Rob Clark <rob.clark@oss.qualcomm.com> wrote:
> >>
> >> On Mon, Jun 1, 2026 at 5:50 AM Vlastimil Babka (SUSE) <vbabka@kernel.org> wrote:
> >> >
> >> > On 6/1/26 13:38, Christoph Hellwig wrote:
> >> > > 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.
> >> >
> >> > 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?
> >>
> >> 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)
> >
> > 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
>
> Thanks, I will amend the commit in slab tree with this as the easiest way to
> go foward.

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

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

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[Vlastimil Babka (SUSE)]

On 6/3/26 13:13, Rob Clark wrote:
> On Wed, Jun 3, 2026 at 2:17 AM Vlastimil Babka (SUSE) <vbabka@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.

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

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[Harry Yoo]

[-- Attachment #1.1: Type: text/plain, Size: 1368 bytes --]



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) <vbabka@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;

-- 
Cheers,
Harry / Hyeonggon

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[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) <vbabka@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?

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

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[Rob Clark]

On Thu, Jun 4, 2026 at 12:35 AM Vlastimil Babka (SUSE)
<vbabka@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) <vbabka@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

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

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 4512 bytes --]

> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index 75d98dad7b1d..b80d7e1d5123 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -1274,10 +1274,9 @@ 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;
>  	}

Does this omit setting op_ctx->rsvd_page_tables.count to pt_count on success?

Since op_ctx is initialized via memset previously, the count will remain 0 when
the allocation succeeds. This could lead to memory leaks when these page tables
are consumed or cleaned up later, since the cleanup code won't know about the
allocated page tables.

The original code set the count to ret on both success and failure. The
updated code only sets count = 0 in the failure path, but doesn't set
count = pt_count in the success path.

[ ... ]

> diff --git a/lib/test_meminit.c b/lib/test_meminit.c
> index 6298f66c964b..3727dee7c262 100644
> --- a/lib/test_meminit.c
> +++ b/lib/test_meminit.c
> @@ -229,14 +229,12 @@ 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);
>  			}

Will this cause a compilation error?

The local variable ret is removed from this scope, but it's still used in the
kmem_cache_free_bulk() call. This should fail to compile because ret is
undeclared.

The fix should probably replace ret with BULK_SIZE in the free call:

	kmem_cache_free_bulk(c, BULK_SIZE, bulk_array);


> @@ -354,17 +352,18 @@ static int __init do_kmem_cache_size_bulk(int size, int *total_failures)
>
>  	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, num, objects);
>  	}
>  	kmem_cache_destroy(c);
>  	*total_failures += fail;

Is num uninitialized here?

It's no longer assigned the result of the allocation call. Passing it to
kmem_cache_free_bulk() will use indeterminate stack data, which could lead to
memory corruption. The fix should probably be:

	kmem_cache_free_bulk(c, ARRAY_SIZE(objects), objects);

[ ... ]

> diff --git a/mm/slub.c b/mm/slub.c
> index a2bf3756ca7d..d9790e7c17f6 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c

[ ... ]

> @@ -7352,18 +7362,18 @@ int kmem_cache_alloc_bulk_noprof(struct kmem_cache *s, gfp_t flags, size_t size,
>  	 */
>  	unsigned int i = 0;
>  	void *kfence_obj;
>
>  	if (!size)
> -		return 0;
> +		return false;

Does this incorrectly report an allocation failure when size is 0?

Previously, callers interpreting the return value against size (ret != size)
would treat 0 objects allocated for a request of 0 as a success. Returning
false here contradicts the typical kernel behavior where allocating 0 items is
a successful no-op.

This might cause callers to incorrectly abort with an error when they request
0 objects, which should be a valid no-op case.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26496962101

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[Jesper Dangaard Brouer]

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 <hch@lst.de>
> ---
>   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;
>   

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[Alexander Lobakin]

From: Jesper Dangaard Brouer <hawk@kernel.org>
Date: Wed, 27 May 2026 10:51:42 +0200

> 
> 
> 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 sorta remember that when I was reading this function, I also noticed
that it always returns only 2 possible values (0 or the requested
number), but didn't pay enough attention or it was already after I
introduced napi_skb_cache_get_bulk().

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

As far as I can see, the diff below doesn't introduce any functional
changes (but allows for a bit better compiler optimization). The logic
is still the same:

1) try to allocate non-zeroed skbs into the cache
2) if not enough, try to allocate zeroed skbs directly
3) if still not enough, return less than requested

The logic is still valid even if kmem_cache_alloc_bulk() return bool --
we might have some skbs in the cache (but less than requested) and then
the first allocation try may fail, but the second one succeed (as it
allocates from a different (the zeroed) zone).

> 
> 
>> Fix this by returning a bool if the allocation succeeded and adding a
>> kerneldoc comment explaining the API.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> ---
>>   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;

kmem_cache_alloc_bulk() allocates `n - nc->skb_count`, but here you
assign `nc->skb_count` to n.
Ah wait,

n -= n - nc->skb_count
n = n - (n - nc->skb_count)
n = n - n + nc->skb_count
n = nc->skb_count

Correct :D

>>       if (likely(nc->skb_count >= n))
>>           goto get;

Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com> # skbuff

Thanks,
Olek

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[Vlastimil Babka (SUSE)]

On 5/27/26 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.
> 
> Fix this by returning a bool if the allocation succeeded and adding a
> kerneldoc comment explaining the API.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Would 0 / -ENOMEM be more like what people would expect? I guess both that
and bool are better than the current API.

Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk

[kernel test robot]

Hi Christoph,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Christoph-Hellwig/mm-slab-improve-kmem_cache_alloc_bulk/20260527-150421
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20260527070239.2252948-2-hch%40lst.de
patch subject: [PATCH] mm/slab: improve kmem_cache_alloc_bulk
config: parisc-randconfig-r071-20260528 (https://download.01.org/0day-ci/archive/20260528/202605281629.y8HhAihO-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 8.5.0
smatch: v0.5.0-9185-gbcc58b9c
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260528/202605281629.y8HhAihO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605281629.y8HhAihO-lkp@intel.com/

All errors (new ones prefixed by >>):

   lib/test_meminit.c: In function 'do_kmem_cache_size':
>> lib/test_meminit.c:239:29: error: 'ret' undeclared (first use in this function); did you mean 'net'?
        kmem_cache_free_bulk(c, ret, bulk_array);
                                ^~~
                                net
   lib/test_meminit.c:239:29: note: each undeclared identifier is reported only once for each function it appears in


vim +239 lib/test_meminit.c

dc5c5ad79f0cc2d Laura Abbott        2019-12-04  209  
5015a300a522c8f Alexander Potapenko 2019-07-16  210  /*
5015a300a522c8f Alexander Potapenko 2019-07-16  211   * Test kmem_cache with given parameters:
5015a300a522c8f Alexander Potapenko 2019-07-16  212   *  want_ctor - use a constructor;
5015a300a522c8f Alexander Potapenko 2019-07-16  213   *  want_rcu - use SLAB_TYPESAFE_BY_RCU;
5015a300a522c8f Alexander Potapenko 2019-07-16  214   *  want_zero - use __GFP_ZERO.
5015a300a522c8f Alexander Potapenko 2019-07-16  215   */
5015a300a522c8f Alexander Potapenko 2019-07-16  216  static int __init do_kmem_cache_size(size_t size, bool want_ctor,
5015a300a522c8f Alexander Potapenko 2019-07-16  217  				     bool want_rcu, bool want_zero,
5015a300a522c8f Alexander Potapenko 2019-07-16  218  				     int *total_failures)
5015a300a522c8f Alexander Potapenko 2019-07-16  219  {
5015a300a522c8f Alexander Potapenko 2019-07-16  220  	struct kmem_cache *c;
5015a300a522c8f Alexander Potapenko 2019-07-16  221  	int iter;
5015a300a522c8f Alexander Potapenko 2019-07-16  222  	bool fail = false;
5015a300a522c8f Alexander Potapenko 2019-07-16  223  	gfp_t alloc_mask = GFP_KERNEL | (want_zero ? __GFP_ZERO : 0);
5015a300a522c8f Alexander Potapenko 2019-07-16  224  	void *buf, *buf_copy;
5015a300a522c8f Alexander Potapenko 2019-07-16  225  
5015a300a522c8f Alexander Potapenko 2019-07-16  226  	c = kmem_cache_create("test_cache", size, 1,
5015a300a522c8f Alexander Potapenko 2019-07-16  227  			      want_rcu ? SLAB_TYPESAFE_BY_RCU : 0,
5015a300a522c8f Alexander Potapenko 2019-07-16  228  			      want_ctor ? test_ctor : NULL);
5015a300a522c8f Alexander Potapenko 2019-07-16  229  	for (iter = 0; iter < 10; iter++) {
dc5c5ad79f0cc2d Laura Abbott        2019-12-04  230  		/* Do a test of bulk allocations */
dc5c5ad79f0cc2d Laura Abbott        2019-12-04  231  		if (!want_rcu && !want_ctor) {
863d4ee245fb4d6 Christoph Hellwig   2026-05-27  232  			if (!kmem_cache_alloc_bulk(c, alloc_mask, BULK_SIZE,
863d4ee245fb4d6 Christoph Hellwig   2026-05-27  233  					bulk_array)) {
dc5c5ad79f0cc2d Laura Abbott        2019-12-04  234  				fail = true;
dc5c5ad79f0cc2d Laura Abbott        2019-12-04  235  			} else {
dc5c5ad79f0cc2d Laura Abbott        2019-12-04  236  				int i;
863d4ee245fb4d6 Christoph Hellwig   2026-05-27  237  				for (i = 0; i < BULK_SIZE; i++)
dc5c5ad79f0cc2d Laura Abbott        2019-12-04  238  					fail |= check_buf(bulk_array[i], size, want_ctor, want_rcu, want_zero);
dc5c5ad79f0cc2d Laura Abbott        2019-12-04 @239  				kmem_cache_free_bulk(c, ret, bulk_array);
dc5c5ad79f0cc2d Laura Abbott        2019-12-04  240  			}
dc5c5ad79f0cc2d Laura Abbott        2019-12-04  241  		}
dc5c5ad79f0cc2d Laura Abbott        2019-12-04  242  
5015a300a522c8f Alexander Potapenko 2019-07-16  243  		buf = kmem_cache_alloc(c, alloc_mask);
5015a300a522c8f Alexander Potapenko 2019-07-16  244  		/* Check that buf is zeroed, if it must be. */
dc5c5ad79f0cc2d Laura Abbott        2019-12-04  245  		fail |= check_buf(buf, size, want_ctor, want_rcu, want_zero);
5015a300a522c8f Alexander Potapenko 2019-07-16  246  		fill_with_garbage_skip(buf, size, want_ctor ? CTOR_BYTES : 0);
d3a811617ae629d Arnd Bergmann       2019-07-16  247  
d3a811617ae629d Arnd Bergmann       2019-07-16  248  		if (!want_rcu) {
d3a811617ae629d Arnd Bergmann       2019-07-16  249  			kmem_cache_free(c, buf);
d3a811617ae629d Arnd Bergmann       2019-07-16  250  			continue;
d3a811617ae629d Arnd Bergmann       2019-07-16  251  		}
d3a811617ae629d Arnd Bergmann       2019-07-16  252  
5015a300a522c8f Alexander Potapenko 2019-07-16  253  		/*
5015a300a522c8f Alexander Potapenko 2019-07-16  254  		 * If this is an RCU cache, use a critical section to ensure we
5015a300a522c8f Alexander Potapenko 2019-07-16  255  		 * can touch objects after they're freed.
5015a300a522c8f Alexander Potapenko 2019-07-16  256  		 */
5015a300a522c8f Alexander Potapenko 2019-07-16  257  		rcu_read_lock();
5015a300a522c8f Alexander Potapenko 2019-07-16  258  		/*
5015a300a522c8f Alexander Potapenko 2019-07-16  259  		 * Copy the buffer to check that it's not wiped on
5015a300a522c8f Alexander Potapenko 2019-07-16  260  		 * free().
5015a300a522c8f Alexander Potapenko 2019-07-16  261  		 */
733d1d1a7745113 Alexander Potapenko 2019-08-02  262  		buf_copy = kmalloc(size, GFP_ATOMIC);
5015a300a522c8f Alexander Potapenko 2019-07-16  263  		if (buf_copy)
5015a300a522c8f Alexander Potapenko 2019-07-16  264  			memcpy(buf_copy, buf, size);
d3a811617ae629d Arnd Bergmann       2019-07-16  265  
4ab7ace465466d2 Alexander Potapenko 2019-07-16  266  		kmem_cache_free(c, buf);
5015a300a522c8f Alexander Potapenko 2019-07-16  267  		/*
5015a300a522c8f Alexander Potapenko 2019-07-16  268  		 * Check that |buf| is intact after kmem_cache_free().
5015a300a522c8f Alexander Potapenko 2019-07-16  269  		 * |want_zero| is false, because we wrote garbage to
5015a300a522c8f Alexander Potapenko 2019-07-16  270  		 * the buffer already.
5015a300a522c8f Alexander Potapenko 2019-07-16  271  		 */
5015a300a522c8f Alexander Potapenko 2019-07-16  272  		fail |= check_buf(buf, size, want_ctor, want_rcu,
5015a300a522c8f Alexander Potapenko 2019-07-16  273  				  false);
5015a300a522c8f Alexander Potapenko 2019-07-16  274  		if (buf_copy) {
5015a300a522c8f Alexander Potapenko 2019-07-16  275  			fail |= (bool)memcmp(buf, buf_copy, size);
5015a300a522c8f Alexander Potapenko 2019-07-16  276  			kfree(buf_copy);
5015a300a522c8f Alexander Potapenko 2019-07-16  277  		}
5015a300a522c8f Alexander Potapenko 2019-07-16  278  		rcu_read_unlock();
5015a300a522c8f Alexander Potapenko 2019-07-16  279  	}
5015a300a522c8f Alexander Potapenko 2019-07-16  280  	kmem_cache_destroy(c);
5015a300a522c8f Alexander Potapenko 2019-07-16  281  
5015a300a522c8f Alexander Potapenko 2019-07-16  282  	*total_failures += fail;
5015a300a522c8f Alexander Potapenko 2019-07-16  283  	return 1;
5015a300a522c8f Alexander Potapenko 2019-07-16  284  }
5015a300a522c8f Alexander Potapenko 2019-07-16  285  

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki