Not an issue with the switch itself: map_init()'s error path misses draining the deferred vm_bo. Will fix in v2.
On Sun, Oct 11, 2026 at 2:36 AM <[email protected]> wrote: > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [High] drm/imagination: memory leak of gpuvm_bo on pvr_vm_map error path > -- > > commit 191fc7b0dba1141708c7edc0b3aeac368ef8588f > Author: Gyeyoung Baek <[email protected]> > > drm/imagination: Switch the GPUVM to immediate mode > > This commit switches the GPUVM to immediate mode so that each GEM's GPUVA list > is protected by its gpuva.lock. It introduces deferred operations such as > drm_gpuvm_bo_put_deferred() and cleanup via drm_gpuvm_bo_deferred_cleanup(). > > > diff --git a/drivers/gpu/drm/imagination/pvr_vm.c > > b/drivers/gpu/drm/imagination/pvr_vm.c > > --- a/drivers/gpu/drm/imagination/pvr_vm.c > > +++ b/drivers/gpu/drm/imagination/pvr_vm.c > [ ... ] > > @@ -214,7 +214,7 @@ static int pvr_vm_bind_op_exec(struct pvr_vm_bind_op > > *bind_op) > > > > static void pvr_vm_bind_op_fini(struct pvr_vm_bind_op *bind_op) > > { > > - drm_gpuvm_bo_put(bind_op->gpuvm_bo); > > + drm_gpuvm_bo_put_deferred(bind_op->gpuvm_bo); > > > > kfree(bind_op->new_va); > > kfree(bind_op->prev_va); > [ ... ] > > @@ -758,7 +769,9 @@ pvr_vm_map(struct pvr_vm_context *vm_ctx, struct > > pvr_gem_object *pvr_obj, > > > > err_cleanup: > > mutex_unlock(&vm_ctx->lock); > > + > > pvr_vm_bind_op_fini(&bind_op); > > + drm_gpuvm_bo_deferred_cleanup(&vm_ctx->gpuvm_mgr); > > > > return err; > > } > > [Severity: High] > Does this code leak the gpuvm_bo and the referenced drm_gem_object on the > early error path of pvr_vm_map()? > > If pvr_vm_bind_op_map_init() fails internally (for example, if memory is > exhausted when calling pvr_gem_object_get_pages_sgt()), it takes its own > error path: > > pvr_vm_bind_op_map_init() { > ... > err_bind_op_fini: > pvr_vm_bind_op_fini(bind_op); > > return err; > } > > This calls the newly modified pvr_vm_bind_op_fini(), which places the BO onto > the gpuvm_mgr bo_defer list via drm_gpuvm_bo_put_deferred(). > > However, back in pvr_vm_map(), the early return bypasses the newly added > cleanup call: > > pvr_vm_map() { > ... > int err = pvr_vm_bind_op_map_init(&bind_op, vm_ctx, pvr_obj, > pvr_obj_offset, device_addr, > size); > > if (err) > return err; > ... > } > > Because drm_gpuvm_bo_deferred_cleanup() is bypassed here, would this cause an > unbounded accumulation of leaked BOs on the bo_defer list? Can this be fixed > by ensuring the cleanup function is also called on this early return path? > > -- > Sashiko AI review · > https://sashiko.dev/#/patchset/[email protected]?part=1 -- Thanks, Gyeyoung
