On Thu, Jun 4, 2026 at 12:35 AM Vlastimil Babka (SUSE)
<[email protected]> 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) <[email protected]>
> >>> 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;
> >
>
