On Mon, Jun 1, 2026 at 6:32 AM Rob Clark <[email protected]> wrote:
>
> On Mon, Jun 1, 2026 at 5:50 AM Vlastimil Babka (SUSE) <[email protected]> 
> 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;

Reply via email to