Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Unbounded memory leak of transient `struct drm_gpuva_op` objects due to bump allocator usage without intermediate resets. - [Low] Undeclared static key variable 'gpuvm_scratchpad_key' used in drm_gpuvm_init(). --
--- Patch [4]: [PATCH 4/9] drm/gpuvm: Route gpuva_op allocations to folio_scratchpad --- commit 6e27181fef6cb87688764c9745ab2086b2091e88 Author: Jim Cromie <[email protected]> Subject: drm/gpuvm: Route gpuva_op allocations to folio_scratchpad This commit modifies the DRM GPUVM subsystem to route default gpuva_op allocations to a folio scratchpad embedded in struct drm_gpuvm. It updates gpuva_op_alloc() to use folio_scratchpad_alloc_obj() and handles the bulk teardown during drm_gpuvm_fini(). > diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c > --- a/drivers/gpu/drm/drm_gpuvm.c > +++ b/drivers/gpu/drm/drm_gpuvm.c > @@ -1109,6 +1109,8 @@ drm_gpuvm_init(struct drm_gpuvm *gpuvm, const char > *name, > spin_lock_init(&gpuvm->evict.lock); > > init_llist_head(&gpuvm->bo_defer); > + folio_scratchpad_init_key(&gpuvm->va_scratchpad, get_order(SZ_64K), > + &gpuvm_scratchpad_key); [Severity: Low] Does this cause a compilation failure? In drm_gpuvm_init(), the code references gpuvm_scratchpad_key, but this static key is not defined until later in the file via the DEFINE_FOLIO_POOL_STATIC_KEY_PARAM macro. [ ... ] > @@ -2814,7 +2820,8 @@ gpuva_op_alloc(struct drm_gpuvm *gpuvm) > if (fn && fn->op_alloc) > op = fn->op_alloc(); > else > - op = kzalloc_obj(*op); > + op = folio_scratchpad_alloc_obj(gpuvm, va_scratchpad, > + struct drm_gpuva_op, > GFP_KERNEL); > > if (unlikely(!op)) > return NULL; > @@ -2831,7 +2838,7 @@ gpuva_op_free(struct drm_gpuvm *gpuvm, > if (fn && fn->op_free) > fn->op_free(op); > else > - kfree(op); > + folio_scratchpad_free_elem(op); > } [Severity: High] Will this cause an unbounded memory leak for long-lived VMs? In gpuva_op_alloc(), struct drm_gpuva_op objects are allocated using gpuvm->va_scratchpad, which acts as a bump allocator. However, the folio_scratchpad_free_elem() called here in gpuva_op_free() cannot reclaim individual elements from a scratchpad bump allocator. Since the memory is only fully reclaimed during bulk teardown in drm_gpuvm_fini(), and these objects are transiently allocated per map/unmap operation, could this result in continuous memory leakage over the lifetime of the GPU VM? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
