On Mon, 20 Oct 2025 10:50:19 +0100 Akash Goel <[email protected]> wrote:
> On 10/20/25 10:30, Boris Brezillon wrote: > > On Mon, 20 Oct 2025 09:59:14 +0100 > > Akash Goel <[email protected]> wrote: > > > >> This commit addresses a memleak issue of panthor_vma (or drm_gpuva) > >> structure in Panthor driver, that can happen if the GPU page table > >> update operation to map the pages fail. > >> The issue is very unlikely to occur in practice. > >> > >> Fixes: 647810ec2476 ("drm/panthor: Add the MMU/VM logical block") > >> Signed-off-by: Akash Goel <[email protected]> > >> --- > >> drivers/gpu/drm/panthor/panthor_mmu.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c > >> b/drivers/gpu/drm/panthor/panthor_mmu.c > >> index 6dec4354e378..34a86f7b58d9 100644 > >> --- a/drivers/gpu/drm/panthor/panthor_mmu.c > >> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c > >> @@ -2081,8 +2081,10 @@ static int panthor_gpuva_sm_step_map(struct > >> drm_gpuva_op *op, void *priv) > >> ret = panthor_vm_map_pages(vm, op->map.va.addr, > >> flags_to_prot(vma->flags), > >> op_ctx->map.sgt, op->map.gem.offset, > >> op->map.va.range); > >> - if (ret) > >> + if (ret) { > >> + kfree(vma); > > > > Calling kfree() in this context is probably fine, but I think I'd > > prefer if we were introducing a panthor_vm_op_ctx_return_vma() helper > > returning the vma to the preallocated array, and letting the deferred > > cleanup function free this up. > > > Thanks for the quick review. > > So need to do like this, where we search for a NULL entry to store the > VMA pointer to be returned ? > > static void > panthor_vm_op_ctx_return_vma(struct panthor_vm_op_ctx *op_ctx, > struct panthor_vma *vma) > { > for (u32 i = 0; i < ARRAY_SIZE(op_ctx->preallocated_vmas); i++) { > if (!op_ctx->preallocated_vmas[i]) { > op_ctx->preallocated_vmas[i] = vma; > return; > } > } > } > > > Please let me know. Yep, that looks good to me.
