Messages in this thread Patch in this message |  | | From | Rob Clark <> | | Date | Mon, 1 Jun 2026 07:39:17 -0700 | | Subject | Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk |
| |
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;
|  |