* [PATCH 0/2] drm/imagination: Switch the GPUVM to immediate mode
@ 2026-10-10 17:22 Gyeyoung Baek
2026-10-10 17:22 ` [PATCH 1/2] " Gyeyoung Baek
2026-10-10 17:22 ` [PATCH 2/2] drm/imagination: Stop locking reservations around map and unmap Gyeyoung Baek
0 siblings, 2 replies; 6+ messages in thread
From: Gyeyoung Baek @ 2026-10-10 17:22 UTC (permalink / raw)
To: Alessio Belle, Luigi Santivetti, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: Alexandru Dadu, Brajesh Gupta, Frank Binns, Danilo Krummrich,
Alice Ryhl, imagination, dri-devel, linux-kernel, Gyeyoung Baek
Switch the GPUVM to immediate mode, as a prerequisite for asynchronous
VM_BIND. A revised VM_BIND series will follow separately; v1 [1] is
linked only for context.
The switch also fixes an issue Sashiko reported on [1]: the GPUVA lists
of other objects are updated without holding their reservations [2].
Based on drm-misc-fixes (depends on 196d4d10c1b3).
Tested on BeagleY-AI with lockdep, KASAN and failslab enabled:
- All IGT imagination tests pass, including pvr_vm_map [3].
- The fail-nth reproducer in patch 1 no longer warns.
[1] https://lore.kernel.org/r/20260817-pvr-vm-bind-v1-0-0a0f21be7d38@gmail.com
[2] https://sashiko.dev/#/patchset/20260817-pvr-vm-bind-v1-0-0a0f21be7d38@gmail.com?part=1
[3] https://lore.kernel.org/igt-dev/20260930125057.1837788-1-gye976@gmail.com/
Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
Gyeyoung Baek (2):
drm/imagination: Switch the GPUVM to immediate mode
drm/imagination: Stop locking reservations around map and unmap
drivers/gpu/drm/imagination/pvr_vm.c | 80 +++++++++++++-----------------------
1 file changed, 28 insertions(+), 52 deletions(-)
---
base-commit: 6ccf996a0dec1852dab94ad865f27b4904750e12
change-id: 20261011-pvr-gpuvm-immediate-f7ea8edc2e7f
Best regards,
--
Gyeyoung Baek <gye976@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] drm/imagination: Switch the GPUVM to immediate mode
2026-10-10 17:22 [PATCH 0/2] drm/imagination: Switch the GPUVM to immediate mode Gyeyoung Baek
@ 2026-10-10 17:22 ` Gyeyoung Baek
2026-10-10 17:36 ` sashiko-bot
2026-10-10 17:22 ` [PATCH 2/2] drm/imagination: Stop locking reservations around map and unmap Gyeyoung Baek
1 sibling, 1 reply; 6+ messages in thread
From: Gyeyoung Baek @ 2026-10-10 17:22 UTC (permalink / raw)
To: Alessio Belle, Luigi Santivetti, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: Alexandru Dadu, Brajesh Gupta, Frank Binns, Danilo Krummrich,
Alice Ryhl, imagination, dri-devel, linux-kernel, Gyeyoung Baek
Use DRM_GPUVM_IMMEDIATE_MODE so that each GEM's GPUVA list is protected
by its gpuva.lock instead of its dma_resv. This prepares for
asynchronous VM_BIND, whose run_job() will update the GPUVA lists
before signalling its fence. That section imposes three rules:
1. No dma_resv: another thread may hold it while waiting for the job's
fence, directly or through a dependency.
-> Protect the lists with gpuva.lock, and take it around
drm_gpuva_link().
2. No allocation under a lock run_job() takes: reclaim may wait for a
fence that depends on the job.
-> Allocate the vm_bo before taking gpuva.lock:
drm_gpuvm_bo_obtain_locked() becomes drm_gpuvm_bo_create() +
drm_gpuvm_bo_obtain_prealloc().
3. No freeing of GEM objects: that takes their dma_resv, and under
gpuva.lock would free the held lock too.
-> Defer it: drm_gpuva_unlink() and drm_gpuvm_bo_put() become
drm_gpuva_unlink_defer() and drm_gpuvm_bo_put_deferred(), and
drm_gpuvm_bo_deferred_cleanup() frees once the locks are dropped.
This also fixes an unlocked GPUVA list update: when a map fails,
pvr_vm_bind_op_fini() drops the last vm_bo reference without holding
the object's dma_resv.
Failing VM_MAP allocations with fail-nth shows
it on a lockdep kernel (real allocation failures under memory pressure
take the same path):
WARNING: drivers/gpu/drm/drm_gpuvm.c:1661 at drm_gpuvm_bo_put+0x2ec/0x660 [drm_gpuvm]
Call trace:
drm_gpuvm_bo_put
pvr_vm_bind_op_fini
pvr_vm_map
pvr_ioctl_vm_map
Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
drivers/gpu/drm/imagination/pvr_vm.c | 37 +++++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
index cbdd15ed74f..3921b765688 100644
--- 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);
@@ -255,11 +255,11 @@ pvr_vm_bind_op_map_init(struct pvr_vm_bind_op *bind_op,
bind_op->type = PVR_VM_BIND_TYPE_MAP;
- dma_resv_lock(obj->resv, NULL);
- bind_op->gpuvm_bo = drm_gpuvm_bo_obtain_locked(&vm_ctx->gpuvm_mgr, obj);
- dma_resv_unlock(obj->resv);
- if (IS_ERR(bind_op->gpuvm_bo))
- return PTR_ERR(bind_op->gpuvm_bo);
+ bind_op->gpuvm_bo = drm_gpuvm_bo_create(&vm_ctx->gpuvm_mgr, obj);
+ if (!bind_op->gpuvm_bo)
+ return -ENOMEM;
+
+ bind_op->gpuvm_bo = drm_gpuvm_bo_obtain_prealloc(bind_op->gpuvm_bo);
bind_op->new_va = kzalloc_obj(*bind_op->new_va);
bind_op->prev_va = kzalloc_obj(*bind_op->prev_va);
@@ -366,7 +366,11 @@ pvr_vm_gpuva_map(struct drm_gpuva_op *op, void *op_ctx)
return err;
drm_gpuva_map(&ctx->vm_ctx->gpuvm_mgr, &ctx->new_va->base, &op->map);
+
+ mutex_lock(&op->map.gem.obj->gpuva.lock);
drm_gpuva_link(&ctx->new_va->base, ctx->gpuvm_bo);
+ mutex_unlock(&op->map.gem.obj->gpuva.lock);
+
ctx->new_va = NULL;
return 0;
@@ -396,7 +400,7 @@ pvr_vm_gpuva_unmap(struct drm_gpuva_op *op, void *op_ctx)
return err;
drm_gpuva_unmap(&op->unmap);
- drm_gpuva_unlink(op->unmap.va);
+ drm_gpuva_unlink_defer(op->unmap.va);
kfree(to_pvr_vm_gpuva(op->unmap.va));
return 0;
@@ -418,6 +422,7 @@ pvr_vm_gpuva_unmap(struct drm_gpuva_op *op, void *op_ctx)
static int
pvr_vm_gpuva_remap(struct drm_gpuva_op *op, void *op_ctx)
{
+ struct drm_gem_object *obj = op->remap.unmap->va->gem.obj;
/* The split parts belong to the object of the mapping being split. */
struct drm_gpuvm_bo *vm_bo = op->remap.unmap->va->vm_bo;
struct pvr_vm_bind_op *ctx = op_ctx;
@@ -435,16 +440,20 @@ pvr_vm_gpuva_remap(struct drm_gpuva_op *op, void *op_ctx)
drm_gpuva_remap(&ctx->prev_va->base, &ctx->next_va->base, &op->remap);
if (op->remap.prev) {
+ mutex_lock(&obj->gpuva.lock);
drm_gpuva_link(&ctx->prev_va->base, vm_bo);
+ mutex_unlock(&obj->gpuva.lock);
ctx->prev_va = NULL;
}
if (op->remap.next) {
+ mutex_lock(&obj->gpuva.lock);
drm_gpuva_link(&ctx->next_va->base, vm_bo);
+ mutex_unlock(&obj->gpuva.lock);
ctx->next_va = NULL;
}
- drm_gpuva_unlink(op->remap.unmap->va);
+ drm_gpuva_unlink_defer(op->remap.unmap->va);
kfree(to_pvr_vm_gpuva(op->remap.unmap->va));
return 0;
@@ -588,7 +597,8 @@ pvr_vm_create_context(struct pvr_device *pvr_dev, bool is_userspace_context)
drm_gem_private_object_init(&pvr_dev->base, &vm_ctx->dummy_gem, 0);
drm_gpuvm_init(&vm_ctx->gpuvm_mgr,
is_userspace_context ? "PowerVR-user-VM" : "PowerVR-FW-VM",
- 0, &pvr_dev->base, &vm_ctx->dummy_gem,
+ DRM_GPUVM_IMMEDIATE_MODE, &pvr_dev->base,
+ &vm_ctx->dummy_gem,
0, 1ULL << device_addr_bits, 0, 0, &pvr_vm_gpuva_ops);
mutex_init(&vm_ctx->lock);
@@ -622,6 +632,7 @@ pvr_vm_context_release(struct kref *ref_count)
pvr_fw_object_destroy(vm_ctx->fw_mem_ctx_obj);
pvr_vm_unmap_all(vm_ctx);
+ drm_gpuvm_bo_deferred_cleanup(&vm_ctx->gpuvm_mgr);
pvr_mmu_context_destroy(vm_ctx->mmu_ctx);
drm_gem_private_object_fini(&vm_ctx->dummy_gem);
@@ -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;
}
@@ -840,6 +853,8 @@ pvr_vm_unmap_obj(struct pvr_vm_context *vm_ctx, struct pvr_gem_object *pvr_obj,
err = pvr_vm_unmap_obj_locked(vm_ctx, pvr_obj, device_addr, size);
mutex_unlock(&vm_ctx->lock);
+ drm_gpuvm_bo_deferred_cleanup(&vm_ctx->gpuvm_mgr);
+
return err;
}
@@ -874,6 +889,8 @@ pvr_vm_unmap(struct pvr_vm_context *vm_ctx, u64 device_addr, u64 size)
mutex_unlock(&vm_ctx->lock);
+ drm_gpuvm_bo_deferred_cleanup(&vm_ctx->gpuvm_mgr);
+
return err;
}
@@ -906,6 +923,8 @@ pvr_vm_unmap_all(struct pvr_vm_context *vm_ctx)
}
mutex_unlock(&vm_ctx->lock);
+
+ drm_gpuvm_bo_deferred_cleanup(&vm_ctx->gpuvm_mgr);
}
/* Static data areas are determined by firmware. */
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] drm/imagination: Stop locking reservations around map and unmap
2026-10-10 17:22 [PATCH 0/2] drm/imagination: Switch the GPUVM to immediate mode Gyeyoung Baek
2026-10-10 17:22 ` [PATCH 1/2] " Gyeyoung Baek
@ 2026-10-10 17:22 ` Gyeyoung Baek
2026-10-10 17:36 ` sashiko-bot
1 sibling, 1 reply; 6+ messages in thread
From: Gyeyoung Baek @ 2026-10-10 17:22 UTC (permalink / raw)
To: Alessio Belle, Luigi Santivetti, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: Alexandru Dadu, Brajesh Gupta, Frank Binns, Danilo Krummrich,
Alice Ryhl, imagination, dri-devel, linux-kernel, Gyeyoung Baek
The GPUVA lists are now protected by gpuva.lock instead of the
reservations, so map and unmap no longer need to lock the reservations.
Nothing else on that path relies on them.
Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
drivers/gpu/drm/imagination/pvr_vm.c | 43 ------------------------------------
1 file changed, 43 deletions(-)
diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
index 3921b765688..2f1bbcfdb60 100644
--- a/drivers/gpu/drm/imagination/pvr_vm.c
+++ b/drivers/gpu/drm/imagination/pvr_vm.c
@@ -702,16 +702,6 @@ void pvr_destroy_vm_contexts_for_file(struct pvr_file *pvr_file)
}
}
-static int
-pvr_vm_lock_extra(struct drm_gpuvm_exec *vm_exec)
-{
- struct pvr_vm_bind_op *bind_op = vm_exec->extra.priv;
- struct pvr_gem_object *pvr_obj = bind_op->pvr_obj;
-
- /* Acquire lock on the GEM object being mapped/unmapped. */
- return drm_exec_lock_obj(&vm_exec->exec, gem_from_pvr_gem(pvr_obj));
-}
-
/**
* pvr_vm_map() - Map a section of physical memory into a section of
* device-virtual memory.
@@ -739,15 +729,6 @@ pvr_vm_map(struct pvr_vm_context *vm_ctx, struct pvr_gem_object *pvr_obj,
u64 pvr_obj_offset, u64 device_addr, u64 size)
{
struct pvr_vm_bind_op bind_op = {0};
- struct drm_gpuvm_exec vm_exec = {
- .vm = &vm_ctx->gpuvm_mgr,
- .flags = DRM_EXEC_INTERRUPTIBLE_WAIT |
- DRM_EXEC_IGNORE_DUPLICATES,
- .extra = {
- .fn = pvr_vm_lock_extra,
- .priv = &bind_op,
- },
- };
int err = pvr_vm_bind_op_map_init(&bind_op, vm_ctx, pvr_obj,
pvr_obj_offset, device_addr,
@@ -759,15 +740,7 @@ pvr_vm_map(struct pvr_vm_context *vm_ctx, struct pvr_gem_object *pvr_obj,
pvr_gem_object_get(pvr_obj);
mutex_lock(&vm_ctx->lock);
- err = drm_gpuvm_exec_lock(&vm_exec);
- if (err)
- goto err_cleanup;
-
err = pvr_vm_bind_op_exec(&bind_op);
-
- drm_gpuvm_exec_unlock(&vm_exec);
-
-err_cleanup:
mutex_unlock(&vm_ctx->lock);
pvr_vm_bind_op_fini(&bind_op);
@@ -800,15 +773,6 @@ pvr_vm_unmap_obj_locked(struct pvr_vm_context *vm_ctx,
u64 device_addr, u64 size)
{
struct pvr_vm_bind_op bind_op = {0};
- struct drm_gpuvm_exec vm_exec = {
- .vm = &vm_ctx->gpuvm_mgr,
- .flags = DRM_EXEC_INTERRUPTIBLE_WAIT |
- DRM_EXEC_IGNORE_DUPLICATES,
- .extra = {
- .fn = pvr_vm_lock_extra,
- .priv = &bind_op,
- },
- };
int err = pvr_vm_bind_op_unmap_init(&bind_op, vm_ctx, pvr_obj,
device_addr, size);
@@ -817,15 +781,8 @@ pvr_vm_unmap_obj_locked(struct pvr_vm_context *vm_ctx,
pvr_gem_object_get(pvr_obj);
- err = drm_gpuvm_exec_lock(&vm_exec);
- if (err)
- goto err_cleanup;
-
err = pvr_vm_bind_op_exec(&bind_op);
- drm_gpuvm_exec_unlock(&vm_exec);
-
-err_cleanup:
pvr_vm_bind_op_fini(&bind_op);
return err;
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] drm/imagination: Switch the GPUVM to immediate mode
2026-10-10 17:22 ` [PATCH 1/2] " Gyeyoung Baek
@ 2026-10-10 17:36 ` sashiko-bot
2026-10-11 5:31 ` Gyeyoung Baek
0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-10-10 17:36 UTC (permalink / raw)
To: Gyeyoung Baek; +Cc: dri-devel
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 <gye976@gmail.com>
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/20261011-pvr-gpuvm-immediate-v1-0-5b298dfce4f8@gmail.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/imagination: Stop locking reservations around map and unmap
2026-10-10 17:22 ` [PATCH 2/2] drm/imagination: Stop locking reservations around map and unmap Gyeyoung Baek
@ 2026-10-10 17:36 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-10-10 17:36 UTC (permalink / raw)
To: Gyeyoung Baek; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/imagination: missing cleanup of deferred GPUVM BOs in pvr_vm_map()
--
commit 7b2c8feb3b00c98681f3c0f485d9d8ac9c58c5eb
Author: Gyeyoung Baek <gye976@gmail.com>
drm/imagination: Stop locking reservations around map and unmap
The GPUVA lists are now protected by gpuva.lock instead of the
reservations, so map and unmap no longer need to lock the reservations.
Nothing else on that path relies on them.
> diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
> index 3921b765688e1..2f1bbcfdb60e3 100644
> --- a/drivers/gpu/drm/imagination/pvr_vm.c
> +++ b/drivers/gpu/drm/imagination/pvr_vm.c
[ ... ]
> @@ -739,15 +729,6 @@ pvr_vm_map(struct pvr_vm_context *vm_ctx, struct pvr_gem_object *pvr_obj,
> u64 pvr_obj_offset, u64 device_addr, u64 size)
> {
> struct pvr_vm_bind_op bind_op = {0};
> - struct drm_gpuvm_exec vm_exec = {
> - .vm = &vm_ctx->gpuvm_mgr,
> - .flags = DRM_EXEC_INTERRUPTIBLE_WAIT |
> - DRM_EXEC_IGNORE_DUPLICATES,
> - .extra = {
> - .fn = pvr_vm_lock_extra,
> - .priv = &bind_op,
> - },
> - };
>
> int err = pvr_vm_bind_op_map_init(&bind_op, vm_ctx, pvr_obj,
> pvr_obj_offset, device_addr,
[Severity: Medium]
If pvr_vm_bind_op_map_init() fails after creating a gpuvm_bo, does this code
leak the deferred BO?
Looking at pvr_vm_bind_op_map_init(), if it fails (e.g., due to ENOMEM or an
import mapping failure in pvr_gem_object_get_pages_sgt()), the error path
calls pvr_vm_bind_op_fini() which defers the BO cleanup:
pvr_vm_bind_op_fini() {
drm_gpuvm_bo_put_deferred(bind_op->gpuvm_bo);
...
}
However, the early return path in pvr_vm_map() (right after the call to
pvr_vm_bind_op_map_init()) skips calling drm_gpuvm_bo_deferred_cleanup():
int err = pvr_vm_bind_op_map_init(&bind_op, vm_ctx, pvr_obj,
pvr_obj_offset, device_addr,
size);
if (err)
return err;
Since pvr_vm_map() is reachable via the PVR_IOCTL_VM_MAP ioctl, could a faulty
or malicious userspace trigger this error path repeatedly and cause the deferred
list to grow unbounded, leaking kernel memory until the VM context is destroyed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261011-pvr-gpuvm-immediate-v1-0-5b298dfce4f8@gmail.com?part=2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] drm/imagination: Switch the GPUVM to immediate mode
2026-10-10 17:36 ` sashiko-bot
@ 2026-10-11 5:31 ` Gyeyoung Baek
0 siblings, 0 replies; 6+ messages in thread
From: Gyeyoung Baek @ 2026-10-11 5:31 UTC (permalink / raw)
To: sashiko-reviews; +Cc: dri-devel
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 <sashiko-bot@kernel.org> 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 <gye976@gmail.com>
>
> 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/20261011-pvr-gpuvm-immediate-v1-0-5b298dfce4f8@gmail.com?part=1
--
Thanks,
Gyeyoung
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-11 5:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-10 17:22 [PATCH 0/2] drm/imagination: Switch the GPUVM to immediate mode Gyeyoung Baek
2026-10-10 17:22 ` [PATCH 1/2] " Gyeyoung Baek
2026-10-10 17:36 ` sashiko-bot
2026-10-11 5:31 ` Gyeyoung Baek
2026-10-10 17:22 ` [PATCH 2/2] drm/imagination: Stop locking reservations around map and unmap Gyeyoung Baek
2026-10-10 17:36 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox