* Refcounting dma_resv v2
@ 2026-08-27 12:38 Christian König
2026-08-27 12:38 ` [PATCH 01/10] dma-buf: Add reference counting to dma_resv Christian König
` (10 more replies)
0 siblings, 11 replies; 25+ messages in thread
From: Christian König @ 2026-08-27 12:38 UTC (permalink / raw)
To: thomas.hellstrom, dakr, ecourtney, matthew.brost, nat, dri-devel,
intel-gfx, intel-xe, amd-gfx
Hi everybody,
The idea of ref-counting dma_resv or ww_mutex came up multiple times from
different people, but so far at least I have abandoned that as to
complicated to implement considering how widely used that object is.
Thanks to AI I gave the task to refcount dma_resv to Claude Sonet 4 just
to check how horrible it would look like.
Well turns out that this is actually a cleanup we should most likely aim
for and I'm really wondering why we haven't done it like this in the
first place.
Not only resolves it a bunch of issues with dma_resv instances shared by
multiple GEM objects (we just recently had a bunch of patches for that on
the mailing list), but also allows TTM to implement it's delayed delete
handling without any zombie resurrection or similar hacks and DMA-buf to
have better contention handling on map/pin in the future.
This patch set here is now the idea full flashed out. I smoke tested it
with amdgpu, self tests and checked everything with kmemleak and of hand
it seems to work.
Patch #1 introduces an interim allocated flag to allow switching users
over to the new interface one by one. Patch #10 then removes that flag
again after everything is done.
While AI is an automated tool I distrust it to always generate the same
code. So better using a defensive approach on changing things.
Please review and comment.
Cheers,
Christian.
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH 01/10] dma-buf: Add reference counting to dma_resv 2026-08-27 12:38 Refcounting dma_resv v2 Christian König @ 2026-08-27 12:38 ` Christian König 2026-08-27 18:39 ` Andi Shyti 2026-08-28 18:21 ` Danilo Krummrich 2026-08-27 12:38 ` [PATCH 02/10] dma-buf/tests: Convert st-dma-resv tests to use dma_resv_alloc Christian König ` (9 subsequent siblings) 10 siblings, 2 replies; 25+ messages in thread From: Christian König @ 2026-08-27 12:38 UTC (permalink / raw) To: thomas.hellstrom, dakr, ecourtney, matthew.brost, nat, dri-devel, intel-gfx, intel-xe, amd-gfx Introduce reference counting for dma_resv objects to better manage their lifecycle. This replaces the previous approach where dma_buf would either embed a dma_resv or allocate it inline using a size trick. Add three new functions: - dma_resv_alloc(): allocates and initializes a dma_resv with refcount - dma_resv_get(): acquires a reference to a dma_resv - dma_resv_put(): releases a reference, freeing when count reaches zero Update all callers to use dma_resv_put() instead of dma_resv_fini(), which now becomes an internal cleanup function. The dma_buf export path now explicitly allocates the dma_resv when needed rather than using pointer arithmetic tricks. This provides clearer ownership semantics and makes the code more maintainable by removing the embedded allocation hack. Signed-off-by: Christian König <christian.koenig@amd.com> Assisted-by: Claude:Sonnet 4 --- drivers/dma-buf/dma-buf.c | 24 +++---- drivers/dma-buf/dma-resv.c | 80 +++++++++++++++++++--- drivers/dma-buf/st-dma-resv.c | 10 +-- drivers/gpu/drm/drm_gem.c | 2 +- drivers/gpu/drm/drm_mode_config.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_object.c | 2 +- drivers/gpu/drm/i915/gt/intel_ggtt.c | 4 +- drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +- drivers/gpu/drm/nouveau/nouveau_bo.c | 2 +- drivers/gpu/drm/ttm/tests/ttm_bo_test.c | 2 +- drivers/gpu/drm/ttm/ttm_bo_util.c | 2 +- include/linux/dma-resv.h | 22 +++++- 12 files changed, 117 insertions(+), 37 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index d504c636dc29..53e428fdaf6b 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -182,8 +182,7 @@ static void dma_buf_release(struct dentry *dentry) dmabuf->ops->release(dmabuf); - if (dmabuf->resv == (struct dma_resv *)&dmabuf[1]) - dma_resv_fini(dmabuf->resv); + dma_resv_put(dmabuf->resv); WARN_ON(!list_empty(&dmabuf->attachments)); module_put(dmabuf->owner); @@ -707,10 +706,9 @@ static struct file *dma_buf_getfile(size_t size, int flags) */ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) { - struct dma_buf *dmabuf; struct dma_resv *resv = exp_info->resv; + struct dma_buf *dmabuf; struct file *file; - size_t alloc_size = sizeof(struct dma_buf); int ret; if (WARN_ON(!exp_info->priv || !exp_info->ops @@ -731,12 +729,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) goto err_module; } - if (!exp_info->resv) - alloc_size += sizeof(struct dma_resv); - else - /* prevent &dma_buf[1] == dma_buf->resv */ - alloc_size += 1; - dmabuf = kzalloc(alloc_size, GFP_KERNEL); + dmabuf = kzalloc_obj(*dmabuf); if (!dmabuf) { ret = -ENOMEM; goto err_file; @@ -754,10 +747,13 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) INIT_LIST_HEAD(&dmabuf->attachments); if (!resv) { - dmabuf->resv = (struct dma_resv *)&dmabuf[1]; - dma_resv_init(dmabuf->resv); + dmabuf->resv = dma_resv_alloc(); + if (!dmabuf->resv) { + ret = -ENOMEM; + goto err_dmabuf; + } } else { - dmabuf->resv = resv; + dmabuf->resv = dma_resv_get(resv); } file->private_data = dmabuf; @@ -770,6 +766,8 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) return dmabuf; +err_dmabuf: + kfree(dmabuf); err_file: fput(file); err_module: diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 2365a6d8ca15..48798cec6ce7 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -36,6 +36,7 @@ #include <linux/dma-resv.h> #include <linux/dma-fence-array.h> #include <linux/export.h> +#include <linux/kref.h> #include <linux/mm.h> #include <linux/sched/mm.h> #include <linux/mmu_notifier.h> @@ -137,26 +138,87 @@ static void dma_resv_list_free(struct dma_resv_list *list) */ void dma_resv_init(struct dma_resv *obj) { + kref_init(&obj->refcount); + obj->allocated = false; ww_mutex_init(&obj->lock, &reservation_ww_class); RCU_INIT_POINTER(obj->fences, NULL); } EXPORT_SYMBOL(dma_resv_init); -/** - * dma_resv_fini - destroys a reservation object - * @obj: the reservation object +/* + * dma_resv_release - release function for kref + * @kref: the kref inside the dma_resv object + * + * This is called when the last reference to a dma_resv object is released. + * Cleans up the object and frees it if it was allocated by dma_resv_alloc(). */ -void dma_resv_fini(struct dma_resv *obj) +static void dma_resv_release(struct kref *kref) { - /* - * This object should be dead and all references must have - * been released to it, so no need to be protected with rcu. - */ + struct dma_resv *obj = container_of(kref, struct dma_resv, refcount); + dma_resv_list_free(rcu_dereference_protected(obj->fences, true)); ww_mutex_destroy(&obj->lock); + if (obj->allocated) + kfree(obj); +} + +/** + * dma_resv_alloc - allocate and initialize a reservation object + * + * Allocates a new dma_resv object, initializes it, and returns it with a + * reference count of 1. The object must be freed with dma_resv_put() when + * no longer needed. + * + * Returns: + * A pointer to the allocated dma_resv object, or NULL on allocation failure. + */ +struct dma_resv *dma_resv_alloc(void) +{ + struct dma_resv *obj; + + obj = kzalloc_obj(*obj); + if (!obj) + return NULL; + + dma_resv_init(obj); + obj->allocated = true; + + return obj; +} +EXPORT_SYMBOL(dma_resv_alloc); + +/** + * dma_resv_get - acquire a reference to a reservation object + * @obj: the reservation object + * + * Increments the reference count on the dma_resv object. + * + * Returns: + * The dma_resv object pointer for convenience. + */ +struct dma_resv *dma_resv_get(struct dma_resv *obj) +{ + if (obj) + kref_get(&obj->refcount); + return obj; +} +EXPORT_SYMBOL(dma_resv_get); + +/** + * dma_resv_put - release a reference to a reservation object + * @obj: the reservation object + * + * Decrements the reference count on the dma_resv object. When the reference + * count reaches zero, the object is cleaned up with dma_resv_fini() and freed + * if it was allocated by dma_resv_alloc(). + */ +void dma_resv_put(struct dma_resv *obj) +{ + if (obj) + kref_put(&obj->refcount, dma_resv_release); } -EXPORT_SYMBOL(dma_resv_fini); +EXPORT_SYMBOL(dma_resv_put); /* Dereference the fences while ensuring RCU rules */ static inline struct dma_resv_list *dma_resv_fences_list(struct dma_resv *obj) diff --git a/drivers/dma-buf/st-dma-resv.c b/drivers/dma-buf/st-dma-resv.c index 0b96136bbd54..2c43d2d2b016 100644 --- a/drivers/dma-buf/st-dma-resv.c +++ b/drivers/dma-buf/st-dma-resv.c @@ -59,7 +59,7 @@ static void test_sanitycheck(struct kunit *test) KUNIT_FAIL(test, "Resv locking failed\n"); else dma_resv_unlock(&resv); - dma_resv_fini(&resv); + dma_resv_put(&resv); } static void test_signaling(struct kunit *test) @@ -101,7 +101,7 @@ static void test_signaling(struct kunit *test) err_unlock: dma_resv_unlock(&resv); err_free: - dma_resv_fini(&resv); + dma_resv_put(&resv); dma_fence_put(f); } @@ -160,7 +160,7 @@ static void test_for_each(struct kunit *test) err_unlock: dma_resv_unlock(&resv); err_free: - dma_resv_fini(&resv); + dma_resv_put(&resv); dma_fence_put(f); } @@ -231,7 +231,7 @@ static void test_for_each_unlocked(struct kunit *test) dma_resv_iter_end(&cursor); dma_fence_signal(f); err_free: - dma_resv_fini(&resv); + dma_resv_put(&resv); dma_fence_put(f); } @@ -282,7 +282,7 @@ static void test_get_fences(struct kunit *test) dma_fence_put(fences[i]); kfree(fences); err_resv: - dma_resv_fini(&resv); + dma_resv_put(&resv); dma_fence_put(f); } diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 018df97d590d..48176a11d552 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -249,7 +249,7 @@ void drm_gem_private_object_fini(struct drm_gem_object *obj) { WARN_ON(obj->dma_buf); - dma_resv_fini(&obj->_resv); + dma_resv_put(&obj->_resv); mutex_destroy(&obj->gpuva.lock); } EXPORT_SYMBOL(drm_gem_private_object_fini); diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 3bcc7bf0900c..312d5e9675f0 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -681,7 +681,7 @@ int drmm_mode_config_init(struct drm_device *dev) drm_modeset_drop_locks(&modeset_ctx); drm_modeset_acquire_fini(&modeset_ctx); - dma_resv_fini(&resv); + dma_resv_put(&resv); } return drmm_add_action_or_reset(dev, drm_mode_config_init_release, diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index 5172d3982654..384c74794fdc 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -144,7 +144,7 @@ void __i915_gem_object_fini(struct drm_i915_gem_object *obj) { mutex_destroy(&obj->mm.get_page.lock); mutex_destroy(&obj->mm.get_dma_page.lock); - dma_resv_fini(&obj->base._resv); + dma_resv_put(&obj->base._resv); } /** diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 64ca5bbc53c6..3cc8df7b8fad 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -1140,7 +1140,7 @@ void i915_ggtt_driver_late_release(struct drm_i915_private *i915) struct i915_ggtt *ggtt = to_gt(i915)->ggtt; GEM_WARN_ON(kref_read(&ggtt->vm.resv_ref) != 1); - dma_resv_fini(&ggtt->vm._resv); + dma_resv_put(&ggtt->vm._resv); } static unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl) @@ -1524,7 +1524,7 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct intel_gt *gt) ret = intel_ggtt_gmch_probe(ggtt); if (ret) { - dma_resv_fini(&ggtt->vm._resv); + dma_resv_put(&ggtt->vm._resv); return ret; } diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c index afbc5c769308..7b1bdb121c88 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.c +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c @@ -225,7 +225,7 @@ void i915_vm_resv_release(struct kref *kref) struct i915_address_space *vm = container_of(kref, typeof(*vm), resv_ref); - dma_resv_fini(&vm->_resv); + dma_resv_put(&vm->_resv); mutex_destroy(&vm->mutex); kfree(vm); diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 0e8de6d4b36f..67c9d32f4f27 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -160,7 +160,7 @@ nouveau_bo_del_ttm(struct ttm_buffer_object *bo) drm_gem_object_release(&bo->base); } else { - dma_resv_fini(&bo->base._resv); + dma_resv_put(&bo->base._resv); } kfree(nvbo); diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c index f3103307b5df..49b0b48c6c2a 100644 --- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c @@ -376,7 +376,7 @@ static void ttm_bo_unreserve_bulk(struct kunit *test) ttm_resource_free(bo1, &res1); ttm_resource_free(bo2, &res2); - dma_resv_fini(resv); + dma_resv_put(resv); } static void ttm_bo_fini_basic(struct kunit *test) diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index bd0b23ac2cc4..af5732f9e489 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -207,7 +207,7 @@ static void ttm_transfered_destroy(struct ttm_buffer_object *bo) struct ttm_transfer_obj *fbo; fbo = container_of(bo, struct ttm_transfer_obj, base); - dma_resv_fini(&fbo->base.base._resv); + dma_resv_put(&fbo->base.base._resv); ttm_bo_put(fbo->bo); kfree(fbo); } diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h index c5ab6fd9ebe8..4d12519df34e 100644 --- a/include/linux/dma-resv.h +++ b/include/linux/dma-resv.h @@ -44,6 +44,7 @@ #include <linux/slab.h> #include <linux/seqlock.h> #include <linux/rcupdate.h> +#include <linux/kref.h> extern struct ww_class reservation_ww_class; @@ -153,6 +154,23 @@ static inline enum dma_resv_usage dma_resv_usage_rw(bool write) * drm_gem_object with the same scheme. */ struct dma_resv { + /** + * @refcount: + * + * Reference count for this reservation object. The object is freed + * when the reference count reaches zero via dma_resv_put(). + */ + struct kref refcount; + + /** + * @allocated: + * + * True if this object was allocated by dma_resv_alloc(), false if + * embedded in another structure. Used to determine whether to free + * the object memory in the release function. + */ + bool allocated; + /** * @lock: * @@ -465,7 +483,9 @@ static inline void dma_resv_unlock(struct dma_resv *obj) } void dma_resv_init(struct dma_resv *obj); -void dma_resv_fini(struct dma_resv *obj); +struct dma_resv *dma_resv_alloc(void); +struct dma_resv *dma_resv_get(struct dma_resv *obj); +void dma_resv_put(struct dma_resv *obj); int dma_resv_reserve_fences(struct dma_resv *obj, unsigned int num_fences); void dma_resv_add_fence(struct dma_resv *obj, struct dma_fence *fence, enum dma_resv_usage usage); -- 2.43.0 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 01/10] dma-buf: Add reference counting to dma_resv 2026-08-27 12:38 ` [PATCH 01/10] dma-buf: Add reference counting to dma_resv Christian König @ 2026-08-27 18:39 ` Andi Shyti 2026-08-28 18:21 ` Danilo Krummrich 1 sibling, 0 replies; 25+ messages in thread From: Andi Shyti @ 2026-08-27 18:39 UTC (permalink / raw) To: christian.koenig Cc: thomas.hellstrom, dakr, ecourtney, matthew.brost, nat, dri-devel, intel-gfx, intel-xe, amd-gfx Hi Christian, > +/** > + * dma_resv_put - release a reference to a reservation object > + * @obj: the reservation object > + * > + * Decrements the reference count on the dma_resv object. When the reference > + * count reaches zero, the object is cleaned up with dma_resv_fini() and freed except that you have removed dma_resv_fini() :-) Probably you want to say that the object is cleaned up by dma_resv_release()? Andi > + * if it was allocated by dma_resv_alloc(). > + */ > +void dma_resv_put(struct dma_resv *obj) > +{ > + if (obj) > + kref_put(&obj->refcount, dma_resv_release); > } > -EXPORT_SYMBOL(dma_resv_fini); > +EXPORT_SYMBOL(dma_resv_put); ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 01/10] dma-buf: Add reference counting to dma_resv 2026-08-27 12:38 ` [PATCH 01/10] dma-buf: Add reference counting to dma_resv Christian König 2026-08-27 18:39 ` Andi Shyti @ 2026-08-28 18:21 ` Danilo Krummrich 2026-08-28 19:17 ` Matthew Brost 1 sibling, 1 reply; 25+ messages in thread From: Danilo Krummrich @ 2026-08-28 18:21 UTC (permalink / raw) To: Christian König Cc: christian.koenig, thomas.hellstrom, ecourtney, matthew.brost, nat, dri-devel, intel-gfx, intel-xe, amd-gfx On Thu Aug 27, 2026 at 2:38 PM CEST, =?UTF-8?q?Christian=20K=C3=B6nig?= wrote: > This provides clearer ownership semantics and makes the code more > maintainable by removing the embedded allocation hack. Again, fundamentally disagree; please see [1] for details and reasons. Besides that, I don't really know what to expect from the series. The cover letter says "Refcounting dma_resv v2", yet there's no changelog telling people what has changed since v1. On the other hand, none of the patches has a v2 prefix, so what is it? [1] https://lore.kernel.org/all/DJVRML4G11GZ.Z7FTZH7I1LZN@kernel.org/ ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 01/10] dma-buf: Add reference counting to dma_resv 2026-08-28 18:21 ` Danilo Krummrich @ 2026-08-28 19:17 ` Matthew Brost 2026-08-28 19:34 ` Matthew Brost 0 siblings, 1 reply; 25+ messages in thread From: Matthew Brost @ 2026-08-28 19:17 UTC (permalink / raw) To: Danilo Krummrich Cc: Christian König, christian.koenig, thomas.hellstrom, ecourtney, nat, dri-devel, intel-gfx, intel-xe, amd-gfx On Fri, Aug 28, 2026 at 08:21:17PM +0200, Danilo Krummrich wrote: > On Thu Aug 27, 2026 at 2:38 PM CEST, =?UTF-8?q?Christian=20K=C3=B6nig?= wrote: > > This provides clearer ownership semantics and makes the code more > > maintainable by removing the embedded allocation hack. > > Again, fundamentally disagree; please see [1] for details and reasons. > I think Christian is doing this incrementally rather than as a single change across the tree. The last patch [2] in the series removes `dma_resv_init` and the `allocated` flag. I believe that was the main concern, and it appears to be addressed now. I'm taking the series for a spin now, as our CI [3] had trouble applying this patch. I'll let you know what my testing shows. [2] https://patchwork.freedesktop.org/patch/748868/?series=172875&rev=1 [3] https://patchwork.freedesktop.org/series/172873/ > Besides that, I don't really know what to expect from the series. The cover > letter says "Refcounting dma_resv v2", yet there's no changelog telling people > what has changed since v1. On the other hand, none of the patches has a v2 > prefix, so what is it? Yes, change logs / cover-letter would help as Danilo likely could have spotted the above change. Matt > > [1] https://lore.kernel.org/all/DJVRML4G11GZ.Z7FTZH7I1LZN@kernel.org/ ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 01/10] dma-buf: Add reference counting to dma_resv 2026-08-28 19:17 ` Matthew Brost @ 2026-08-28 19:34 ` Matthew Brost 0 siblings, 0 replies; 25+ messages in thread From: Matthew Brost @ 2026-08-28 19:34 UTC (permalink / raw) To: Danilo Krummrich Cc: Christian König, christian.koenig, thomas.hellstrom, ecourtney, nat, dri-devel, intel-gfx, intel-xe, amd-gfx On Fri, Aug 28, 2026 at 12:17:57PM -0700, Matthew Brost wrote: > On Fri, Aug 28, 2026 at 08:21:17PM +0200, Danilo Krummrich wrote: > > On Thu Aug 27, 2026 at 2:38 PM CEST, =?UTF-8?q?Christian=20K=C3=B6nig?= wrote: > > > This provides clearer ownership semantics and makes the code more > > > maintainable by removing the embedded allocation hack. > > > > Again, fundamentally disagree; please see [1] for details and reasons. > > > > I think Christian is doing this incrementally rather than as a single > change across the tree. The last patch [2] in the series removes > `dma_resv_init` and the `allocated` flag. I believe that was the main > concern, and it appears to be addressed now. > > I'm taking the series for a spin now, as our CI [3] had trouble applying > this patch. I'll let you know what my testing shows. > I ran our basic CI suite and didn't see any regressions. I also ran some eviction and shrinker tests that stress interactions with active fences and exercise individualization paths, and those passed as well. Matt > [2] https://patchwork.freedesktop.org/patch/748868/?series=172875&rev=1 > [3] https://patchwork.freedesktop.org/series/172873/ > > > Besides that, I don't really know what to expect from the series. The cover > > letter says "Refcounting dma_resv v2", yet there's no changelog telling people > > what has changed since v1. On the other hand, none of the patches has a v2 > > prefix, so what is it? > > Yes, change logs / cover-letter would help as Danilo likely could have > spotted the above change. > > Matt > > > > > [1] https://lore.kernel.org/all/DJVRML4G11GZ.Z7FTZH7I1LZN@kernel.org/ ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 02/10] dma-buf/tests: Convert st-dma-resv tests to use dma_resv_alloc 2026-08-27 12:38 Refcounting dma_resv v2 Christian König 2026-08-27 12:38 ` [PATCH 01/10] dma-buf: Add reference counting to dma_resv Christian König @ 2026-08-27 12:38 ` Christian König 2026-08-27 13:08 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 03/10] drm/gem: Add helper for drm_gem_object resv assignment Christian König ` (8 subsequent siblings) 10 siblings, 1 reply; 25+ messages in thread From: Christian König @ 2026-08-27 12:38 UTC (permalink / raw) To: thomas.hellstrom, dakr, ecourtney, matthew.brost, nat, dri-devel, intel-gfx, intel-xe, amd-gfx Update all test cases in st-dma-resv.c to use the new dma_resv_alloc() API instead of stack-allocating dma_resv objects. This demonstrates the proper usage of the new allocation and reference counting interface. All five test functions now: - Use dma_resv_alloc() to allocate objects - Check for allocation failure with KUNIT_ASSERT_NOT_NULL - Clean up with dma_resv_put() which handles freeing This change also provides better test coverage for the reference counting implementation. Signed-off-by: Christian König <christian.koenig@amd.com> Assisted-by: Claude:Sonnet 4 --- drivers/dma-buf/st-dma-resv.c | 90 +++++++++++++++++++---------------- 1 file changed, 50 insertions(+), 40 deletions(-) diff --git a/drivers/dma-buf/st-dma-resv.c b/drivers/dma-buf/st-dma-resv.c index 2c43d2d2b016..b843d9041782 100644 --- a/drivers/dma-buf/st-dma-resv.c +++ b/drivers/dma-buf/st-dma-resv.c @@ -41,7 +41,7 @@ static struct dma_fence *alloc_fence(void) static void test_sanitycheck(struct kunit *test) { - struct dma_resv resv; + struct dma_resv *resv; struct dma_fence *f; int r; @@ -53,20 +53,22 @@ static void test_sanitycheck(struct kunit *test) dma_fence_signal(f); dma_fence_put(f); - dma_resv_init(&resv); - r = dma_resv_lock(&resv, NULL); + resv = dma_resv_alloc(); + KUNIT_ASSERT_NOT_NULL(test, resv); + + r = dma_resv_lock(resv, NULL); if (r) KUNIT_FAIL(test, "Resv locking failed\n"); else - dma_resv_unlock(&resv); - dma_resv_put(&resv); + dma_resv_unlock(resv); + dma_resv_put(resv); } static void test_signaling(struct kunit *test) { const struct dma_resv_usage_param *param = test->param_value; enum dma_resv_usage usage = param->usage; - struct dma_resv resv; + struct dma_resv *resv; struct dma_fence *f; int r; @@ -75,33 +77,35 @@ static void test_signaling(struct kunit *test) dma_fence_enable_signaling(f); - dma_resv_init(&resv); - r = dma_resv_lock(&resv, NULL); + resv = dma_resv_alloc(); + KUNIT_ASSERT_NOT_NULL(test, resv); + + r = dma_resv_lock(resv, NULL); if (r) { KUNIT_FAIL(test, "Resv locking failed"); goto err_free; } - r = dma_resv_reserve_fences(&resv, 1); + r = dma_resv_reserve_fences(resv, 1); if (r) { KUNIT_FAIL(test, "Resv shared slot allocation failed"); goto err_unlock; } - dma_resv_add_fence(&resv, f, usage); - if (dma_resv_test_signaled(&resv, usage)) { + dma_resv_add_fence(resv, f, usage); + if (dma_resv_test_signaled(resv, usage)) { KUNIT_FAIL(test, "Resv unexpectedly signaled"); goto err_unlock; } dma_fence_signal(f); - if (!dma_resv_test_signaled(&resv, usage)) { + if (!dma_resv_test_signaled(resv, usage)) { KUNIT_FAIL(test, "Resv not reporting signaled"); goto err_unlock; } err_unlock: - dma_resv_unlock(&resv); + dma_resv_unlock(resv); err_free: - dma_resv_put(&resv); + dma_resv_put(resv); dma_fence_put(f); } @@ -111,7 +115,7 @@ static void test_for_each(struct kunit *test) enum dma_resv_usage usage = param->usage; struct dma_resv_iter cursor; struct dma_fence *f, *fence; - struct dma_resv resv; + struct dma_resv *resv; int r; f = alloc_fence(); @@ -119,23 +123,25 @@ static void test_for_each(struct kunit *test) dma_fence_enable_signaling(f); - dma_resv_init(&resv); - r = dma_resv_lock(&resv, NULL); + resv = dma_resv_alloc(); + KUNIT_ASSERT_NOT_NULL(test, resv); + + r = dma_resv_lock(resv, NULL); if (r) { KUNIT_FAIL(test, "Resv locking failed"); goto err_free; } - r = dma_resv_reserve_fences(&resv, 1); + r = dma_resv_reserve_fences(resv, 1); if (r) { KUNIT_FAIL(test, "Resv shared slot allocation failed"); goto err_unlock; } - dma_resv_add_fence(&resv, f, usage); + dma_resv_add_fence(resv, f, usage); r = -ENOENT; - dma_resv_for_each_fence(&cursor, &resv, usage, fence) { + dma_resv_for_each_fence(&cursor, resv, usage, fence) { if (!r) { KUNIT_FAIL(test, "More than one fence found"); goto err_unlock; @@ -158,9 +164,9 @@ static void test_for_each(struct kunit *test) } dma_fence_signal(f); err_unlock: - dma_resv_unlock(&resv); + dma_resv_unlock(resv); err_free: - dma_resv_put(&resv); + dma_resv_put(resv); dma_fence_put(f); } @@ -170,7 +176,7 @@ static void test_for_each_unlocked(struct kunit *test) enum dma_resv_usage usage = param->usage; struct dma_resv_iter cursor; struct dma_fence *f, *fence; - struct dma_resv resv; + struct dma_resv *resv; int r; f = alloc_fence(); @@ -178,25 +184,27 @@ static void test_for_each_unlocked(struct kunit *test) dma_fence_enable_signaling(f); - dma_resv_init(&resv); - r = dma_resv_lock(&resv, NULL); + resv = dma_resv_alloc(); + KUNIT_ASSERT_NOT_NULL(test, resv); + + r = dma_resv_lock(resv, NULL); if (r) { KUNIT_FAIL(test, "Resv locking failed"); goto err_free; } - r = dma_resv_reserve_fences(&resv, 1); + r = dma_resv_reserve_fences(resv, 1); if (r) { KUNIT_FAIL(test, "Resv shared slot allocation failed"); - dma_resv_unlock(&resv); + dma_resv_unlock(resv); goto err_free; } - dma_resv_add_fence(&resv, f, usage); - dma_resv_unlock(&resv); + dma_resv_add_fence(resv, f, usage); + dma_resv_unlock(resv); r = -ENOENT; - dma_resv_iter_begin(&cursor, &resv, usage); + dma_resv_iter_begin(&cursor, resv, usage); dma_resv_for_each_fence_unlocked(&cursor, fence) { if (!r) { KUNIT_FAIL(test, "More than one fence found"); @@ -231,7 +239,7 @@ static void test_for_each_unlocked(struct kunit *test) dma_resv_iter_end(&cursor); dma_fence_signal(f); err_free: - dma_resv_put(&resv); + dma_resv_put(resv); dma_fence_put(f); } @@ -240,7 +248,7 @@ static void test_get_fences(struct kunit *test) const struct dma_resv_usage_param *param = test->param_value; enum dma_resv_usage usage = param->usage; struct dma_fence *f, **fences = NULL; - struct dma_resv resv; + struct dma_resv *resv; int r, i; f = alloc_fence(); @@ -248,24 +256,26 @@ static void test_get_fences(struct kunit *test) dma_fence_enable_signaling(f); - dma_resv_init(&resv); - r = dma_resv_lock(&resv, NULL); + resv = dma_resv_alloc(); + KUNIT_ASSERT_NOT_NULL(test, resv); + + r = dma_resv_lock(resv, NULL); if (r) { KUNIT_FAIL(test, "Resv locking failed"); goto err_resv; } - r = dma_resv_reserve_fences(&resv, 1); + r = dma_resv_reserve_fences(resv, 1); if (r) { KUNIT_FAIL(test, "Resv shared slot allocation failed"); - dma_resv_unlock(&resv); + dma_resv_unlock(resv); goto err_resv; } - dma_resv_add_fence(&resv, f, usage); - dma_resv_unlock(&resv); + dma_resv_add_fence(resv, f, usage); + dma_resv_unlock(resv); - r = dma_resv_get_fences(&resv, usage, &i, &fences); + r = dma_resv_get_fences(resv, usage, &i, &fences); if (r) { KUNIT_FAIL(test, "get_fences failed"); goto err_free; @@ -282,7 +292,7 @@ static void test_get_fences(struct kunit *test) dma_fence_put(fences[i]); kfree(fences); err_resv: - dma_resv_put(&resv); + dma_resv_put(resv); dma_fence_put(f); } -- 2.43.0 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 02/10] dma-buf/tests: Convert st-dma-resv tests to use dma_resv_alloc 2026-08-27 12:38 ` [PATCH 02/10] dma-buf/tests: Convert st-dma-resv tests to use dma_resv_alloc Christian König @ 2026-08-27 13:08 ` sashiko-bot 0 siblings, 0 replies; 25+ messages in thread From: sashiko-bot @ 2026-08-27 13:08 UTC (permalink / raw) To: Christian König; +Cc: dri-devel, intel-xe, intel-gfx Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Memory leak of manually allocated 'dma_fence' object on test failure path due to KUNIT_ASSERT_NOT_NULL aborting the test. -- commit 2140307021746c828365fffb6d1a6a49b2fe2aa6 Author: Christian König <ckoenig.leichtzumerken@gmail.com> dma-buf/tests: Convert st-dma-resv tests to use dma_resv_alloc This patch updates the test cases in st-dma-resv.c to use the new dma_resv_alloc API instead of stack-allocated objects. It introduces pointer allocations and adds reference counting verification by calling dma_resv_put for cleanup. > diff --git a/drivers/dma-buf/st-dma-resv.c b/drivers/dma-buf/st-dma-resv.c > index 2c43d2d2b0169..b843d90417822 100644 > --- a/drivers/dma-buf/st-dma-resv.c > +++ b/drivers/dma-buf/st-dma-resv.c [ ... ] > @@ -75,33 +77,35 @@ static void test_signaling(struct kunit *test) > > dma_fence_enable_signaling(f); > > - dma_resv_init(&resv); > - r = dma_resv_lock(&resv, NULL); > + resv = dma_resv_alloc(); > + KUNIT_ASSERT_NOT_NULL(test, resv); [Severity: Medium] Does this code leak the allocated dma_fence 'f' if the assertion fails? In test_signaling(), 'f' is allocated right before this code block. If dma_resv_alloc() fails due to memory pressure, KUNIT_ASSERT_NOT_NULL() will abort the test execution immediately. This bypasses the err_free cleanup label and leaves 'f' stranded without its corresponding dma_fence_put() call. Since this pattern is repeated in the other tests below, would it be possible to either reorder the allocations or use KUNIT_EXPECT_NOT_NULL() to allow for proper cleanup on failure? > + > + r = dma_resv_lock(resv, NULL); [ ... ] > @@ -119,23 +123,25 @@ static void test_for_each(struct kunit *test) > > dma_fence_enable_signaling(f); > > - dma_resv_init(&resv); > - r = dma_resv_lock(&resv, NULL); > + resv = dma_resv_alloc(); > + KUNIT_ASSERT_NOT_NULL(test, resv); [Severity: Medium] Similarly here in test_for_each(), does a failure on this assertion leak the 'f' object? > + > + r = dma_resv_lock(resv, NULL); [ ... ] > @@ -178,25 +184,27 @@ static void test_for_each_unlocked(struct kunit *test) > > dma_fence_enable_signaling(f); > > - dma_resv_init(&resv); > - r = dma_resv_lock(&resv, NULL); > + resv = dma_resv_alloc(); > + KUNIT_ASSERT_NOT_NULL(test, resv); [Severity: Medium] Does this assertion also leak 'f' in test_for_each_unlocked() if it fails? > + > + r = dma_resv_lock(resv, NULL); [ ... ] > @@ -248,24 +256,26 @@ static void test_get_fences(struct kunit *test) > > dma_fence_enable_signaling(f); > > - dma_resv_init(&resv); > - r = dma_resv_lock(&resv, NULL); > + resv = dma_resv_alloc(); > + KUNIT_ASSERT_NOT_NULL(test, resv); [Severity: Medium] Does this assertion also leak 'f' in test_get_fences() if it fails? > + > + r = dma_resv_lock(resv, NULL); -- Sashiko AI review · https://sashiko.dev/#/patchset/20260827124910.2245-1-christian.koenig@amd.com?part=2 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 03/10] drm/gem: Add helper for drm_gem_object resv assignment 2026-08-27 12:38 Refcounting dma_resv v2 Christian König 2026-08-27 12:38 ` [PATCH 01/10] dma-buf: Add reference counting to dma_resv Christian König 2026-08-27 12:38 ` [PATCH 02/10] dma-buf/tests: Convert st-dma-resv tests to use dma_resv_alloc Christian König @ 2026-08-27 12:38 ` Christian König 2026-08-27 13:22 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 04/10] drm/gem: Convert drm_gem_private_object_init to return error code Christian König ` (7 subsequent siblings) 10 siblings, 1 reply; 25+ messages in thread From: Christian König @ 2026-08-27 12:38 UTC (permalink / raw) To: thomas.hellstrom, dakr, ecourtney, matthew.brost, nat, dri-devel, intel-gfx, intel-xe, amd-gfx Introduce drm_gem_object_set_resv() to safely assign reservation objects to GEM objects with proper reference counting. This helper replaces all direct assignments to drm_gem_object.resv throughout the DRM subsystem. The function: - Acquires a reference to the new resv via dma_resv_get() - Assigns it to obj->resv - Releases the old resv reference via dma_resv_put() This ensures proper reference counting when sharing reservation objects between GEM objects or when importing dma-bufs, preventing reference leaks and use-after-free bugs. Update all drivers to use the new helper: - drm_prime: dma-buf import path - drm_gem_shmem_helper: shmem prime import - i915: dmabuf import - msm: MSM_BO_NO_SHARE case - panthor: exclusive_vm case - virtio: dma-buf import - xe: dummy object creation - ttm: external resv assignment - ttm/tests: test cases Signed-off-by: Christian König <christian.koenig@amd.com> Assisted-by: Claude:Sonnet 4 --- drivers/gpu/drm/drm_gem.c | 31 ++++++++++++++++++- drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- drivers/gpu/drm/drm_prime.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 2 +- drivers/gpu/drm/msm/msm_gem.c | 2 +- drivers/gpu/drm/panthor/panthor_gem.c | 2 +- drivers/gpu/drm/ttm/tests/ttm_bo_test.c | 2 +- drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 7 ++--- drivers/gpu/drm/virtio/virtgpu_prime.c | 2 +- drivers/gpu/drm/xe/xe_dma_buf.c | 2 +- include/drm/drm_gem.h | 2 ++ 12 files changed, 43 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 48176a11d552..bbcbd25f014f 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -230,7 +230,7 @@ void drm_gem_private_object_init(struct drm_device *dev, mutex_init(&obj->gpuva.lock); dma_resv_init(&obj->_resv); if (!obj->resv) - obj->resv = &obj->_resv; + obj->resv = dma_resv_get(&obj->_resv); drm_gem_gpuva_init(obj); @@ -249,11 +249,40 @@ void drm_gem_private_object_fini(struct drm_gem_object *obj) { WARN_ON(obj->dma_buf); + dma_resv_put(obj->resv); dma_resv_put(&obj->_resv); mutex_destroy(&obj->gpuva.lock); } EXPORT_SYMBOL(drm_gem_private_object_fini); +/** + * drm_gem_object_set_resv - Set the reservation object for a GEM object + * @obj: GEM object + * @resv: reservation object to assign + * + * This function safely assigns a new reservation object to a GEM object. + * It releases the old reservation object reference (if any) and acquires + * a reference to the new one. + * + * This should be used when changing the reservation object of an already + * initialized GEM object, for example when importing a dma-buf or sharing + * a reservation object with another object. + * + * Returns: + * The new reservation object pointer for convenience. + */ +struct dma_resv *drm_gem_object_set_resv(struct drm_gem_object *obj, + struct dma_resv *resv) +{ + struct dma_resv *old_resv = obj->resv; + + obj->resv = dma_resv_get(resv); + dma_resv_put(old_resv); + + return obj->resv; +} +EXPORT_SYMBOL(drm_gem_object_set_resv); + static void drm_gem_object_handle_get(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 22ec52e2ffb8..76a8d7f252e0 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -977,7 +977,7 @@ struct drm_gem_object *drm_gem_shmem_prime_import_no_map(struct drm_device *dev, drm_dbg_prime(dev, "size = %zu\n", size); shmem->base.import_attach = attach; - shmem->base.resv = dma_buf->resv; + drm_gem_object_set_resv(&shmem->base, dma_buf->resv); return &shmem->base; diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 9b44c78cd77f..6cfeca347faf 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -1006,7 +1006,7 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev, } obj->import_attach = attach; - obj->resv = dma_buf->resv; + drm_gem_object_set_resv(obj, dma_buf->resv); return obj; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index b43d34c7d641..c824606497d2 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -320,7 +320,7 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev, i915_gem_object_init(obj, &i915_gem_object_dmabuf_ops, &lock_class, I915_BO_ALLOC_USER); obj->base.import_attach = attach; - obj->base.resv = dma_buf->resv; + drm_gem_object_set_resv(&obj->base, dma_buf->resv); /* We use GTT as shorthand for a coherent domain, one that is * neither in the GPU cache nor in the CPU cache, where all diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index efd3d3c9a449..81ebb1e1094c 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -1151,7 +1151,7 @@ int msm_gem_new_handle(struct drm_device *dev, struct drm_file *file, drm_gem_object_get(r_obj); - obj->resv = r_obj->resv; + drm_gem_object_set_resv(obj, r_obj->resv); } ret = drm_gem_handle_create(file, obj, handle); diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c index 9855df738194..2de24f1b6018 100644 --- a/drivers/gpu/drm/panthor/panthor_gem.c +++ b/drivers/gpu/drm/panthor/panthor_gem.c @@ -1024,7 +1024,7 @@ panthor_gem_create(struct drm_device *dev, size_t size, uint32_t flags, if (exclusive_vm) { bo->exclusive_vm_root_gem = panthor_vm_root_gem(exclusive_vm); drm_gem_object_get(bo->exclusive_vm_root_gem); - bo->base.resv = bo->exclusive_vm_root_gem->resv; + drm_gem_object_set_resv(&bo->base, bo->exclusive_vm_root_gem->resv); } panthor_gem_debugfs_set_usage_flags(bo, usage_flags); diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c index 49b0b48c6c2a..3aff1c427cd1 100644 --- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c @@ -461,7 +461,7 @@ static void ttm_bo_fini_shared_resv(struct kunit *test) bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL); bo->type = ttm_bo_type_device; - bo->base.resv = external_resv; + drm_gem_object_set_resv(&bo->base, external_resv); ttm_bo_fini(bo); } diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c index 5cfe8f3f80d7..1146cc9ae522 100644 --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c @@ -181,7 +181,7 @@ struct ttm_buffer_object *ttm_bo_kunit_init(struct kunit *test, bo->base = gem_obj; if (obj) - bo->base.resv = obj; + drm_gem_object_set_resv(&bo->base, obj); err = drm_gem_object_init(devs->drm, &bo->base, size); KUNIT_ASSERT_EQ(test, err, 0); diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 1b9477381a1d..999c24251fb7 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -209,7 +209,7 @@ static int ttm_bo_individualize_resv(struct ttm_buffer_object *bo) * the resv object while holding the lru_lock. */ spin_lock(&bo->bdev->lru_lock); - bo->base.resv = &bo->base._resv; + drm_gem_object_set_resv(&bo->base, &bo->base._resv); spin_unlock(&bo->bdev->lru_lock); } @@ -1197,10 +1197,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, bo->pin_count = 0; bo->sg = sg; bo->bulk_move = NULL; - if (resv) - bo->base.resv = resv; - else - bo->base.resv = &bo->base._resv; + drm_gem_object_set_resv(&bo->base, resv ?: &bo->base._resv); atomic_inc(&ttm_glob.bo_count); /* diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c index 216c77cd0d21..79964b436243 100644 --- a/drivers/gpu/drm/virtio/virtgpu_prime.c +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c @@ -361,7 +361,7 @@ struct drm_gem_object *virtgpu_gem_prime_import(struct drm_device *dev, INIT_LIST_HEAD(&bo->restore_node); obj = &bo->base.base; - obj->resv = buf->resv; + drm_gem_object_set_resv(obj, buf->resv); obj->funcs = &virtgpu_gem_dma_buf_funcs; drm_gem_private_object_init(dev, obj, buf->size); diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c b/drivers/gpu/drm/xe/xe_dma_buf.c index 8a920e58245c..33bebf85ac62 100644 --- a/drivers/gpu/drm/xe/xe_dma_buf.c +++ b/drivers/gpu/drm/xe/xe_dma_buf.c @@ -293,7 +293,7 @@ xe_dma_buf_create_obj(struct drm_device *dev, struct dma_buf *dma_buf) if (!dummy_obj) return ERR_PTR(-ENOMEM); - dummy_obj->resv = resv; + drm_gem_object_set_resv(dummy_obj, resv); xe_validation_guard(&ctx, &xe->val, &exec, (struct xe_val_flags) {}, ret) { ret = drm_exec_lock_obj(&exec, dummy_obj); drm_exec_retry_on_contention(&exec); diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 885244e375d3..09f5ec28218a 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -529,6 +529,8 @@ int drm_gem_object_init(struct drm_device *dev, void drm_gem_private_object_init(struct drm_device *dev, struct drm_gem_object *obj, size_t size); void drm_gem_private_object_fini(struct drm_gem_object *obj); +struct dma_resv *drm_gem_object_set_resv(struct drm_gem_object *obj, + struct dma_resv *resv); void drm_gem_vm_open(struct vm_area_struct *vma); void drm_gem_vm_close(struct vm_area_struct *vma); int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, -- 2.43.0 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 03/10] drm/gem: Add helper for drm_gem_object resv assignment 2026-08-27 12:38 ` [PATCH 03/10] drm/gem: Add helper for drm_gem_object resv assignment Christian König @ 2026-08-27 13:22 ` sashiko-bot 0 siblings, 0 replies; 25+ messages in thread From: sashiko-bot @ 2026-08-27 13:22 UTC (permalink / raw) To: Christian König; +Cc: intel-gfx, dri-devel, intel-xe Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [Critical] Direct assignments to `obj->resv` bypass `drm_gem_object_set_resv()`, causing reference counting underflows and memory leaks. - [High] i915 driver open-coded GEM teardown misses `dma_resv_put(obj->base.resv)`, permanently leaking the reservation object references for all i915 GEM objects. - [High] Memory leaks on the error path in virtgpu_gem_prime_import(). - [High] Calling drm_gem_object_set_resv() under a spinlock can trigger dma_resv and fence release callbacks, causing potential deadlocks. -- commit ad191ee55ce616a6f881f13681e85331c2eeb4ec Author: Christian König <ckoenig.leichtzumerken@gmail.com> drm/gem: Add helper for drm_gem_object resv assignment Introduce drm_gem_object_set_resv() to safely assign reservation objects to GEM objects with proper reference counting. This helper replaces all direct assignments to drm_gem_object.resv throughout the DRM subsystem. [ ... ] > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index 88d60fb17f43b..bc87cc8fbeffa 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -230,7 +230,7 @@ void drm_gem_private_object_init(struct drm_device *dev, > mutex_init(&obj->gpuva.lock); > dma_resv_init(&obj->_resv); > if (!obj->resv) > - obj->resv = &obj->_resv; > + obj->resv = dma_resv_get(&obj->_resv); [Severity: Critical] With this change, obj->resv holds a reference that must be released. It looks like some drivers still use direct assignment (e.g., obj->resv = dma_buf->resv) which will leak this initial reference and fail to acquire a reference on the new object. For example, these assignments appear to remain: - drivers/accel/amdxdna/amdxdna_gem.c: gobj->resv = dma_buf->resv; - drivers/accel/ivpu/ivpu_gem.c: obj->resv = dma_buf->resv; - drivers/accel/qaic/qaic_data.c: obj->resv = dma_buf->resv; - drivers/gpu/drm/i915/gt/intel_gtt.c: obj->base.resv = i915_vm_resv_get(vm); - rust/kernel/drm/gem/shmem.rs: unsafe { (*obj.obj.get()).base.resv = parent_resv.raw_dma_resv() }; When these GEM objects are destroyed, drm_gem_private_object_fini() will call dma_resv_put() on the external reservation object. Since a reference was never acquired during assignment, could this cause a use-after-free? Should these callers also be updated to use drm_gem_object_set_resv()? > > drm_gem_gpuva_init(obj); > > @@ -249,11 +249,40 @@ void drm_gem_private_object_fini(struct drm_gem_object *obj) > { > WARN_ON(obj->dma_buf); > > + dma_resv_put(obj->resv); > dma_resv_put(&obj->_resv); > mutex_destroy(&obj->gpuva.lock); > } [Severity: High] This correctly releases the reference for objects using the standard teardown path. However, the i915 driver open-codes GEM teardown and doesn't call drm_gem_private_object_fini(): drivers/gpu/drm/i915/gem/i915_gem_object.c:__i915_gem_object_fini() { mutex_destroy(&obj->mm.get_page.lock); mutex_destroy(&obj->mm.get_dma_page.lock); dma_resv_put(&obj->base._resv); } Does __i915_gem_object_fini() also need a dma_resv_put(obj->base.resv) call to prevent permanently leaking the reservation object references for i915 GEM objects? [ ... ] > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index ef56c18ded1b8..216c83bb0a0bb 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -209,7 +209,7 @@ static int ttm_bo_individualize_resv(struct ttm_buffer_object *bo) > * the resv object while holding the lru_lock. > */ > spin_lock(&bo->bdev->lru_lock); > - bo->base.resv = &bo->base._resv; > + drm_gem_object_set_resv(&bo->base, &bo->base._resv); > spin_unlock(&bo->bdev->lru_lock); > } [Severity: High] Calling drm_gem_object_set_resv() here drops the reference to the old reservation object while holding bo->bdev->lru_lock. If this drops the last reference to a shared reservation object, dma_resv_release() will be invoked synchronously under the spinlock. Since dma_resv_release() frees the fence list and drops all attached fences, could this invoke fence release callbacks that attempt to acquire the same lru_lock (e.g., via ttm_bo_release()), leading to a deadlock? [ ... ] > diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c > index 216c77cd0d21b..79964b4362439 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_prime.c > +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c > @@ -361,7 +361,7 @@ struct drm_gem_object *virtgpu_gem_prime_import(struct drm_device *dev, > INIT_LIST_HEAD(&bo->restore_node); > > obj = &bo->base.base; > - obj->resv = buf->resv; > + drm_gem_object_set_resv(obj, buf->resv); > obj->funcs = &virtgpu_gem_dma_buf_funcs; > drm_gem_private_object_init(dev, obj, buf->size); > [Severity: High] If the subsequent calls to drm_gem_private_object_init() or dma_buf_dynamic_attach() fail, the error paths just call kfree(bo): ret = drm_gem_private_object_init(dev, obj, buf->size); if (ret) { kfree(bo); return ERR_PTR(ret); } attach = dma_buf_dynamic_attach(buf, dev->dev, &virtgpu_dma_buf_attach_ops, obj); if (IS_ERR(attach)) { kfree(bo); return ERR_CAST(attach); } Does this leak the new reference acquired by drm_gem_object_set_resv() to buf->resv, as well as the resources initialized by drm_gem_private_object_init() in the latter case? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260827124910.2245-1-christian.koenig@amd.com?part=3 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 04/10] drm/gem: Convert drm_gem_private_object_init to return error code 2026-08-27 12:38 Refcounting dma_resv v2 Christian König ` (2 preceding siblings ...) 2026-08-27 12:38 ` [PATCH 03/10] drm/gem: Add helper for drm_gem_object resv assignment Christian König @ 2026-08-27 12:38 ` Christian König 2026-08-27 13:19 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 05/10] drm/gem: Use dynamic allocation for GEM object dma_resv Christian König ` (6 subsequent siblings) 10 siblings, 1 reply; 25+ messages in thread From: Christian König @ 2026-08-27 12:38 UTC (permalink / raw) To: thomas.hellstrom, dakr, ecourtney, matthew.brost, nat, dri-devel, intel-gfx, intel-xe, amd-gfx Change drm_gem_private_object_init() to return an int instead of void, allowing it to report errors instead of crashing the kernel with BUG_ON. The function now returns -EINVAL if the size parameter is not page-aligned, rather than using BUG_ON to crash the system. This change improves error handling throughout the DRM subsystem by replacing a kernel panic with graceful error propagation. All 60+ call sites across the DRM and accelerator drivers have been updated to check the return value and handle errors appropriately. Updated components: - Core: drm_gem.c, drm_gem_shmem_helper.c, drm_gem_dma_helper.c, drm_gpuvm.c - AMD: amdgpu, radeon - Intel: i915 (multiple modules), xe, gvt - ARM/Mobile: armada, etnaviv, gma500, msm, omapdrm, panthor, imagination - Virtual: virtio, vmwgfx - Other: renesas/rcar-du - Accelerators: qaic, amdxdna - Tests: drm_exec_test, ttm_bo_validate_test, i915 selftests Each caller now properly checks the return value and either propagates the error up the call stack or handles it with appropriate cleanup. Signed-off-by: Christian König <christian.koenig@amd.com> Assisted-by: Claude:Sonnet 4 --- drivers/accel/amdxdna/amdxdna_gem.c | 6 ++- drivers/accel/qaic/qaic_data.c | 10 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 ++- drivers/gpu/drm/armada/armada_gem.c | 7 ++- drivers/gpu/drm/drm_gem.c | 17 ++++-- drivers/gpu/drm/drm_gem_dma_helper.c | 2 +- drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- drivers/gpu/drm/drm_gpuvm.c | 7 ++- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 6 ++- drivers/gpu/drm/gma500/gem.c | 4 +- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 7 ++- drivers/gpu/drm/i915/gem/i915_gem_internal.c | 8 ++- drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 5 +- drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 5 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 5 +- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 7 ++- .../drm/i915/gem/selftests/huge_gem_object.c | 8 ++- .../gpu/drm/i915/gem/selftests/huge_pages.c | 15 +++++- drivers/gpu/drm/i915/gvt/dmabuf.c | 8 ++- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 6 ++- drivers/gpu/drm/i915/selftests/mock_region.c | 6 ++- drivers/gpu/drm/imagination/pvr_vm.c | 8 ++- drivers/gpu/drm/msm/msm_gem.c | 6 ++- drivers/gpu/drm/omapdrm/omap_gem.c | 4 +- drivers/gpu/drm/panthor/panthor_gem.c | 6 ++- drivers/gpu/drm/radeon/radeon_object.c | 6 ++- drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 6 ++- drivers/gpu/drm/tests/drm_exec_test.c | 18 ++++--- .../gpu/drm/ttm/tests/ttm_bo_validate_test.c | 54 ++++++++++++------- drivers/gpu/drm/virtio/virtgpu_prime.c | 6 ++- drivers/gpu/drm/virtio/virtgpu_vram.c | 6 ++- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 +- drivers/gpu/drm/xe/xe_bo.c | 4 +- drivers/gpu/drm/xe/xe_mmio_gem.c | 6 ++- include/drm/drm_gem.h | 4 +- 35 files changed, 222 insertions(+), 63 deletions(-) diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c index 1f190b319bb6..f4c017f496fa 100644 --- a/drivers/accel/amdxdna/amdxdna_gem.c +++ b/drivers/accel/amdxdna/amdxdna_gem.c @@ -1023,7 +1023,11 @@ amdxdna_drm_create_dev_bo(struct drm_device *dev, return ERR_PTR(ret); } - drm_gem_private_object_init(dev, gobj, aligned_sz); + ret = drm_gem_private_object_init(dev, gobj, aligned_sz); + if (ret) { + amdxdna_gem_destroy_obj(abo); + return ERR_PTR(ret); + } return abo; } diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c index 4d55531bf1c9..dcd21faaef4a 100644 --- a/drivers/accel/qaic/qaic_data.c +++ b/drivers/accel/qaic/qaic_data.c @@ -770,7 +770,11 @@ int qaic_create_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *fi } obj = &bo->base; - drm_gem_private_object_init(dev, obj, size); + ret = drm_gem_private_object_init(dev, obj, size); + if (ret) { + kfree(bo); + goto unlock_dev_srcu; + } obj->funcs = &qaic_gem_funcs; ret = create_sgt(qdev, &bo->sgt, size); @@ -867,7 +871,9 @@ struct drm_gem_object *qaic_gem_prime_import(struct drm_device *dev, struct dma_ goto size_align_fail; } - drm_gem_private_object_init(dev, obj, attach->dmabuf->size); + ret = drm_gem_private_object_init(dev, obj, attach->dmabuf->size); + if (ret) + goto size_align_fail; /* * skipping dma_buf_map_attachment() as we do not know the direction * just yet. Once the direction is known in the subsequent IOCTL to diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index f98bfba59a2c..6c5182d54f7d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -663,7 +663,11 @@ int amdgpu_bo_create(struct amdgpu_device *adev, bo = kvzalloc(bp->bo_ptr_size, GFP_KERNEL); if (bo == NULL) return -ENOMEM; - drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size); + r = drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size); + if (r) { + kvfree(bo); + return r; + } bo->tbo.base.funcs = &amdgpu_gem_object_funcs; bo->vm_bo = NULL; bo->preferred_domains = bp->preferred_domain ? bp->preferred_domain : diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c index da7335cbe82d..da6f5506743e 100644 --- a/drivers/gpu/drm/armada/armada_gem.c +++ b/drivers/gpu/drm/armada/armada_gem.c @@ -197,6 +197,7 @@ struct armada_gem_object * armada_gem_alloc_private_object(struct drm_device *dev, size_t size) { struct armada_gem_object *obj; + int ret; size = roundup_gem_size(size); @@ -206,7 +207,11 @@ armada_gem_alloc_private_object(struct drm_device *dev, size_t size) obj->obj.funcs = &armada_gem_object_funcs; - drm_gem_private_object_init(dev, &obj->obj, size); + ret = drm_gem_private_object_init(dev, &obj->obj, size); + if (ret) { + kfree(obj); + return NULL; + } DRM_DEBUG_DRIVER("alloc private obj %p size %zu\n", obj, size); diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index bbcbd25f014f..d2ce18bbddea 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -187,8 +187,11 @@ int drm_gem_object_init(struct drm_device *dev, struct drm_gem_object *obj, struct vfsmount *huge_mnt; struct file *filp; const vma_flags_t flags = mk_vma_flags(VMA_NORESERVE_BIT); + int ret; - drm_gem_private_object_init(dev, obj, size); + ret = drm_gem_private_object_init(dev, obj, size); + if (ret) + return ret; huge_mnt = drm_gem_get_huge_mnt(dev); if (huge_mnt) @@ -215,11 +218,15 @@ EXPORT_SYMBOL(drm_gem_object_init); * Initialize an already allocated GEM object of the specified size with * no GEM provided backing store. Instead the caller is responsible for * backing the object and handling it. + * + * Returns: + * 0 on success, or a negative error code on failure. */ -void drm_gem_private_object_init(struct drm_device *dev, - struct drm_gem_object *obj, size_t size) +int drm_gem_private_object_init(struct drm_device *dev, + struct drm_gem_object *obj, size_t size) { - BUG_ON((size & (PAGE_SIZE - 1)) != 0); + if ((size & (PAGE_SIZE - 1)) != 0) + return -EINVAL; obj->dev = dev; obj->filp = NULL; @@ -236,6 +243,8 @@ void drm_gem_private_object_init(struct drm_device *dev, drm_vma_node_reset(&obj->vma_node); INIT_LIST_HEAD(&obj->lru_node); + + return 0; } EXPORT_SYMBOL(drm_gem_private_object_init); diff --git a/drivers/gpu/drm/drm_gem_dma_helper.c b/drivers/gpu/drm/drm_gem_dma_helper.c index 1c00a71ab3c9..fa354a36640b 100644 --- a/drivers/gpu/drm/drm_gem_dma_helper.c +++ b/drivers/gpu/drm/drm_gem_dma_helper.c @@ -92,7 +92,7 @@ __drm_gem_dma_create(struct drm_device *drm, size_t size, bool private) gem_obj->funcs = &drm_gem_dma_default_funcs; if (private) { - drm_gem_private_object_init(drm, gem_obj, size); + ret = drm_gem_private_object_init(drm, gem_obj, size); /* Always use writecombine for dma-buf mappings */ dma_obj->map_noncoherent = false; diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 76a8d7f252e0..9f910b0ed0b0 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -61,7 +61,7 @@ static int __drm_gem_shmem_init(struct drm_device *dev, struct drm_gem_shmem_obj obj->funcs = &drm_gem_shmem_funcs; if (private) { - drm_gem_private_object_init(dev, obj, size); + ret = drm_gem_private_object_init(dev, obj, size); shmem->map_wc = false; /* dma-buf mappings use always writecombine */ } else { ret = drm_gem_object_init(dev, obj, size); diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c index d1c80ad3dead..ac1be020c6df 100644 --- a/drivers/gpu/drm/drm_gpuvm.c +++ b/drivers/gpu/drm/drm_gpuvm.c @@ -1060,13 +1060,18 @@ struct drm_gem_object * drm_gpuvm_resv_object_alloc(struct drm_device *drm) { struct drm_gem_object *obj; + int ret; obj = kzalloc_obj(*obj); if (!obj) return NULL; obj->funcs = &drm_gpuvm_object_funcs; - drm_gem_private_object_init(drm, obj, 0); + ret = drm_gem_private_object_init(drm, obj, 0); + if (ret) { + kfree(obj); + return NULL; + } return obj; } diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c index b0436a1e103f..94fa4d49f3fc 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c @@ -656,7 +656,11 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags, if (ret) return ret; - drm_gem_private_object_init(dev, obj, size); + ret = drm_gem_private_object_init(dev, obj, size); + if (ret) { + kfree(to_etnaviv_bo(obj)); + return ret; + } *res = to_etnaviv_bo(obj); diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c index 88f1e86c8903..11d0bb01f005 100644 --- a/drivers/gpu/drm/gma500/gem.c +++ b/drivers/gpu/drm/gma500/gem.c @@ -168,7 +168,9 @@ psb_gem_create(struct drm_device *dev, u64 size, const char *name, bool stolen, obj->funcs = &psb_gem_object_funcs; if (stolen) { - drm_gem_private_object_init(dev, obj, size); + ret = drm_gem_private_object_init(dev, obj, size); + if (ret) + goto err_release_resource; } else { ret = drm_gem_object_init(dev, obj, size); if (ret) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index c824606497d2..65ce865a80e2 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -316,7 +316,10 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev, goto fail_detach; } - drm_gem_private_object_init(dev, &obj->base, dma_buf->size); + ret = drm_gem_private_object_init(dev, &obj->base, dma_buf->size); + if (ret) + goto fail_free_obj; + i915_gem_object_init(obj, &i915_gem_object_dmabuf_ops, &lock_class, I915_BO_ALLOC_USER); obj->base.import_attach = attach; @@ -334,6 +337,8 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev, return &obj->base; +fail_free_obj: + i915_gem_object_free(obj); fail_detach: dma_buf_detach(dma_buf, attach); dma_buf_put(dma_buf); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_internal.c b/drivers/gpu/drm/i915/gem/i915_gem_internal.c index 37d286ecb99b..191d63aaf0a9 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_internal.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_internal.c @@ -142,6 +142,7 @@ __i915_gem_object_create_internal(struct drm_i915_private *i915, static struct lock_class_key lock_class; struct drm_i915_gem_object *obj; unsigned int cache_level; + int ret; GEM_BUG_ON(!size); GEM_BUG_ON(!IS_ALIGNED(size, PAGE_SIZE)); @@ -153,7 +154,12 @@ __i915_gem_object_create_internal(struct drm_i915_private *i915, if (!obj) return ERR_PTR(-ENOMEM); - drm_gem_private_object_init(&i915->drm, &obj->base, size); + ret = drm_gem_private_object_init(&i915->drm, &obj->base, size); + if (ret) { + i915_gem_object_free(obj); + return ERR_PTR(ret); + } + i915_gem_object_init(obj, ops, &lock_class, 0); obj->mem_flags |= I915_BO_FLAG_STRUCT_PAGE; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index 06543ae60706..d1a6d72c57ec 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -508,8 +508,11 @@ static int __create_shmem(struct drm_i915_private *i915, const vma_flags_t shmem_flags = mk_vma_flags(VMA_NORESERVE_BIT); struct vfsmount *huge_mnt; struct file *filp; + int ret; - drm_gem_private_object_init(&i915->drm, obj, size); + ret = drm_gem_private_object_init(&i915->drm, obj, size); + if (ret) + return ret; /* XXX: The __shmem_file_setup() function returns -EINVAL if size is * greater than MAX_LFS_FILESIZE. diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c index 1cfdcf5c1118..e39e82ec9237 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c @@ -737,7 +737,10 @@ static int __i915_gem_object_create_stolen(struct intel_memory_region *mem, */ flags = I915_BO_ALLOC_CONTIGUOUS; - drm_gem_private_object_init(&mem->i915->drm, &obj->base, stolen->size); + err = drm_gem_private_object_init(&mem->i915->drm, &obj->base, stolen->size); + if (err) + return err; + i915_gem_object_init(obj, &i915_gem_object_stolen_ops, &lock_class, flags); obj->stolen = stolen; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index df3fcc2b1248..ae7394be54a0 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -1301,7 +1301,10 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem, enum ttm_bo_type bo_type; int ret; - drm_gem_private_object_init(&i915->drm, &obj->base, size); + ret = drm_gem_private_object_init(&i915->drm, &obj->base, size); + if (ret) + return ret; + i915_gem_object_init(obj, &i915_gem_ttm_obj_ops, &lock_class, flags); obj->bo_offset = offset; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c index 043095f93ac6..1a2f9a058d55 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c @@ -521,7 +521,12 @@ i915_gem_userptr_ioctl(struct drm_device *dev, if (obj == NULL) return -ENOMEM; - drm_gem_private_object_init(dev, &obj->base, args->user_size); + ret = drm_gem_private_object_init(dev, &obj->base, args->user_size); + if (ret) { + i915_gem_object_free(obj); + return ret; + } + i915_gem_object_init(obj, &i915_gem_userptr_ops, &lock_class, I915_BO_ALLOC_USER); obj->mem_flags = I915_BO_FLAG_STRUCT_PAGE; diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_gem_object.c b/drivers/gpu/drm/i915/gem/selftests/huge_gem_object.c index 8caaf325e56a..fdc0186e8045 100644 --- a/drivers/gpu/drm/i915/gem/selftests/huge_gem_object.c +++ b/drivers/gpu/drm/i915/gem/selftests/huge_gem_object.c @@ -105,6 +105,7 @@ huge_gem_object(struct drm_i915_private *i915, static struct lock_class_key lock_class; struct drm_i915_gem_object *obj; unsigned int cache_level; + int ret; GEM_BUG_ON(!phys_size || phys_size > dma_size); GEM_BUG_ON(!IS_ALIGNED(phys_size, PAGE_SIZE)); @@ -117,7 +118,12 @@ huge_gem_object(struct drm_i915_private *i915, if (!obj) return ERR_PTR(-ENOMEM); - drm_gem_private_object_init(&i915->drm, &obj->base, dma_size); + ret = drm_gem_private_object_init(&i915->drm, &obj->base, dma_size); + if (ret) { + i915_gem_object_free(obj); + return ERR_PTR(ret); + } + i915_gem_object_init(obj, &huge_ops, &lock_class, 0); obj->mem_flags |= I915_BO_FLAG_STRUCT_PAGE; diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c index 44718e728291..a76f38ae9335 100644 --- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c +++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c @@ -178,6 +178,7 @@ huge_pages_object(struct drm_i915_private *i915, static struct lock_class_key lock_class; struct drm_i915_gem_object *obj; unsigned int cache_level; + int ret; GEM_BUG_ON(!size); GEM_BUG_ON(!IS_ALIGNED(size, BIT(__ffs(page_mask)))); @@ -192,7 +193,12 @@ huge_pages_object(struct drm_i915_private *i915, if (!obj) return ERR_PTR(-ENOMEM); - drm_gem_private_object_init(&i915->drm, &obj->base, size); + ret = drm_gem_private_object_init(&i915->drm, &obj->base, size); + if (ret) { + i915_gem_object_free(obj); + return ERR_PTR(ret); + } + i915_gem_object_init(obj, &huge_page_ops, &lock_class, 0); obj->mem_flags |= I915_BO_FLAG_STRUCT_PAGE; i915_gem_object_set_volatile(obj); @@ -329,6 +335,7 @@ fake_huge_pages_object(struct drm_i915_private *i915, u64 size, bool single) { static struct lock_class_key lock_class; struct drm_i915_gem_object *obj; + int ret; GEM_BUG_ON(!size); GEM_BUG_ON(!IS_ALIGNED(size, I915_GTT_PAGE_SIZE)); @@ -343,7 +350,11 @@ fake_huge_pages_object(struct drm_i915_private *i915, u64 size, bool single) if (!obj) return ERR_PTR(-ENOMEM); - drm_gem_private_object_init(&i915->drm, &obj->base, size); + ret = drm_gem_private_object_init(&i915->drm, &obj->base, size); + if (ret) { + i915_gem_object_free(obj); + return ERR_PTR(ret); + } if (single) i915_gem_object_init(obj, &fake_ops_single, &lock_class, 0); diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c b/drivers/gpu/drm/i915/gvt/dmabuf.c index 8a1d6c5636c2..5b198162c588 100644 --- a/drivers/gpu/drm/i915/gvt/dmabuf.c +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c @@ -202,13 +202,19 @@ static struct drm_i915_gem_object *vgpu_create_gem(struct drm_device *dev, static struct lock_class_key lock_class; struct drm_i915_private *dev_priv = to_i915(dev); struct drm_i915_gem_object *obj; + int ret; obj = i915_gem_object_alloc(); if (obj == NULL) return NULL; - drm_gem_private_object_init(dev, &obj->base, + ret = drm_gem_private_object_init(dev, &obj->base, roundup(info->size, PAGE_SIZE)); + if (ret) { + i915_gem_object_free(obj); + return NULL; + } + i915_gem_object_init(obj, &intel_vgpu_gem_ops, &lock_class, 0); i915_gem_object_set_readonly(obj); diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c index e6bd06774816..7eb254a0be57 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c @@ -128,7 +128,11 @@ fake_dma_object(struct drm_i915_private *i915, u64 size) if (!obj) goto err; - drm_gem_private_object_init(&i915->drm, &obj->base, size); + if (drm_gem_private_object_init(&i915->drm, &obj->base, size)) { + i915_gem_object_free(obj); + goto err; + } + i915_gem_object_init(obj, &fake_ops, &lock_class, 0); i915_gem_object_set_volatile(obj); diff --git a/drivers/gpu/drm/i915/selftests/mock_region.c b/drivers/gpu/drm/i915/selftests/mock_region.c index 6324eb32d4dd..8bee0df1def9 100644 --- a/drivers/gpu/drm/i915/selftests/mock_region.c +++ b/drivers/gpu/drm/i915/selftests/mock_region.c @@ -66,11 +66,15 @@ static int mock_object_init(struct intel_memory_region *mem, { static struct lock_class_key lock_class; struct drm_i915_private *i915 = mem->i915; + int ret; if (size > resource_size(&mem->region)) return -E2BIG; - drm_gem_private_object_init(&i915->drm, &obj->base, size); + ret = drm_gem_private_object_init(&i915->drm, &obj->base, size); + if (ret) + return ret; + i915_gem_object_init(obj, &mock_region_obj_ops, &lock_class, flags); obj->bo_offset = offset; diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c index 396d349fb6ce..a3bc64e56728 100644 --- a/drivers/gpu/drm/imagination/pvr_vm.c +++ b/drivers/gpu/drm/imagination/pvr_vm.c @@ -585,7 +585,10 @@ pvr_vm_create_context(struct pvr_device *pvr_dev, bool is_userspace_context) goto err_page_table_destroy; } - drm_gem_private_object_init(&pvr_dev->base, &vm_ctx->dummy_gem, 0); + err = drm_gem_private_object_init(&pvr_dev->base, &vm_ctx->dummy_gem, 0); + if (err) + goto err_fw_object_destroy; + drm_gpuvm_init(&vm_ctx->gpuvm_mgr, is_userspace_context ? "PowerVR-user-VM" : "PowerVR-FW-VM", 0, &pvr_dev->base, &vm_ctx->dummy_gem, @@ -596,6 +599,9 @@ pvr_vm_create_context(struct pvr_device *pvr_dev, bool is_userspace_context) return vm_ctx; +err_fw_object_destroy: + if (is_userspace_context) + pvr_fw_object_destroy(vm_ctx->fw_mem_ctx_obj); err_page_table_destroy: pvr_mmu_context_destroy(vm_ctx->mmu_ctx); diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 81ebb1e1094c..c3e95194b24f 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -1296,7 +1296,11 @@ struct drm_gem_object *msm_gem_import(struct drm_device *dev, if (ret) return ERR_PTR(ret); - drm_gem_private_object_init(dev, obj, size); + ret = drm_gem_private_object_init(dev, obj, size); + if (ret) { + kfree(to_msm_bo(obj)); + return ERR_PTR(ret); + } npages = size / PAGE_SIZE; diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c index 8e013e4f2c6b..364d29686c46 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem.c +++ b/drivers/gpu/drm/omapdrm/omap_gem.c @@ -1348,7 +1348,9 @@ struct drm_gem_object *omap_gem_new(struct drm_device *dev, /* Initialize the GEM object. */ if (!(flags & OMAP_BO_MEM_SHMEM)) { - drm_gem_private_object_init(dev, obj, size); + ret = drm_gem_private_object_init(dev, obj, size); + if (ret) + goto err_free; } else { ret = drm_gem_object_init(dev, obj, size); if (ret) diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c index 2de24f1b6018..cb3935ace1e6 100644 --- a/drivers/gpu/drm/panthor/panthor_gem.c +++ b/drivers/gpu/drm/panthor/panthor_gem.c @@ -1047,7 +1047,11 @@ panthor_gem_prime_import_sg_table(struct drm_device *dev, if (IS_ERR(bo)) return ERR_CAST(bo); - drm_gem_private_object_init(dev, &bo->base, attach->dmabuf->size); + ret = drm_gem_private_object_init(dev, &bo->base, attach->dmabuf->size); + if (ret) { + kfree(bo); + return ERR_PTR(ret); + } ret = drm_gem_create_mmap_offset(&bo->base); if (ret) diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index a0fc0801abb0..c7557c39ead0 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -150,7 +150,11 @@ int radeon_bo_create(struct radeon_device *rdev, bo = kzalloc(sizeof(struct radeon_bo), GFP_KERNEL); if (bo == NULL) return -ENOMEM; - drm_gem_private_object_init(rdev_to_drm(rdev), &bo->tbo.base, size); + r = drm_gem_private_object_init(rdev_to_drm(rdev), &bo->tbo.base, size); + if (r) { + kfree(bo); + return r; + } bo->tbo.base.funcs = &radeon_gem_object_funcs; bo->rdev = rdev; bo->surface_reg = -1; diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c index 9228a7ad0dfa..977ceb4462bd 100644 --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c @@ -389,7 +389,11 @@ struct drm_gem_object *rcar_du_gem_prime_import_sg_table(struct drm_device *dev, gem_obj = &dma_obj->base; gem_obj->funcs = &rcar_du_gem_funcs; - drm_gem_private_object_init(dev, gem_obj, attach->dmabuf->size); + ret = drm_gem_private_object_init(dev, gem_obj, attach->dmabuf->size); + if (ret) { + kfree(dma_obj); + return ERR_PTR(ret); + } dma_obj->map_noncoherent = false; ret = drm_gem_create_mmap_offset(gem_obj); diff --git a/drivers/gpu/drm/tests/drm_exec_test.c b/drivers/gpu/drm/tests/drm_exec_test.c index 2fc47f3b463b..b4427484da26 100644 --- a/drivers/gpu/drm/tests/drm_exec_test.c +++ b/drivers/gpu/drm/tests/drm_exec_test.c @@ -56,7 +56,8 @@ static void test_lock(struct kunit *test) struct drm_exec exec; int ret; - drm_gem_private_object_init(priv->drm, &gobj, PAGE_SIZE); + ret = drm_gem_private_object_init(priv->drm, &gobj, PAGE_SIZE); + KUNIT_ASSERT_EQ(test, ret, 0); drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0); drm_exec_until_all_locked(&exec) { @@ -76,7 +77,8 @@ static void test_lock_unlock(struct kunit *test) struct drm_exec exec; int ret; - drm_gem_private_object_init(priv->drm, &gobj, PAGE_SIZE); + ret = drm_gem_private_object_init(priv->drm, &gobj, PAGE_SIZE); + KUNIT_ASSERT_EQ(test, ret, 0); drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0); drm_exec_until_all_locked(&exec) { @@ -103,7 +105,8 @@ static void test_duplicates(struct kunit *test) struct drm_exec exec; int ret; - drm_gem_private_object_init(priv->drm, &gobj, PAGE_SIZE); + ret = drm_gem_private_object_init(priv->drm, &gobj, PAGE_SIZE); + KUNIT_ASSERT_EQ(test, ret, 0); drm_exec_init(&exec, DRM_EXEC_IGNORE_DUPLICATES, 0); drm_exec_until_all_locked(&exec) { @@ -130,7 +133,8 @@ static void test_prepare(struct kunit *test) struct drm_exec exec; int ret; - drm_gem_private_object_init(priv->drm, &gobj, PAGE_SIZE); + ret = drm_gem_private_object_init(priv->drm, &gobj, PAGE_SIZE); + KUNIT_ASSERT_EQ(test, ret, 0); drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0); drm_exec_until_all_locked(&exec) { @@ -162,8 +166,10 @@ static void test_prepare_array(struct kunit *test) return; } - drm_gem_private_object_init(priv->drm, gobj1, PAGE_SIZE); - drm_gem_private_object_init(priv->drm, gobj2, PAGE_SIZE); + ret = drm_gem_private_object_init(priv->drm, gobj1, PAGE_SIZE); + KUNIT_ASSERT_EQ(test, ret, 0); + ret = drm_gem_private_object_init(priv->drm, gobj2, PAGE_SIZE); + KUNIT_ASSERT_EQ(test, ret, 0); drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0); drm_exec_until_all_locked(&exec) diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c index 56ad8ef32584..bb430763e7a5 100644 --- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c @@ -119,7 +119,8 @@ static void ttm_bo_init_reserved_sys_man(struct kunit *test) place = ttm_place_kunit_init(test, TTM_PL_SYSTEM, 0); placement = ttm_placement_kunit_init(test, place, 1); - drm_gem_private_object_init(priv->drm, &bo->base, size); + err = drm_gem_private_object_init(priv->drm, &bo->base, size); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo, bo_type, placement, PAGE_SIZE, &ctx, NULL, NULL, @@ -168,7 +169,8 @@ static void ttm_bo_init_reserved_mock_man(struct kunit *test) place = ttm_place_kunit_init(test, mem_type, 0); placement = ttm_placement_kunit_init(test, place, 1); - drm_gem_private_object_init(priv->drm, &bo->base, size); + err = drm_gem_private_object_init(priv->drm, &bo->base, size); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo, bo_type, placement, PAGE_SIZE, &ctx, NULL, NULL, @@ -208,7 +210,8 @@ static void ttm_bo_init_reserved_resv(struct kunit *test) place = ttm_place_kunit_init(test, TTM_PL_SYSTEM, 0); placement = ttm_placement_kunit_init(test, place, 1); - drm_gem_private_object_init(priv->drm, &bo->base, size); + err = drm_gem_private_object_init(priv->drm, &bo->base, size); + KUNIT_ASSERT_EQ(test, err, 0); dma_resv_init(&resv); dma_resv_lock(&resv, NULL); @@ -244,7 +247,8 @@ static void ttm_bo_validate_basic(struct kunit *test) bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, bo); - drm_gem_private_object_init(priv->drm, &bo->base, size); + err = drm_gem_private_object_init(priv->drm, &bo->base, size); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo, params->bo_type, fst_placement, PAGE_SIZE, &ctx_init, NULL, @@ -390,7 +394,8 @@ static void ttm_bo_validate_same_placement(struct kunit *test) bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, bo); - drm_gem_private_object_init(priv->drm, &bo->base, size); + err = drm_gem_private_object_init(priv->drm, &bo->base, size); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo, params->bo_type, placement, PAGE_SIZE, &ctx_init, NULL, @@ -431,7 +436,8 @@ static void ttm_bo_validate_busy_placement(struct kunit *test) bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, bo); - drm_gem_private_object_init(priv->drm, &bo->base, size); + err = drm_gem_private_object_init(priv->drm, &bo->base, size); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo, bo_type, placement_init, PAGE_SIZE, &ctx_init, NULL, NULL, @@ -478,7 +484,8 @@ static void ttm_bo_validate_multihop(struct kunit *test) bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, bo); - drm_gem_private_object_init(priv->drm, &bo->base, size); + err = drm_gem_private_object_init(priv->drm, &bo->base, size); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo, params->bo_type, placement_init, PAGE_SIZE, &ctx_init, NULL, @@ -717,7 +724,8 @@ static void ttm_bo_validate_move_fence_not_signaled(struct kunit *test) bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, bo); - drm_gem_private_object_init(priv->drm, &bo->base, size); + err = drm_gem_private_object_init(priv->drm, &bo->base, size); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo, bo_type, placement_init, PAGE_SIZE, &ctx_init, NULL, NULL, @@ -782,7 +790,8 @@ static void ttm_bo_validate_swapout(struct kunit *test) bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, bo); - drm_gem_private_object_init(priv->drm, &bo->base, MANAGER_SIZE); + err = drm_gem_private_object_init(priv->drm, &bo->base, MANAGER_SIZE); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo, bo_type, placement, PAGE_SIZE, &ctx_init, NULL, NULL, &dummy_ttm_bo_destroy); @@ -826,7 +835,8 @@ static void ttm_bo_validate_happy_evict(struct kunit *test) memset(bos, 0, sizeof(*bos) * bo_no); for (i = 0; i < bo_no; i++) { - drm_gem_private_object_init(priv->drm, &bos[i].base, bo_sizes[i]); + err = drm_gem_private_object_init(priv->drm, &bos[i].base, bo_sizes[i]); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, &bos[i], bo_type, placement, PAGE_SIZE, &ctx_init, NULL, NULL, &dummy_ttm_bo_destroy); @@ -875,7 +885,8 @@ static void ttm_bo_validate_all_pinned_evict(struct kunit *test) bo_big = kunit_kzalloc(test, sizeof(*bo_big), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, bo_big); - drm_gem_private_object_init(priv->drm, &bo_big->base, MANAGER_SIZE); + err = drm_gem_private_object_init(priv->drm, &bo_big->base, MANAGER_SIZE); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo_big, bo_type, placement, PAGE_SIZE, &ctx_init, NULL, NULL, &dummy_ttm_bo_destroy); @@ -926,7 +937,8 @@ static void ttm_bo_validate_allowed_only_evict(struct kunit *test) bo_pinned = kunit_kzalloc(test, sizeof(*bo_pinned), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, bo_pinned); - drm_gem_private_object_init(priv->drm, &bo_pinned->base, size); + err = drm_gem_private_object_init(priv->drm, &bo_pinned->base, size); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo_pinned, bo_type, placement, PAGE_SIZE, &ctx_init, NULL, NULL, &dummy_ttm_bo_destroy); @@ -937,7 +949,8 @@ static void ttm_bo_validate_allowed_only_evict(struct kunit *test) bo_evictable = kunit_kzalloc(test, sizeof(*bo_evictable), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, bo_evictable); - drm_gem_private_object_init(priv->drm, &bo_evictable->base, size); + err = drm_gem_private_object_init(priv->drm, &bo_evictable->base, size); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo_evictable, bo_type, placement, PAGE_SIZE, &ctx_init, NULL, NULL, &dummy_ttm_bo_destroy); @@ -991,7 +1004,8 @@ static void ttm_bo_validate_deleted_evict(struct kunit *test) bo_big = kunit_kzalloc(test, sizeof(*bo_big), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, bo_big); - drm_gem_private_object_init(priv->drm, &bo_big->base, big); + err = drm_gem_private_object_init(priv->drm, &bo_big->base, big); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo_big, bo_type, placement, PAGE_SIZE, &ctx_init, NULL, NULL, &dummy_ttm_bo_destroy); @@ -1048,7 +1062,8 @@ static void ttm_bo_validate_busy_domain_evict(struct kunit *test) bo_init = kunit_kzalloc(test, sizeof(*bo_init), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, bo_init); - drm_gem_private_object_init(priv->drm, &bo_init->base, MANAGER_SIZE); + err = drm_gem_private_object_init(priv->drm, &bo_init->base, MANAGER_SIZE); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo_init, bo_type, placement, PAGE_SIZE, &ctx_init, NULL, NULL, &dummy_ttm_bo_destroy); @@ -1092,7 +1107,8 @@ static void ttm_bo_validate_evict_gutting(struct kunit *test) bo_evict = kunit_kzalloc(test, sizeof(*bo_evict), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, bo_evict); - drm_gem_private_object_init(priv->drm, &bo_evict->base, MANAGER_SIZE); + err = drm_gem_private_object_init(priv->drm, &bo_evict->base, MANAGER_SIZE); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo_evict, bo_type, placement, PAGE_SIZE, &ctx_init, NULL, NULL, &dummy_ttm_bo_destroy); @@ -1143,14 +1159,16 @@ static void ttm_bo_validate_recrusive_evict(struct kunit *test) bo_mock = kunit_kzalloc(test, sizeof(*bo_mock), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, bo_mock); - drm_gem_private_object_init(priv->drm, &bo_tt->base, MANAGER_SIZE); + err = drm_gem_private_object_init(priv->drm, &bo_tt->base, MANAGER_SIZE); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo_tt, bo_type, placement_tt, PAGE_SIZE, &ctx_init, NULL, NULL, &dummy_ttm_bo_destroy); KUNIT_EXPECT_EQ(test, err, 0); dma_resv_unlock(bo_tt->base.resv); - drm_gem_private_object_init(priv->drm, &bo_mock->base, MANAGER_SIZE); + err = drm_gem_private_object_init(priv->drm, &bo_mock->base, MANAGER_SIZE); + KUNIT_ASSERT_EQ(test, err, 0); err = ttm_bo_init_reserved(priv->ttm_dev, bo_mock, bo_type, placement_mock, PAGE_SIZE, &ctx_init, NULL, NULL, &dummy_ttm_bo_destroy); diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c index 79964b436243..752f2369d73b 100644 --- a/drivers/gpu/drm/virtio/virtgpu_prime.c +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c @@ -363,7 +363,11 @@ struct drm_gem_object *virtgpu_gem_prime_import(struct drm_device *dev, obj = &bo->base.base; drm_gem_object_set_resv(obj, buf->resv); obj->funcs = &virtgpu_gem_dma_buf_funcs; - drm_gem_private_object_init(dev, obj, buf->size); + ret = drm_gem_private_object_init(dev, obj, buf->size); + if (ret) { + kfree(bo); + return ERR_PTR(ret); + } attach = dma_buf_dynamic_attach(buf, dev->dev, &virtgpu_dma_buf_attach_ops, obj); diff --git a/drivers/gpu/drm/virtio/virtgpu_vram.c b/drivers/gpu/drm/virtio/virtgpu_vram.c index e2e799b42c61..eec843f6701f 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vram.c +++ b/drivers/gpu/drm/virtio/virtgpu_vram.c @@ -211,7 +211,11 @@ int virtio_gpu_vram_create(struct virtio_gpu_device *vgdev, INIT_LIST_HEAD(&vram->base.restore_node); params->size = PAGE_ALIGN(params->size); - drm_gem_private_object_init(vgdev->ddev, obj, params->size); + ret = drm_gem_private_object_init(vgdev->ddev, obj, params->size); + if (ret) { + kfree(vram); + return ret; + } /* Create fake offset */ ret = drm_gem_create_mmap_offset(obj); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 9c7a73c0b0dc..e4e4b87e738d 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -426,7 +426,9 @@ static int vmw_bo_init(struct vmw_private *dev_priv, atomic_set(&vmw_bo->map_count, 0); params->size = ALIGN(params->size, PAGE_SIZE); - drm_gem_private_object_init(vdev, &vmw_bo->tbo.base, params->size); + ret = drm_gem_private_object_init(vdev, &vmw_bo->tbo.base, params->size); + if (ret) + return ret; vmw_bo_placement_set(vmw_bo, params->domain, params->busy_domain); ret = ttm_bo_init_reserved(bdev, &vmw_bo->tbo, params->bo_type, diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c index 85e6d9a0f575..41b404cdd680 100644 --- a/drivers/gpu/drm/xe/xe_bo.c +++ b/drivers/gpu/drm/xe/xe_bo.c @@ -2363,7 +2363,9 @@ struct xe_bo *xe_bo_init_locked(struct xe_device *xe, struct xe_bo *bo, /* Initialize purge advisory state */ bo->purgeable.state = XE_MADV_PURGEABLE_WILLNEED; - drm_gem_private_object_init(&xe->drm, &bo->ttm.base, size); + err = drm_gem_private_object_init(&xe->drm, &bo->ttm.base, size); + if (err) + return ERR_PTR(err); if (resv) { ctx.allow_res_evict = !(flags & XE_BO_FLAG_NO_RESV_EVICT); diff --git a/drivers/gpu/drm/xe/xe_mmio_gem.c b/drivers/gpu/drm/xe/xe_mmio_gem.c index 8c803ef233cc..c1fb4da22f03 100644 --- a/drivers/gpu/drm/xe/xe_mmio_gem.c +++ b/drivers/gpu/drm/xe/xe_mmio_gem.c @@ -86,7 +86,11 @@ struct xe_mmio_gem *xe_mmio_gem_create(struct xe_device *xe, struct drm_file *fi base->funcs = &xe_mmio_gem_funcs; obj->phys_addr = phys_addr; - drm_gem_private_object_init(&xe->drm, base, size); + err = drm_gem_private_object_init(&xe->drm, base, size); + if (err) { + kfree(obj); + return ERR_PTR(err); + } err = drm_gem_create_mmap_offset(base); if (err) diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 09f5ec28218a..f889232e8525 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -526,8 +526,8 @@ void drm_gem_object_release(struct drm_gem_object *obj); void drm_gem_object_free(struct kref *kref); int drm_gem_object_init(struct drm_device *dev, struct drm_gem_object *obj, size_t size); -void drm_gem_private_object_init(struct drm_device *dev, - struct drm_gem_object *obj, size_t size); +int drm_gem_private_object_init(struct drm_device *dev, + struct drm_gem_object *obj, size_t size); void drm_gem_private_object_fini(struct drm_gem_object *obj); struct dma_resv *drm_gem_object_set_resv(struct drm_gem_object *obj, struct dma_resv *resv); -- 2.43.0 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 04/10] drm/gem: Convert drm_gem_private_object_init to return error code 2026-08-27 12:38 ` [PATCH 04/10] drm/gem: Convert drm_gem_private_object_init to return error code Christian König @ 2026-08-27 13:19 ` sashiko-bot 0 siblings, 0 replies; 25+ messages in thread From: sashiko-bot @ 2026-08-27 13:19 UTC (permalink / raw) To: Christian König; +Cc: intel-gfx, dri-devel, intel-xe Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Medium] Memory leak in xe_bo_init_locked() on drm_gem_private_object_init() failure. - [Medium] Memory leak in vmw_bo_create() when vmw_bo_init() fails at GEM initialization. - [Medium] Heap allocation leak in amdxdna_drm_create_dev_bo() when drm_gem_private_object_init() fails. -- commit 31f8fb9f49fa7d6352b54fca867dc5132c27a261 Author: Christian König <ckoenig.leichtzumerken@gmail.com> drm/gem: Convert drm_gem_private_object_init to return error code This commit changes drm_gem_private_object_init() to return an error code instead of void, allowing it to gracefully fail rather than crashing the kernel with BUG_ON. Call sites across various DRM and accelerator drivers have been updated to check the return value and handle errors appropriately. [ ... ] > diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c > index b01f5d95fac83..96490aa0af1be 100644 > --- a/drivers/accel/amdxdna/amdxdna_gem.c > +++ b/drivers/accel/amdxdna/amdxdna_gem.c > @@ -1050,7 +1050,11 @@ amdxdna_drm_create_dev_bo(struct drm_device *dev, > return ERR_PTR(ret); > } > > - drm_gem_private_object_init(dev, gobj, aligned_sz); > + ret = drm_gem_private_object_init(dev, gobj, aligned_sz); > + if (ret) { > + amdxdna_gem_destroy_obj(abo); > + return ERR_PTR(ret); > + } [Severity: Medium] Does this code leak the heap allocation? In amdxdna_drm_create_dev_bo(), there is an earlier successful call to amdxdna_gem_heap_alloc(). If drm_gem_private_object_init() fails, the error handler only calls amdxdna_gem_destroy_obj() and skips calling amdxdna_gem_heap_free(), which leaves a persistent resource leak in the client's heap tracking. [ ... ] > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > index 9c7a73c0b0dc3..e4e4b87e738d3 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > @@ -426,7 +426,9 @@ static int vmw_bo_init(struct vmw_private *dev_priv, > atomic_set(&vmw_bo->map_count, 0); > > params->size = ALIGN(params->size, PAGE_SIZE); > - drm_gem_private_object_init(vdev, &vmw_bo->tbo.base, params->size); > + ret = drm_gem_private_object_init(vdev, &vmw_bo->tbo.base, params->size); > + if (ret) > + return ret; > > vmw_bo_placement_set(vmw_bo, params->domain, params->busy_domain); > ret = ttm_bo_init_reserved(bdev, &vmw_bo->tbo, params->bo_type, [Severity: Medium] Could this early return cause the vmw_bo structure to be leaked? When vmw_bo_create() allocates the object, it passes it to vmw_bo_init() with a destruction callback. If drm_gem_private_object_init() fails and returns early here, it bypasses the ttm_bo_init_reserved() setup. Because TTM never receives the destruction callback, the caller receives the error and nullifies the pointer without actually freeing it, permanently leaking the struct. [ ... ] > diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c > index dde309821237b..85e49b48f9d3a 100644 > --- a/drivers/gpu/drm/xe/xe_bo.c > +++ b/drivers/gpu/drm/xe/xe_bo.c > @@ -2400,7 +2400,9 @@ struct xe_bo *xe_bo_init_locked(struct xe_device *xe, struct xe_bo *bo, > /* Initialize purge advisory state */ > bo->purgeable.state = XE_MADV_PURGEABLE_WILLNEED; > > - drm_gem_private_object_init(&xe->drm, &bo->ttm.base, size); > + err = drm_gem_private_object_init(&xe->drm, &bo->ttm.base, size); > + if (err) > + return ERR_PTR(err); > > if (resv) { > ctx.allow_res_evict = !(flags & XE_BO_FLAG_NO_RESV_EVICT); [Severity: Medium] Can this error path leak the xe_bo struct? If xe_bo_init_locked() is called with a NULL bo pointer, such as from xe_bo_create_locked(), it allocates a new xe_bo structure internally. If drm_gem_private_object_init() then fails, returning ERR_PTR(err) immediately means the newly allocated bo reference is lost before it can be freed with xe_bo_free(). -- Sashiko AI review · https://sashiko.dev/#/patchset/20260827124910.2245-1-christian.koenig@amd.com?part=4 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 05/10] drm/gem: Use dynamic allocation for GEM object dma_resv 2026-08-27 12:38 Refcounting dma_resv v2 Christian König ` (3 preceding siblings ...) 2026-08-27 12:38 ` [PATCH 04/10] drm/gem: Convert drm_gem_private_object_init to return error code Christian König @ 2026-08-27 12:38 ` Christian König 2026-08-27 13:13 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 06/10] drm/mode_config: Use dma_resv_alloc for lockdep annotation Christian König ` (5 subsequent siblings) 10 siblings, 1 reply; 25+ messages in thread From: Christian König @ 2026-08-27 12:38 UTC (permalink / raw) To: thomas.hellstrom, dakr, ecourtney, matthew.brost, nat, dri-devel, intel-gfx, intel-xe, amd-gfx Remove the embedded dma_resv (_resv) from struct drm_gem_object and use dynamically allocated reservations instead. This change aligns with the new reference counted dma_resv infrastructure and reduces the size of drm_gem_object. In drm_gem_private_object_init(), when no external resv is provided, allocate a new dma_resv using dma_resv_alloc() instead of initializing the embedded _resv field. This allows proper error handling if the allocation fails, returning -ENOMEM to the caller. The corresponding cleanup in drm_gem_private_object_fini() is simplified to only put the resv pointer, removing the redundant put on _resv. For TTM buffer objects, add an individual_resv field to track the dynamically allocated reservation object for proper cleanup during destruction. This ensures we can distinguish between shared and individual reservations when freeing resources. This change builds on the previous work to make drm_gem_private_object_init() return an error code, now utilizing that capability to handle allocation failures properly. Signed-off-by: Christian König <christian.koenig@amd.com> Assisted-by: Claude:Sonnet 4 --- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 12 ++--- drivers/gpu/drm/drm_gem.c | 9 ++-- drivers/gpu/drm/i915/gem/i915_gem_object.c | 1 - drivers/gpu/drm/nouveau/nouveau_bo.c | 3 -- .../gpu/drm/ttm/tests/ttm_bo_validate_test.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 26 ++++++----- drivers/gpu/drm/ttm/ttm_bo_util.c | 44 +++++++++++++------ drivers/gpu/drm/xe/xe_bo.c | 8 ++-- include/drm/drm_gem.h | 9 ---- include/drm/ttm/ttm_bo.h | 2 + 11 files changed, 65 insertions(+), 53 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index 20831dbebc31..a168083edab2 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -397,7 +397,7 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo, */ void amdgpu_amdkfd_remove_all_eviction_fences(struct amdgpu_bo *bo) { - struct dma_resv *resv = &bo->tbo.base._resv; + struct dma_resv *resv = bo->tbo.individual_resv; struct dma_fence *fence, *stub; struct dma_resv_iter cursor; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 6c5182d54f7d..026821f6da7c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -1319,7 +1319,7 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object *bo) * So when this locking here fails something is wrong with the reference * counting. */ - if (WARN_ON_ONCE(!dma_resv_trylock(&bo->base._resv))) + if (WARN_ON_ONCE(!dma_resv_trylock(bo->individual_resv))) return; amdgpu_amdkfd_remove_all_eviction_fences(abo); @@ -1329,22 +1329,22 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object *bo) adev->in_suspend || drm_dev_is_unplugged(adev_to_drm(adev))) goto out; - r = dma_resv_reserve_fences(&bo->base._resv, 1); + r = dma_resv_reserve_fences(bo->individual_resv, 1); if (r) goto out; r = amdgpu_ttm_clear_buffer(amdgpu_ttm_next_clear_entity(adev), - abo, &bo->base._resv, &fence, - false, AMDGPU_KERNEL_JOB_ID_CLEAR_ON_RELEASE); + abo, bo->individual_resv, &fence, false, + AMDGPU_KERNEL_JOB_ID_CLEAR_ON_RELEASE); if (WARN_ON(r)) goto out; amdgpu_vram_mgr_set_cleared(bo->resource); - dma_resv_add_fence(&bo->base._resv, fence, DMA_RESV_USAGE_KERNEL); + dma_resv_add_fence(bo->individual_resv, fence, DMA_RESV_USAGE_KERNEL); dma_fence_put(fence); out: - dma_resv_unlock(&bo->base._resv); + dma_resv_unlock(bo->individual_resv); } /** diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index d2ce18bbddea..480391df556d 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -235,9 +235,11 @@ int drm_gem_private_object_init(struct drm_device *dev, obj->handle_count = 0; obj->size = size; mutex_init(&obj->gpuva.lock); - dma_resv_init(&obj->_resv); - if (!obj->resv) - obj->resv = dma_resv_get(&obj->_resv); + if (!obj->resv) { + obj->resv = dma_resv_alloc(); + if (!obj->resv) + return -ENOMEM; + } drm_gem_gpuva_init(obj); @@ -259,7 +261,6 @@ void drm_gem_private_object_fini(struct drm_gem_object *obj) WARN_ON(obj->dma_buf); dma_resv_put(obj->resv); - dma_resv_put(&obj->_resv); mutex_destroy(&obj->gpuva.lock); } EXPORT_SYMBOL(drm_gem_private_object_fini); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index 384c74794fdc..5e48e169f2de 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -144,7 +144,6 @@ void __i915_gem_object_fini(struct drm_i915_gem_object *obj) { mutex_destroy(&obj->mm.get_page.lock); mutex_destroy(&obj->mm.get_dma_page.lock); - dma_resv_put(&obj->base._resv); } /** diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 67c9d32f4f27..65e1f0c2f984 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -159,8 +159,6 @@ nouveau_bo_del_ttm(struct ttm_buffer_object *bo) drm_gem_object_put(nvbo->r_obj); drm_gem_object_release(&bo->base); - } else { - dma_resv_put(&bo->base._resv); } kfree(nvbo); @@ -385,7 +383,6 @@ nouveau_bo_new(struct nouveau_cli *cli, u64 size, int align, return PTR_ERR(nvbo); nvbo->bo.base.size = size; - dma_resv_init(&nvbo->bo.base._resv); drm_vma_node_reset(&nvbo->bo.base.vma_node); /* This must be called before ttm_bo_init_reserved(). Subsequent diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c index bb430763e7a5..6fc381a9d2da 100644 --- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c @@ -637,7 +637,7 @@ static void ttm_bo_validate_no_placement_not_signaled(struct kunit *test) KUNIT_EXPECT_EQ(test, ctx.bytes_moved, 0); if (bo->type != ttm_bo_type_sg) - KUNIT_ASSERT_PTR_EQ(test, bo->base.resv, &bo->base._resv); + KUNIT_ASSERT_PTR_EQ(test, bo->base.resv, &bo->individual_resv); /* Make sure we have an idle object at this point */ dma_resv_wait_timeout(bo->base.resv, usage, false, MAX_SCHEDULE_TIMEOUT); diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 999c24251fb7..ecb5f55a0dd1 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -193,13 +193,13 @@ static int ttm_bo_individualize_resv(struct ttm_buffer_object *bo) { int r; - if (bo->base.resv == &bo->base._resv) + if (bo->base.resv == bo->individual_resv) return 0; - BUG_ON(!dma_resv_trylock(&bo->base._resv)); + BUG_ON(!dma_resv_trylock(bo->individual_resv)); - r = dma_resv_copy_fences(&bo->base._resv, bo->base.resv); - dma_resv_unlock(&bo->base._resv); + r = dma_resv_copy_fences(bo->individual_resv, bo->base.resv); + dma_resv_unlock(bo->individual_resv); if (r) return r; @@ -209,7 +209,7 @@ static int ttm_bo_individualize_resv(struct ttm_buffer_object *bo) * the resv object while holding the lru_lock. */ spin_lock(&bo->bdev->lru_lock); - drm_gem_object_set_resv(&bo->base, &bo->base._resv); + drm_gem_object_set_resv(&bo->base, bo->individual_resv); spin_unlock(&bo->bdev->lru_lock); } @@ -218,7 +218,7 @@ static int ttm_bo_individualize_resv(struct ttm_buffer_object *bo) static void ttm_bo_flush_all_fences(struct ttm_buffer_object *bo) { - struct dma_resv *resv = &bo->base._resv; + struct dma_resv *resv = bo->individual_resv; struct dma_resv_iter cursor; struct dma_fence *fence; @@ -238,8 +238,8 @@ static void ttm_bo_delayed_delete(struct work_struct *work) bo = container_of(work, typeof(*bo), delayed_delete); - dma_resv_wait_timeout(&bo->base._resv, DMA_RESV_USAGE_BOOKKEEP, false, - MAX_SCHEDULE_TIMEOUT); + dma_resv_wait_timeout(bo->individual_resv, DMA_RESV_USAGE_BOOKKEEP, + false, MAX_SCHEDULE_TIMEOUT); dma_resv_lock(bo->base.resv, NULL); ttm_bo_cleanup_memtype_use(bo); dma_resv_unlock(bo->base.resv); @@ -273,7 +273,7 @@ static void ttm_bo_release(struct kref *kref) drm_vma_offset_remove(bdev->vma_manager, &bo->base.vma_node); ttm_mem_io_free(bdev, bo->resource); - if (!dma_resv_test_signaled(&bo->base._resv, + if (!dma_resv_test_signaled(bo->individual_resv, DMA_RESV_USAGE_BOOKKEEP) || (want_init_on_free() && (bo->ttm != NULL)) || bo->type == ttm_bo_type_sg || @@ -316,6 +316,8 @@ static void ttm_bo_release(struct kref *kref) } atomic_dec(&ttm_glob.bo_count); + dma_resv_put(bo->individual_resv); + bo->individual_resv = NULL; bo->destroy(bo); } @@ -1197,7 +1199,11 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, bo->pin_count = 0; bo->sg = sg; bo->bulk_move = NULL; - drm_gem_object_set_resv(&bo->base, resv ?: &bo->base._resv); + + /* Save the original resv object before overwriting it */ + bo->individual_resv = dma_resv_get(bo->base.resv); + if (resv) + drm_gem_object_set_resv(&bo->base, resv); atomic_inc(&ttm_glob.bo_count); /* diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index af5732f9e489..7457a16b744f 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -207,7 +207,7 @@ static void ttm_transfered_destroy(struct ttm_buffer_object *bo) struct ttm_transfer_obj *fbo; fbo = container_of(bo, struct ttm_transfer_obj, base); - dma_resv_put(&fbo->base.base._resv); + dma_resv_put(fbo->base.individual_resv); ttm_bo_put(fbo->bo); kfree(fbo); } @@ -237,12 +237,23 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, if (!fbo) return -ENOMEM; - fbo->base = *bo; /** * Fix up members that we shouldn't copy directly: * TODO: Explicit member copy would probably be better here. */ + fbo->base = *bo; + + fbo->base.individual_resv = dma_resv_alloc(); + if (!fbo->base.individual_resv) { + ret = -ENOMEM; + goto error_free; + } + + if (bo->type != ttm_bo_type_sg) + fbo->base.base.resv = dma_resv_get(fbo->base.individual_resv); + else + dma_resv_get(fbo->base.base.resv); atomic_inc(&ttm_glob.bo_count); drm_vma_node_reset(&fbo->base.base.vma_node); @@ -250,19 +261,16 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, kref_init(&fbo->base.kref); fbo->base.destroy = &ttm_transfered_destroy; fbo->base.pin_count = 0; - if (bo->type != ttm_bo_type_sg) - fbo->base.base.resv = &fbo->base.base._resv; - dma_resv_init(&fbo->base.base._resv); fbo->base.base.dev = NULL; - ret = dma_resv_trylock(&fbo->base.base._resv); + ret = dma_resv_trylock(fbo->base.individual_resv); WARN_ON(!ret); - ret = dma_resv_reserve_fences(&fbo->base.base._resv, TTM_NUM_MOVE_FENCES); + ret = dma_resv_reserve_fences(fbo->base.individual_resv, + TTM_NUM_MOVE_FENCES); if (ret) { - dma_resv_unlock(&fbo->base.base._resv); - kfree(fbo); - return ret; + dma_resv_unlock(fbo->base.individual_resv); + goto error_unref; } if (fbo->base.resource) { @@ -280,6 +288,14 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, *new_obj = &fbo->base; return 0; + +error_unref: + dma_resv_put(fbo->base.individual_resv); + dma_resv_put(fbo->base.base.resv); + +error_free: + kfree(fbo); + return ret; } /** @@ -617,7 +633,7 @@ static int ttm_bo_move_to_ghost(struct ttm_buffer_object *bo, if (ret) return ret; - dma_resv_add_fence(&ghost_obj->base._resv, fence, + dma_resv_add_fence(ghost_obj->individual_resv, fence, DMA_RESV_USAGE_KERNEL); /** @@ -631,7 +647,7 @@ static int ttm_bo_move_to_ghost(struct ttm_buffer_object *bo, else bo->ttm = NULL; - dma_resv_unlock(&ghost_obj->base._resv); + dma_resv_unlock(ghost_obj->individual_resv); ttm_bo_put(ghost_obj); return 0; } @@ -801,14 +817,14 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) if (ret) goto error_destroy_tt; - ret = dma_resv_copy_fences(&ghost->base._resv, bo->base.resv); + ret = dma_resv_copy_fences(ghost->individual_resv, bo->base.resv); /* Last resort, wait for the BO to be idle when we are OOM */ if (ret) { dma_resv_wait_timeout(bo->base.resv, DMA_RESV_USAGE_BOOKKEEP, false, MAX_SCHEDULE_TIMEOUT); } - dma_resv_unlock(&ghost->base._resv); + dma_resv_unlock(ghost->individual_resv); ttm_bo_put(ghost); bo->ttm = ttm; return 0; diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c index 41b404cdd680..37e9e7ce6574 100644 --- a/drivers/gpu/drm/xe/xe_bo.c +++ b/drivers/gpu/drm/xe/xe_bo.c @@ -1660,7 +1660,7 @@ static bool xe_ttm_bo_lock_in_destructor(struct ttm_buffer_object *ttm_bo) * always succeed here, as long as we hold the lru lock. */ spin_lock(&ttm_bo->bdev->lru_lock); - locked = dma_resv_trylock(&ttm_bo->base._resv); + locked = dma_resv_trylock(ttm_bo->individual_resv); spin_unlock(&ttm_bo->bdev->lru_lock); xe_assert(xe, locked); @@ -1689,14 +1689,14 @@ static void xe_ttm_bo_release_notify(struct ttm_buffer_object *ttm_bo) * TODO: Don't do this for external bos once we scrub them after * unbind. */ - dma_resv_for_each_fence(&cursor, &ttm_bo->base._resv, + dma_resv_for_each_fence(&cursor, ttm_bo->individual_resv, DMA_RESV_USAGE_BOOKKEEP, fence) { if (xe_fence_is_xe_preempt(fence) && !dma_fence_is_signaled(fence)) { if (!replacement) replacement = dma_fence_get_stub(); - dma_resv_replace_fences(&ttm_bo->base._resv, + dma_resv_replace_fences(ttm_bo->individual_resv, fence->context, replacement, DMA_RESV_USAGE_BOOKKEEP); @@ -1704,7 +1704,7 @@ static void xe_ttm_bo_release_notify(struct ttm_buffer_object *ttm_bo) } dma_fence_put(replacement); - dma_resv_unlock(&ttm_bo->base._resv); + dma_resv_unlock(ttm_bo->individual_resv); } static void xe_ttm_bo_delete_mem_notify(struct ttm_buffer_object *ttm_bo) diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index f889232e8525..8aed680bfca2 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -385,15 +385,6 @@ struct drm_gem_object { */ struct dma_resv *resv; - /** - * @_resv: - * - * A reservation object for this GEM object. - * - * This is unused for imported GEM objects. - */ - struct dma_resv _resv; - /** * @gpuva: Fields used by GPUVM to manage mappings pointing to this GEM object. * diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h index 32791c4db2a9..f132caa9647f 100644 --- a/include/drm/ttm/ttm_bo.h +++ b/include/drm/ttm/ttm_bo.h @@ -77,6 +77,7 @@ enum ttm_bo_type { * @bdev: Pointer to the buffer object device structure. * @type: The bo type. * @page_alignment: Page alignment. + * @individual_resv: Individual resv for destruction * @destroy: Destruction function. If NULL, kfree is used. * @kref: Reference count of this buffer object. When this refcount reaches * zero, the object is destroyed or put on the delayed delete list. @@ -107,6 +108,7 @@ struct ttm_buffer_object { struct ttm_device *bdev; enum ttm_bo_type type; uint32_t page_alignment; + struct dma_resv *individual_resv; void (*destroy) (struct ttm_buffer_object *); /* -- 2.43.0 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 05/10] drm/gem: Use dynamic allocation for GEM object dma_resv 2026-08-27 12:38 ` [PATCH 05/10] drm/gem: Use dynamic allocation for GEM object dma_resv Christian König @ 2026-08-27 13:13 ` sashiko-bot 0 siblings, 0 replies; 25+ messages in thread From: sashiko-bot @ 2026-08-27 13:13 UTC (permalink / raw) To: Christian König; +Cc: intel-gfx, intel-xe, dri-devel Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [Critical] Uninitialized `dma_resv` for internal Nouveau buffer objects causes NULL pointer dereferences and memory leaks. - [Critical] Massive memory leak of dynamically allocated `dma_resv` objects for all i915 GEM objects. - [High] Memory leak of `dma_resv` object in TTM ghost objects due to missing `dma_resv_put()` in `ttm_transfered_destroy()`. - [Medium] KUnit test runtime crash and compilation failure due to comparing `struct dma_resv *` with `struct dma_resv **`. -- commit ea9a73424b175ca45f706102b9727e26c5e65f0b Author: Christian König <ckoenig.leichtzumerken@gmail.com> drm/gem: Use dynamic allocation for GEM object dma_resv This patch removes the embedded dma_resv from struct drm_gem_object and replaces it with a dynamically allocated reservation object. It updates GEM object initialization to allocate the object and cleans up references appropriately. It also updates TTM and driver-specific code to handle the dynamically allocated individual_resv. > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c > index 384c74794fdc1..5e48e169f2de0 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c > @@ -144,7 +144,6 @@ void __i915_gem_object_fini(struct drm_i915_gem_object *obj) > { > mutex_destroy(&obj->mm.get_page.lock); > mutex_destroy(&obj->mm.get_dma_page.lock); > - dma_resv_put(&obj->base._resv); > } [Severity: Critical] Does this code completely leak the dynamically allocated dma_resv object for all i915 GEM objects? Since the custom destruction logic in __i915_gem_object_fini() no longer calls dma_resv_put() for the base GEM resv object, and drm_gem_private_object_fini() is bypassed by i915, it seems the resv allocated in drm_gem_private_object_init() is permanently lost upon destruction. [ ... ] > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c > index 67c9d32f4f27b..65e1f0c2f984d 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c [ ... ] > @@ -385,7 +383,6 @@ nouveau_bo_new(struct nouveau_cli *cli, u64 size, int align, > return PTR_ERR(nvbo); > > nvbo->bo.base.size = size; > - dma_resv_init(&nvbo->bo.base._resv); > drm_vma_node_reset(&nvbo->bo.base.vma_node); [Severity: Critical] Does this leave the dma_resv pointer uninitialized (NULL) for internal Nouveau buffer objects? Because nouveau_bo_alloc() uses kzalloc() and bypasses standard GEM initialization, bo->base.resv remains NULL without this initialization. Later, when ttm_bo_init_reserved() evaluates WARN_ON(!dma_resv_trylock(bo->base.resv)), won't this result in an immediate NULL pointer dereference because bo->base.resv is NULL? [ ... ] > diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c > index bb430763e7a52..6fc381a9d2da6 100644 > --- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c > +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c > @@ -637,7 +637,7 @@ static void ttm_bo_validate_no_placement_not_signaled(struct kunit *test) > KUNIT_EXPECT_EQ(test, ctx.bytes_moved, 0); > > if (bo->type != ttm_bo_type_sg) > - KUNIT_ASSERT_PTR_EQ(test, bo->base.resv, &bo->base._resv); > + KUNIT_ASSERT_PTR_EQ(test, bo->base.resv, &bo->individual_resv); [Severity: Medium] Can this compile cleanly or pass at runtime? The variable bo->individual_resv is now a pointer to struct dma_resv. By using the address-of operator here, this assertion attempts to compare a struct dma_resv pointer with a double pointer, which will unconditionally fail the test and break the KUnit suite. [ ... ] > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c > index af5732f9e489e..7457a16b744f5 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c > @@ -207,7 +207,7 @@ static void ttm_transfered_destroy(struct ttm_buffer_object *bo) > struct ttm_transfer_obj *fbo; > > fbo = container_of(bo, struct ttm_transfer_obj, base); > - dma_resv_put(&fbo->base.base._resv); > + dma_resv_put(fbo->base.individual_resv); > ttm_bo_put(fbo->bo); > kfree(fbo); > } [Severity: High] Does this code permanently leak the dma_resv object dynamically allocated for a ghost object? In ttm_buffer_object_transfer(), the ghost object acquires an extra reference to fbo->base.base.resv: fbo->base.individual_resv = dma_resv_alloc(); ... fbo->base.base.resv = dma_resv_get(fbo->base.individual_resv); However, when the ghost object is destroyed, ttm_bo_release() sets bo->individual_resv to NULL before calling the destroyer callback. As a result, when ttm_transfered_destroy() calls dma_resv_put(fbo->base.individual_resv), it is operating on a NULL pointer. This leaves the reference on the base GEM dma_resv unreleased, causing a memory leak during buffer evictions. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260827124910.2245-1-christian.koenig@amd.com?part=5 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 06/10] drm/mode_config: Use dma_resv_alloc for lockdep annotation 2026-08-27 12:38 Refcounting dma_resv v2 Christian König ` (4 preceding siblings ...) 2026-08-27 12:38 ` [PATCH 05/10] drm/gem: Use dynamic allocation for GEM object dma_resv Christian König @ 2026-08-27 12:38 ` Christian König 2026-08-27 13:02 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 07/10] drm/xe: " Christian König ` (4 subsequent siblings) 10 siblings, 1 reply; 25+ messages in thread From: Christian König @ 2026-08-27 12:38 UTC (permalink / raw) To: thomas.hellstrom, dakr, ecourtney, matthew.brost, nat, dri-devel, intel-gfx, intel-xe, amd-gfx Replace the stack-allocated dma_resv in the lockdep annotation code with a dynamically allocated one using dma_resv_alloc(). This aligns with the move away from embedded dma_resv structures and towards reference-counted dynamic allocation. The lockdep annotation block in drmm_mode_config_init() is used to establish lock ordering dependencies between modeset locks and dma_resv locks. Since this only runs when CONFIG_LOCKDEP is enabled and the resv is immediately freed after use, the change is straightforward. If allocation fails, return -ENOMEM early. The existing dma_resv_put() at the end of the block properly handles the cleanup. Signed-off-by: Christian König <christian.koenig@amd.com> Assisted-by: Claude:Sonnet 4 --- drivers/gpu/drm/drm_mode_config.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 312d5e9675f0..de424b755c62 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -658,10 +658,12 @@ int drmm_mode_config_init(struct drm_device *dev) if (IS_ENABLED(CONFIG_LOCKDEP)) { struct drm_modeset_acquire_ctx modeset_ctx; struct ww_acquire_ctx resv_ctx; - struct dma_resv resv; + struct dma_resv *resv; int ret; - dma_resv_init(&resv); + resv = dma_resv_alloc(); + if (!resv) + return -ENOMEM; drm_modeset_acquire_init(&modeset_ctx, 0); ret = drm_modeset_lock(&dev->mode_config.connection_mutex, @@ -672,16 +674,16 @@ int drmm_mode_config_init(struct drm_device *dev) might_fault(); ww_acquire_init(&resv_ctx, &reservation_ww_class); - ret = dma_resv_lock(&resv, &resv_ctx); + ret = dma_resv_lock(resv, &resv_ctx); if (ret == -EDEADLK) - dma_resv_lock_slow(&resv, &resv_ctx); + dma_resv_lock_slow(resv, &resv_ctx); - dma_resv_unlock(&resv); + dma_resv_unlock(resv); ww_acquire_fini(&resv_ctx); drm_modeset_drop_locks(&modeset_ctx); drm_modeset_acquire_fini(&modeset_ctx); - dma_resv_put(&resv); + dma_resv_put(resv); } return drmm_add_action_or_reset(dev, drm_mode_config_init_release, -- 2.43.0 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 06/10] drm/mode_config: Use dma_resv_alloc for lockdep annotation 2026-08-27 12:38 ` [PATCH 06/10] drm/mode_config: Use dma_resv_alloc for lockdep annotation Christian König @ 2026-08-27 13:02 ` sashiko-bot 0 siblings, 0 replies; 25+ messages in thread From: sashiko-bot @ 2026-08-27 13:02 UTC (permalink / raw) To: Christian König; +Cc: intel-gfx, intel-xe, dri-devel Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Missing cleanup of drm_mode_config resources on dma_resv_alloc() failure path. -- commit 80153f7004414d9b79ec627bceffaa8edbb89641 Author: Christian König <ckoenig.leichtzumerken@gmail.com> drm/mode_config: Use dma_resv_alloc for lockdep annotation This commit replaces the stack-allocated dma_resv in the lockdep annotation code with a dynamically allocated one using dma_resv_alloc(). If the allocation fails, it returns -ENOMEM early. > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c > --- a/drivers/gpu/drm/drm_mode_config.c > +++ b/drivers/gpu/drm/drm_mode_config.c > @@ -658,10 +658,12 @@ int drmm_mode_config_init(struct drm_device *dev) > if (IS_ENABLED(CONFIG_LOCKDEP)) { > struct drm_modeset_acquire_ctx modeset_ctx; > struct ww_acquire_ctx resv_ctx; > - struct dma_resv resv; > + struct dma_resv *resv; > int ret; > > - dma_resv_init(&resv); > + resv = dma_resv_alloc(); > + if (!resv) > + return -ENOMEM; [Severity: Medium] Does returning -ENOMEM here leak the drm_mode_config resources? Since drmm_add_action_or_reset() hasn't been called yet to register the managed cleanup action, should this failure path call drm_mode_config_cleanup() before returning, similar to how it is handled earlier in drmm_mode_config_init() when drm_mode_create_standard_properties() fails? [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/20260827124910.2245-1-christian.koenig@amd.com?part=6 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 07/10] drm/xe: Use dma_resv_alloc for lockdep annotation 2026-08-27 12:38 Refcounting dma_resv v2 Christian König ` (5 preceding siblings ...) 2026-08-27 12:38 ` [PATCH 06/10] drm/mode_config: Use dma_resv_alloc for lockdep annotation Christian König @ 2026-08-27 12:38 ` Christian König 2026-08-27 13:04 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 08/10] drm/i915/gt: Use dma_resv_alloc for VM reservation objects Christian König ` (3 subsequent siblings) 10 siblings, 1 reply; 25+ messages in thread From: Christian König @ 2026-08-27 12:38 UTC (permalink / raw) To: thomas.hellstrom, dakr, ecourtney, matthew.brost, nat, dri-devel, intel-gfx, intel-xe, amd-gfx Replace the stack-allocated dma_resv in xe_pm_runtime_lockdep_prime() with a dynamically allocated one using dma_resv_alloc(). This aligns with the move away from embedded dma_resv structures and towards reference-counted dynamic allocation. The lockdep priming function is used to establish lock ordering dependencies between runtime PM and dma_resv locks. The resv is only used temporarily for lockdep annotation and is immediately freed after use with dma_resv_put(). If allocation fails, we silently return since this is optional lockdep annotation that only affects dependency checking, not functional behavior. Signed-off-by: Christian König <christian.koenig@amd.com> Assisted-by: Claude:Sonnet 4 --- drivers/gpu/drm/xe/xe_pm.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c index 99562f691080..33b19bcb6f71 100644 --- a/drivers/gpu/drm/xe/xe_pm.c +++ b/drivers/gpu/drm/xe/xe_pm.c @@ -762,15 +762,20 @@ static void xe_rpm_might_enter_cb(const struct xe_device *xe) */ static void xe_pm_runtime_lockdep_prime(void) { - struct dma_resv lockdep_resv; + struct dma_resv *lockdep_resv; + + lockdep_resv = dma_resv_alloc(); + if (!lockdep_resv) + return; - dma_resv_init(&lockdep_resv); lock_map_acquire(&xe_pm_runtime_d3cold_map); /* D3Cold takes the dma_resv locks to evict bos */ - dma_resv_lock(&lockdep_resv, NULL); - dma_resv_unlock(&lockdep_resv); + dma_resv_lock(lockdep_resv, NULL); + dma_resv_unlock(lockdep_resv); lock_map_release(&xe_pm_runtime_d3cold_map); + dma_resv_put(lockdep_resv); + /* Shrinkers might like to wake up the device under reclaim. */ fs_reclaim_acquire(GFP_KERNEL); lock_map_acquire(&xe_pm_runtime_nod3cold_map); -- 2.43.0 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 07/10] drm/xe: Use dma_resv_alloc for lockdep annotation 2026-08-27 12:38 ` [PATCH 07/10] drm/xe: " Christian König @ 2026-08-27 13:04 ` sashiko-bot 0 siblings, 0 replies; 25+ messages in thread From: sashiko-bot @ 2026-08-27 13:04 UTC (permalink / raw) To: Christian König; +Cc: intel-xe, dri-devel, intel-gfx Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Early return on dma_resv_alloc() failure unintentionally skips unrelated lockdep annotations. -- commit 2c2f2285d05d44caf485778222c036bce79fbb3f Author: Christian König <ckoenig.leichtzumerken@gmail.com> drm/xe: Use dma_resv_alloc for lockdep annotation This patch replaces the stack-allocated dma_resv in xe_pm_runtime_lockdep_prime() with a dynamically allocated one to align with the move towards reference-counted dynamic allocation. > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c > index a5289a9df8d25..13f25d15f5482 100644 > --- a/drivers/gpu/drm/xe/xe_pm.c > +++ b/drivers/gpu/drm/xe/xe_pm.c > @@ -764,15 +764,20 @@ static void xe_rpm_might_enter_cb(const struct xe_device *xe) > */ > static void xe_pm_runtime_lockdep_prime(void) > { > - struct dma_resv lockdep_resv; > + struct dma_resv *lockdep_resv; > + > + lockdep_resv = dma_resv_alloc(); > + if (!lockdep_resv) > + return; [Severity: Low] Does this unconditional early return unintentionally skip the unrelated lockdep annotations that follow? If dma_resv_alloc() fails, it looks like we abort the entire xe_pm_runtime_lockdep_prime() function. This skips the completely independent shrinker and nod3cold annotations below. Could this lead to incomplete lockdep coverage under memory pressure? > > - dma_resv_init(&lockdep_resv); > lock_map_acquire(&xe_pm_runtime_d3cold_map); > /* D3Cold takes the dma_resv locks to evict bos */ > - dma_resv_lock(&lockdep_resv, NULL); > - dma_resv_unlock(&lockdep_resv); > + dma_resv_lock(lockdep_resv, NULL); > + dma_resv_unlock(lockdep_resv); > lock_map_release(&xe_pm_runtime_d3cold_map); > > + dma_resv_put(lockdep_resv); > + > /* Shrinkers might like to wake up the device under reclaim. */ > fs_reclaim_acquire(GFP_KERNEL); > lock_map_acquire(&xe_pm_runtime_nod3cold_map); -- Sashiko AI review · https://sashiko.dev/#/patchset/20260827124910.2245-1-christian.koenig@amd.com?part=7 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 08/10] drm/i915/gt: Use dma_resv_alloc for VM reservation objects 2026-08-27 12:38 Refcounting dma_resv v2 Christian König ` (6 preceding siblings ...) 2026-08-27 12:38 ` [PATCH 07/10] drm/xe: " Christian König @ 2026-08-27 12:38 ` Christian König 2026-08-27 13:11 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 09/10] drm/ttm/tests: Use dma_resv_alloc in test files Christian König ` (2 subsequent siblings) 10 siblings, 1 reply; 25+ messages in thread From: Christian König @ 2026-08-27 12:38 UTC (permalink / raw) To: thomas.hellstrom, dakr, ecourtney, matthew.brost, nat, dri-devel, intel-gfx, intel-xe, amd-gfx Replace the embedded dma_resv (_resv) in struct i915_address_space with a dynamically allocated one using dma_resv_alloc(). This aligns with the move away from embedded dma_resv structures and towards reference-counted dynamic allocation throughout the DRM subsystem. The i915 address space structures (GGTT, PPGTT) maintain their own reservation locks for page directory objects and buffer pools. By switching to dynamic allocation, we reduce the size of the VM structures and gain proper error handling if allocation fails. Changes include: - Convert i915_address_space._resv from embedded struct to pointer - Update i915_address_space_init() to return int for error handling - Update ppgtt_init() to return int and handle allocation failures - Update all callers in GGTT, PPGTT, gen6_ppgtt, and gen8_ppgtt code - Update helper functions like i915_vm_resv_get() to use pointer - Add proper cleanup on allocation failures All initialization functions now check the allocation and return -ENOMEM if it fails, with appropriate cleanup of already-allocated resources. Signed-off-by: Christian König <christian.koenig@amd.com> Assisted-by: Claude:Sonnet 4 --- drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 7 ++++++- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 7 ++++++- drivers/gpu/drm/i915/gt/intel_ggtt.c | 14 ++++++++++---- drivers/gpu/drm/i915/gt/intel_gtt.c | 13 +++++++++---- drivers/gpu/drm/i915/gt/intel_gtt.h | 8 ++++---- drivers/gpu/drm/i915/gt/intel_ppgtt.c | 16 ++++++++++++---- 6 files changed, 47 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c index 438cd4724ac4..f8586467aef6 100644 --- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c @@ -437,7 +437,12 @@ struct i915_ppgtt *gen6_ppgtt_create(struct intel_gt *gt) mutex_init(&ppgtt->flush); - ppgtt_init(&ppgtt->base, gt, 0); + err = ppgtt_init(&ppgtt->base, gt, 0); + if (err) { + kfree(ppgtt); + return ERR_PTR(err); + } + ppgtt->base.vm.pd_shift = ilog2(SZ_4K * SZ_4K / sizeof(gen6_pte_t)); ppgtt->base.vm.top = 1; diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index cfa09b250a1b..21cb9db10688 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -1010,7 +1010,12 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt, if (!ppgtt) return ERR_PTR(-ENOMEM); - ppgtt_init(ppgtt, gt, lmem_pt_obj_flags); + err = ppgtt_init(ppgtt, gt, lmem_pt_obj_flags); + if (err) { + kfree(ppgtt); + return ERR_PTR(err); + } + ppgtt->vm.top = i915_vm_is_4lvl(&ppgtt->vm) ? 3 : 2; ppgtt->vm.pd_shift = ilog2(SZ_4K * SZ_4K / sizeof(gen8_pte_t)); diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 3cc8df7b8fad..735f53fa648f 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -56,8 +56,11 @@ static void i915_ggtt_color_adjust(const struct drm_mm_node *node, static int ggtt_init_hw(struct i915_ggtt *ggtt) { struct drm_i915_private *i915 = ggtt->vm.i915; + int ret; - i915_address_space_init(&ggtt->vm, VM_CLASS_GGTT); + ret = i915_address_space_init(&ggtt->vm, VM_CLASS_GGTT); + if (ret) + return ret; ggtt->vm.is_ggtt = true; @@ -1140,7 +1143,7 @@ void i915_ggtt_driver_late_release(struct drm_i915_private *i915) struct i915_ggtt *ggtt = to_gt(i915)->ggtt; GEM_WARN_ON(kref_read(&ggtt->vm.resv_ref) != 1); - dma_resv_put(&ggtt->vm._resv); + dma_resv_put(ggtt->vm._resv); } static unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl) @@ -1514,7 +1517,10 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct intel_gt *gt) ggtt->vm.gt = gt; ggtt->vm.i915 = i915; ggtt->vm.dma = i915->drm.dev; - dma_resv_init(&ggtt->vm._resv); + + ggtt->vm._resv = dma_resv_alloc(); + if (!ggtt->vm._resv) + return -ENOMEM; if (GRAPHICS_VER(i915) >= 8) ret = gen8_gmch_probe(ggtt); @@ -1524,7 +1530,7 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct intel_gt *gt) ret = intel_ggtt_gmch_probe(ggtt); if (ret) { - dma_resv_put(&ggtt->vm._resv); + dma_resv_put(ggtt->vm._resv); return ret; } diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c index 7b1bdb121c88..670cbe0b8cc0 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.c +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c @@ -197,7 +197,7 @@ static void __i915_vm_close(struct i915_address_space *vm) int i915_vm_lock_objects(struct i915_address_space *vm, struct i915_gem_ww_ctx *ww) { - if (vm->scratch[0]->base.resv == &vm->_resv) { + if (vm->scratch[0]->base.resv == vm->_resv) { return i915_gem_object_lock(vm->scratch[0], ww); } else { struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm); @@ -225,7 +225,7 @@ void i915_vm_resv_release(struct kref *kref) struct i915_address_space *vm = container_of(kref, typeof(*vm), resv_ref); - dma_resv_put(&vm->_resv); + dma_resv_put(vm->_resv); mutex_destroy(&vm->mutex); kfree(vm); @@ -258,7 +258,7 @@ void i915_vm_release(struct kref *kref) queue_work(vm->i915->wq, &vm->release_work); } -void i915_address_space_init(struct i915_address_space *vm, int subclass) +int i915_address_space_init(struct i915_address_space *vm, int subclass) { kref_init(&vm->ref); @@ -295,7 +295,10 @@ void i915_address_space_init(struct i915_address_space *vm, int subclass) might_alloc(GFP_KERNEL); mutex_release(&vm->mutex.dep_map, _THIS_IP_); } - dma_resv_init(&vm->_resv); + + vm->_resv = dma_resv_alloc(); + if (!vm->_resv) + return -ENOMEM; GEM_BUG_ON(!vm->total); drm_mm_init(&vm->mm, 0, vm->total); @@ -312,6 +315,8 @@ void i915_address_space_init(struct i915_address_space *vm, int subclass) INIT_LIST_HEAD(&vm->bound_list); INIT_LIST_HEAD(&vm->unbound_list); + + return 0; } void *__px_vaddr(struct drm_i915_gem_object *p) diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h index b54ee4f25af1..3976c176110f 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.h +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h @@ -266,7 +266,7 @@ struct i915_address_space { struct mutex mutex; /* protects vma and our lists */ struct kref resv_ref; /* kref to keep the reservation lock alive. */ - struct dma_resv _resv; /* reservation lock for all pd objects, and buffer pool */ + struct dma_resv *_resv; /* reservation lock for all pd objects, and buffer pool */ #define VM_CLASS_GGTT 0 #define VM_CLASS_PPGTT 1 #define VM_CLASS_DPT 2 @@ -504,7 +504,7 @@ static inline void assert_vm_alive(struct i915_address_space *vm) static inline struct dma_resv *i915_vm_resv_get(struct i915_address_space *vm) { kref_get(&vm->resv_ref); - return &vm->_resv; + return vm->_resv; } void i915_vm_release(struct kref *kref); @@ -525,7 +525,7 @@ static inline void i915_vm_resv_put(struct i915_address_space *vm) kref_put(&vm->resv_ref, i915_vm_resv_release); } -void i915_address_space_init(struct i915_address_space *vm, int subclass); +int i915_address_space_init(struct i915_address_space *vm, int subclass); void i915_address_space_fini(struct i915_address_space *vm); static inline u32 i915_pte_index(u64 address, unsigned int pde_shift) @@ -583,7 +583,7 @@ i915_page_dir_dma_addr(const struct i915_ppgtt *ppgtt, const unsigned int n) return __px_dma(pt ? px_base(pt) : ppgtt->vm.scratch[ppgtt->vm.top]); } -void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt, +int ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt, unsigned long lmem_pt_obj_flags); void intel_ggtt_bind_vma(struct i915_address_space *vm, struct i915_vm_pt_stash *stash, diff --git a/drivers/gpu/drm/i915/gt/intel_ppgtt.c b/drivers/gpu/drm/i915/gt/intel_ppgtt.c index 72d8473a448b..9abd4faa2e55 100644 --- a/drivers/gpu/drm/i915/gt/intel_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ppgtt.c @@ -304,8 +304,8 @@ void i915_vm_free_pt_stash(struct i915_address_space *vm, } } -void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt, - unsigned long lmem_pt_obj_flags) +int ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt, + unsigned long lmem_pt_obj_flags) { struct drm_i915_private *i915 = gt->i915; @@ -315,9 +315,17 @@ void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt, ppgtt->vm.total = BIT_ULL(RUNTIME_INFO(i915)->ppgtt_size); ppgtt->vm.lmem_pt_obj_flags = lmem_pt_obj_flags; - dma_resv_init(&ppgtt->vm._resv); - i915_address_space_init(&ppgtt->vm, VM_CLASS_PPGTT); + ppgtt->vm._resv = dma_resv_alloc(); + if (!ppgtt->vm._resv) + return -ENOMEM; + + if (i915_address_space_init(&ppgtt->vm, VM_CLASS_PPGTT)) { + dma_resv_put(ppgtt->vm._resv); + return -ENOMEM; + } ppgtt->vm.vma_ops.bind_vma = ppgtt_bind_vma; ppgtt->vm.vma_ops.unbind_vma = ppgtt_unbind_vma; + + return 0; } -- 2.43.0 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 08/10] drm/i915/gt: Use dma_resv_alloc for VM reservation objects 2026-08-27 12:38 ` [PATCH 08/10] drm/i915/gt: Use dma_resv_alloc for VM reservation objects Christian König @ 2026-08-27 13:11 ` sashiko-bot 0 siblings, 0 replies; 25+ messages in thread From: sashiko-bot @ 2026-08-27 13:11 UTC (permalink / raw) To: Christian König; +Cc: dri-devel, intel-gfx, intel-xe Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Double allocation of `dma_resv` causes a permanent memory leak because `vm->_resv` is allocated twice and the first pointer is unconditionally overwritten. - [High] Calls to `i915_address_space_init()` ignore the new `-ENOMEM` error return value, leading to incomplete initialization and potential NULL pointer dereferences. -- commit 5239b0e3c5a536da51a7fa10affcf28e378bea31 Author: Christian König <ckoenig.leichtzumerken@gmail.com> drm/i915/gt: Use dma_resv_alloc for VM reservation objects Replaces the embedded dma_resv object in struct i915_address_space with a dynamically allocated pointer, returning int from initialization functions to properly handle allocation failures. > diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c > index 7b1bdb121c88d..670cbe0b8cc05 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gtt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c > @@ -258,7 +258,7 @@ void i915_vm_release(struct kref *kref) > queue_work(vm->i915->wq, &vm->release_work); > } > > -void i915_address_space_init(struct i915_address_space *vm, int subclass) > +int i915_address_space_init(struct i915_address_space *vm, int subclass) > { [Severity: High] Does this change leave some callers unhandled? i915_address_space_init() now returns an error code, but it appears some existing callers were not updated to check the return value. For instance, i915_dpt_create() in drivers/gpu/drm/i915/i915_dpt.c, as well as the mock tests in drivers/gpu/drm/i915/selftests/mock_gtt.c, seem to ignore this return value. If dma_resv_alloc() fails under memory pressure, it could leave vm->mm uninitialized and vm->_resv as NULL, which might lead to a panic later when the Display Page Table is used. [ ... ] > @@ -295,7 +295,10 @@ void i915_address_space_init(struct i915_address_space *vm, int subclass) > might_alloc(GFP_KERNEL); > mutex_release(&vm->mutex.dep_map, _THIS_IP_); > } > - dma_resv_init(&vm->_resv); > + > + vm->_resv = dma_resv_alloc(); > + if (!vm->_resv) > + return -ENOMEM; [Severity: High] Could this unconditionally overwrite an already allocated vm->_resv pointer? The callers ppgtt_init() and ggtt_probe_hw() explicitly allocate vm->_resv before calling i915_address_space_init(), which then allocates it again here. This appears to cause a permanent memory leak of the first allocation on every successful VM initialization. > > GEM_BUG_ON(!vm->total); [ ... ] > diff --git a/drivers/gpu/drm/i915/gt/intel_ppgtt.c b/drivers/gpu/drm/i915/gt/intel_ppgtt.c > index 72d8473a448b9..9abd4faa2e550 100644 > --- a/drivers/gpu/drm/i915/gt/intel_ppgtt.c > +++ b/drivers/gpu/drm/i915/gt/intel_ppgtt.c > @@ -315,9 +315,17 @@ void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt, > ppgtt->vm.total = BIT_ULL(RUNTIME_INFO(i915)->ppgtt_size); > ppgtt->vm.lmem_pt_obj_flags = lmem_pt_obj_flags; > > - dma_resv_init(&ppgtt->vm._resv); > - i915_address_space_init(&ppgtt->vm, VM_CLASS_PPGTT); > + ppgtt->vm._resv = dma_resv_alloc(); > + if (!ppgtt->vm._resv) > + return -ENOMEM; > + > + if (i915_address_space_init(&ppgtt->vm, VM_CLASS_PPGTT)) { > + dma_resv_put(ppgtt->vm._resv); > + return -ENOMEM; > + } [Severity: High] If i915_address_space_init() fails, does this leak the first allocation? Inside i915_address_space_init(), if its own dma_resv_alloc() fails, ppgtt->vm._resv is clobbered to NULL. When control returns here and dma_resv_put() is called, it will be passing NULL, leaving the original allocation leaked. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260827124910.2245-1-christian.koenig@amd.com?part=8 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 09/10] drm/ttm/tests: Use dma_resv_alloc in test files 2026-08-27 12:38 Refcounting dma_resv v2 Christian König ` (7 preceding siblings ...) 2026-08-27 12:38 ` [PATCH 08/10] drm/i915/gt: Use dma_resv_alloc for VM reservation objects Christian König @ 2026-08-27 12:38 ` Christian König 2026-08-27 13:12 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 10/10] dma-buf: Inline dma_resv_init and remove allocated flag Christian König 2026-08-27 13:39 ` ✗ Fi.CI.BUILD: failure for series starting with [01/10] dma-buf: Add reference counting to dma_resv Patchwork 10 siblings, 1 reply; 25+ messages in thread From: Christian König @ 2026-08-27 12:38 UTC (permalink / raw) To: thomas.hellstrom, dakr, ecourtney, matthew.brost, nat, dri-devel, intel-gfx, intel-xe, amd-gfx Replace the remaining dma_resv_init usage in TTM test files with dma_resv_alloc(). This completes the migration away from embedded dma_resv structures to reference-counted dynamic allocation throughout the DRM subsystem. Changes in test files: - ttm_bo_validate_test.c: Convert stack-allocated resv to pointer and use dma_resv_alloc() with proper cleanup via dma_resv_put() - ttm_bo_test.c: Replace kunit_kzalloc + dma_resv_init with direct dma_resv_alloc() calls, add cleanup for external_resv All test assertions updated to check for successful allocation. The tests now properly exercise the dynamic allocation path that production code uses. Signed-off-by: Christian König <christian.koenig@amd.com> Assisted-by: Claude:Sonnet 4 --- drivers/gpu/drm/ttm/tests/ttm_bo_test.c | 11 ++++------- drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c | 13 ++++++++----- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c index 3aff1c427cd1..853024724af9 100644 --- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c @@ -339,14 +339,12 @@ static void ttm_bo_unreserve_bulk(struct kunit *test) ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, ttm_dev); - resv = kunit_kzalloc(test, sizeof(*resv), GFP_KERNEL); - KUNIT_ASSERT_NOT_NULL(test, resv); - err = ttm_device_kunit_init(priv, ttm_dev, 0); KUNIT_ASSERT_EQ(test, err, 0); priv->ttm_dev = ttm_dev; - dma_resv_init(resv); + resv = dma_resv_alloc(); + KUNIT_ASSERT_NOT_NULL(test, resv); bo1 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, resv); bo2 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, resv); @@ -441,11 +439,9 @@ static void ttm_bo_fini_shared_resv(struct kunit *test) KUNIT_ASSERT_EQ(test, err, 0); priv->ttm_dev = ttm_dev; - external_resv = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL); + external_resv = dma_resv_alloc(); KUNIT_ASSERT_NOT_NULL(test, external_resv); - dma_resv_init(external_resv); - fence = kunit_kzalloc(test, sizeof(*fence), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, fence); @@ -464,6 +460,7 @@ static void ttm_bo_fini_shared_resv(struct kunit *test) drm_gem_object_set_resv(&bo->base, external_resv); ttm_bo_fini(bo); + dma_resv_put(external_resv); } static void ttm_bo_pin_basic(struct kunit *test) diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c index 6fc381a9d2da..4559352e7115 100644 --- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c @@ -201,7 +201,7 @@ static void ttm_bo_init_reserved_resv(struct kunit *test) struct ttm_placement *placement; struct ttm_buffer_object *bo; struct ttm_place *place; - struct dma_resv resv; + struct dma_resv *resv; int err; bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL); @@ -212,19 +212,22 @@ static void ttm_bo_init_reserved_resv(struct kunit *test) err = drm_gem_private_object_init(priv->drm, &bo->base, size); KUNIT_ASSERT_EQ(test, err, 0); - dma_resv_init(&resv); - dma_resv_lock(&resv, NULL); + + resv = dma_resv_alloc(); + KUNIT_ASSERT_NOT_NULL(test, resv); + dma_resv_lock(resv, NULL); err = ttm_bo_init_reserved(priv->ttm_dev, bo, bo_type, placement, - PAGE_SIZE, &ctx, NULL, &resv, + PAGE_SIZE, &ctx, NULL, resv, &dummy_ttm_bo_destroy); dma_resv_unlock(bo->base.resv); KUNIT_EXPECT_EQ(test, err, 0); - KUNIT_EXPECT_PTR_EQ(test, bo->base.resv, &resv); + KUNIT_EXPECT_PTR_EQ(test, bo->base.resv, resv); ttm_resource_free(bo, &bo->resource); ttm_bo_fini(bo); + dma_resv_put(resv); } static void ttm_bo_validate_basic(struct kunit *test) -- 2.43.0 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 09/10] drm/ttm/tests: Use dma_resv_alloc in test files 2026-08-27 12:38 ` [PATCH 09/10] drm/ttm/tests: Use dma_resv_alloc in test files Christian König @ 2026-08-27 13:12 ` sashiko-bot 0 siblings, 0 replies; 25+ messages in thread From: sashiko-bot @ 2026-08-27 13:12 UTC (permalink / raw) To: Christian König; +Cc: dri-devel, intel-gfx, intel-xe Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Permanent memory leak of dynamically allocated `dma_resv` objects in test files due to unmanaged allocations without proper cleanup on success and error paths. -- commit 12a4cfd4069727fd22b42f06cb1cc43d2134b9c2 Author: Christian König <ckoenig.leichtzumerken@gmail.com> drm/ttm/tests: Use dma_resv_alloc in test files This commit replaces dma_resv_init usage with dma_resv_alloc() in the TTM test files, moving away from embedded structures to reference-counted dynamic allocations. > --- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c > +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c > @@ -339,14 +339,12 @@ static void ttm_bo_unreserve_bulk(struct kunit *test) [ ... ] > err = ttm_device_kunit_init(priv, ttm_dev, 0); > KUNIT_ASSERT_EQ(test, err, 0); > priv->ttm_dev = ttm_dev; > > - dma_resv_init(resv); > + resv = dma_resv_alloc(); > + KUNIT_ASSERT_NOT_NULL(test, resv); > > bo1 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, resv); > bo2 = ttm_bo_kunit_init(test, test->priv, BO_SIZE, resv); [Severity: Medium] Does this leak the dynamically allocated dma_resv object? Because resv is no longer KUnit-managed, and both bo1 and bo2 take references to it via ttm_bo_kunit_init(), it seems the object will never be fully freed. While this function has a dma_resv_put(resv) at the end, bo1 and bo2 are never explicitly destroyed (e.g., via ttm_bo_fini() as seen in other tests), leaving the refcount elevated on the success path. > @@ -441,11 +439,9 @@ static void ttm_bo_fini_shared_resv(struct kunit *test) > KUNIT_ASSERT_EQ(test, err, 0); > priv->ttm_dev = ttm_dev; > > - external_resv = kunit_kzalloc(test, sizeof(*ttm_dev), GFP_KERNEL); > + external_resv = dma_resv_alloc(); > KUNIT_ASSERT_NOT_NULL(test, external_resv); > > - dma_resv_init(external_resv); > - > fence = kunit_kzalloc(test, sizeof(*fence), GFP_KERNEL); > KUNIT_ASSERT_NOT_NULL(test, fence); [Severity: Medium] Can this leak external_resv on the error path? If the kunit_kzalloc() for the fence fails, the KUNIT_ASSERT_NOT_NULL() macro will abort the test thread immediately. This bypasses the explicit dma_resv_put(external_resv) cleanup at the end of the function, leaking the unmanaged allocation. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260827124910.2245-1-christian.koenig@amd.com?part=9 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 10/10] dma-buf: Inline dma_resv_init and remove allocated flag 2026-08-27 12:38 Refcounting dma_resv v2 Christian König ` (8 preceding siblings ...) 2026-08-27 12:38 ` [PATCH 09/10] drm/ttm/tests: Use dma_resv_alloc in test files Christian König @ 2026-08-27 12:38 ` Christian König 2026-08-27 13:31 ` sashiko-bot 2026-08-27 13:39 ` ✗ Fi.CI.BUILD: failure for series starting with [01/10] dma-buf: Add reference counting to dma_resv Patchwork 10 siblings, 1 reply; 25+ messages in thread From: Christian König @ 2026-08-27 12:38 UTC (permalink / raw) To: thomas.hellstrom, dakr, ecourtney, matthew.brost, nat, dri-devel, intel-gfx, intel-xe, amd-gfx Now that all users have migrated to dma_resv_alloc(), inline the initialization code directly into dma_resv_alloc() and remove the dma_resv_init() function entirely. Additionally, remove the 'allocated' flag from struct dma_resv since all dma_resv objects are now dynamically allocated. This simplifies the reference counting logic - dma_resv_release() now always frees the object unconditionally. The last remaining use of dma_resv_init() in dma_resv_lockdep() has been converted to use dma_resv_alloc() instead. Signed-off-by: Christian König <christian.koenig@amd.com> Assisted-by: Claude:Sonnet 4 --- drivers/dma-buf/dma-resv.c | 41 ++++++++++++++++---------------------- include/linux/dma-resv.h | 10 ---------- 2 files changed, 17 insertions(+), 34 deletions(-) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 48798cec6ce7..a3586e79c2eb 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -132,26 +132,13 @@ static void dma_resv_list_free(struct dma_resv_list *list) kfree_rcu(list, rcu); } -/** - * dma_resv_init - initialize a reservation object - * @obj: the reservation object - */ -void dma_resv_init(struct dma_resv *obj) -{ - kref_init(&obj->refcount); - obj->allocated = false; - ww_mutex_init(&obj->lock, &reservation_ww_class); - - RCU_INIT_POINTER(obj->fences, NULL); -} -EXPORT_SYMBOL(dma_resv_init); - /* * dma_resv_release - release function for kref * @kref: the kref inside the dma_resv object * * This is called when the last reference to a dma_resv object is released. - * Cleans up the object and frees it if it was allocated by dma_resv_alloc(). + * All dma_resv objects are now dynamically allocated, so this always frees + * the object after cleanup. */ static void dma_resv_release(struct kref *kref) { @@ -159,8 +146,7 @@ static void dma_resv_release(struct kref *kref) dma_resv_list_free(rcu_dereference_protected(obj->fences, true)); ww_mutex_destroy(&obj->lock); - if (obj->allocated) - kfree(obj); + kfree(obj); } /** @@ -181,8 +167,9 @@ struct dma_resv *dma_resv_alloc(void) if (!obj) return NULL; - dma_resv_init(obj); - obj->allocated = true; + kref_init(&obj->refcount); + ww_mutex_init(&obj->lock, &reservation_ww_class); + RCU_INIT_POINTER(obj->fences, NULL); return obj; } @@ -838,23 +825,28 @@ static int __init dma_resv_lockdep(void) { struct mm_struct *mm = mm_alloc(); struct ww_acquire_ctx ctx; - struct dma_resv obj; + struct dma_resv *obj; struct address_space mapping; int ret; if (!mm) return -ENOMEM; - dma_resv_init(&obj); + obj = dma_resv_alloc(); + if (!obj) { + mmput(mm); + return -ENOMEM; + } + address_space_init_once(&mapping); mmap_read_lock(mm); ww_acquire_init(&ctx, &reservation_ww_class); - ret = dma_resv_lock(&obj, &ctx); + ret = dma_resv_lock(obj, &ctx); if (ret) { /* Only EDEADLK from the error injection is possible here */ WARN_ON(ret != -EDEADLK); - dma_resv_lock_slow(&obj, &ctx); + dma_resv_lock_slow(obj, &ctx); } fs_reclaim_acquire(GFP_KERNEL); /* for unmap_mapping_range on trylocked buffer objects in shrinkers */ @@ -868,10 +860,11 @@ static int __init dma_resv_lockdep(void) __dma_fence_might_wait(); #endif fs_reclaim_release(GFP_KERNEL); - ww_mutex_unlock(&obj.lock); + ww_mutex_unlock(&obj->lock); ww_acquire_fini(&ctx); mmap_read_unlock(mm); + dma_resv_put(obj); mmput(mm); return 0; diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h index 4d12519df34e..cf689d3d4ba6 100644 --- a/include/linux/dma-resv.h +++ b/include/linux/dma-resv.h @@ -162,15 +162,6 @@ struct dma_resv { */ struct kref refcount; - /** - * @allocated: - * - * True if this object was allocated by dma_resv_alloc(), false if - * embedded in another structure. Used to determine whether to free - * the object memory in the release function. - */ - bool allocated; - /** * @lock: * @@ -482,7 +473,6 @@ static inline void dma_resv_unlock(struct dma_resv *obj) ww_mutex_unlock(&obj->lock); } -void dma_resv_init(struct dma_resv *obj); struct dma_resv *dma_resv_alloc(void); struct dma_resv *dma_resv_get(struct dma_resv *obj); void dma_resv_put(struct dma_resv *obj); -- 2.43.0 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 10/10] dma-buf: Inline dma_resv_init and remove allocated flag 2026-08-27 12:38 ` [PATCH 10/10] dma-buf: Inline dma_resv_init and remove allocated flag Christian König @ 2026-08-27 13:31 ` sashiko-bot 0 siblings, 0 replies; 25+ messages in thread From: sashiko-bot @ 2026-08-27 13:31 UTC (permalink / raw) To: Christian König; +Cc: intel-xe, dri-devel, intel-gfx Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Critical] Synchronous freeing of shared dma_resv objects causes Use-After-Free for concurrent lockless RCU readers. - [High] Drivers like i915 leak dynamically allocated dma_resv objects when destroying GEM objects because they bypass standard DRM cleanup routines. -- commit d696682993c0c4cb40a8a895d00e6e7b158307dd Author: Christian König <ckoenig.leichtzumerken@gmail.com> dma-buf: Inline dma_resv_init and remove allocated flag This commit migrates all dma_resv object initializations directly into dma_resv_alloc() and removes the dma_resv_init() function. It also makes dma_resv objects strictly dynamically allocated, removing the allocated flag and simplifying the cleanup in dma_resv_release() to an unconditional kfree(). > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index 1251b18e1b540..b553ad9c87445 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c [ ... ] > -/** > - * dma_resv_init - initialize a reservation object > - * @obj: the reservation object > - */ > -void dma_resv_init(struct dma_resv *obj) > -{ > - kref_init(&obj->refcount); > - obj->allocated = false; > - ww_mutex_init(&obj->lock, &reservation_ww_class); > - > - RCU_INIT_POINTER(obj->fences, NULL); > -} > -EXPORT_SYMBOL(dma_resv_init); [Severity: High] Does replacing the static initialization with dynamic allocation via dma_resv_alloc() cause memory leaks in drivers that bypass standard DRM cleanup routines? Drivers like i915 use custom cleanup logic. For instance, i915_gem_object_alloc() calls drm_gem_private_object_init(), which dynamically allocates obj->resv via dma_resv_alloc(). When the object is freed, i915 uses __i915_gem_free_object(), which bypasses drm_gem_object_release() and only calls __i915_gem_object_fini(). Since dma_resv_fini() was removed from __i915_gem_object_fini() and not replaced with a call to dma_resv_put(obj->base.resv), the reference is never dropped. Could this permanently leak the dma_resv allocation on every GEM object destruction? [ ... ] > @@ -159,8 +146,7 @@ static void dma_resv_release(struct kref *kref) > > dma_resv_list_free(rcu_dereference_protected(obj->fences, true)); > ww_mutex_destroy(&obj->lock); > - if (obj->allocated) > - kfree(obj); > + kfree(obj); > } [Severity: Critical] Does this synchronous free cause a use-after-free for concurrent lockless RCU readers? In the i915 driver, GEM objects are destroyed using call_rcu() (via __i915_gem_free_object_rcu). Lockless readers like i915_gem_busy_ioctl() look up objects under rcu_read_lock() and access obj->base.resv. If an object shares its resv instance (e.g., via obj->shares_resv_from with an i915_address_space), __i915_gem_free_object() drops the lock reference synchronously via i915_vm_resv_put(). If that drops the last reference, dma_resv_release() synchronously frees the memory here. However, the GEM object itself remains valid during the RCU grace period. Can concurrent RCU readers dereference the already freed dma_resv pointer when calling dma_resv_iter_begin(&cursor, obj->base.resv, ...) in i915_gem_busy_ioctl()? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260827124910.2245-1-christian.koenig@amd.com?part=10 ^ permalink raw reply [flat|nested] 25+ messages in thread
* ✗ Fi.CI.BUILD: failure for series starting with [01/10] dma-buf: Add reference counting to dma_resv 2026-08-27 12:38 Refcounting dma_resv v2 Christian König ` (9 preceding siblings ...) 2026-08-27 12:38 ` [PATCH 10/10] dma-buf: Inline dma_resv_init and remove allocated flag Christian König @ 2026-08-27 13:39 ` Patchwork 10 siblings, 0 replies; 25+ messages in thread From: Patchwork @ 2026-08-27 13:39 UTC (permalink / raw) To: Christian König; +Cc: intel-gfx == Series Details == Series: series starting with [01/10] dma-buf: Add reference counting to dma_resv URL : https://patchwork.freedesktop.org/series/172874/ State : failure == Summary == Error: patch https://patchwork.freedesktop.org/api/1.0/series/172874/revisions/1/mbox/ not applied Applying: dma-buf: Add reference counting to dma_resv Applying: dma-buf/tests: Convert st-dma-resv tests to use dma_resv_alloc Applying: drm/gem: Add helper for drm_gem_object resv assignment Using index info to reconstruct a base tree... M drivers/gpu/drm/drm_gem_shmem_helper.c M drivers/gpu/drm/drm_prime.c M drivers/gpu/drm/msm/msm_gem.c M drivers/gpu/drm/panthor/panthor_gem.c M drivers/gpu/drm/xe/xe_dma_buf.c Falling back to patching base and 3-way merge... Auto-merging drivers/gpu/drm/xe/xe_dma_buf.c Auto-merging drivers/gpu/drm/panthor/panthor_gem.c Auto-merging drivers/gpu/drm/msm/msm_gem.c CONFLICT (content): Merge conflict in drivers/gpu/drm/msm/msm_gem.c Auto-merging drivers/gpu/drm/drm_prime.c Auto-merging drivers/gpu/drm/drm_gem_shmem_helper.c error: Failed to merge in the changes. hint: Use 'git am --show-current-patch=diff' to see the failed patch Patch failed at 0003 drm/gem: Add helper for drm_gem_object resv assignment When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Build failed, no error log produced ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2026-08-28 19:34 UTC | newest] Thread overview: 25+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-27 12:38 Refcounting dma_resv v2 Christian König 2026-08-27 12:38 ` [PATCH 01/10] dma-buf: Add reference counting to dma_resv Christian König 2026-08-27 18:39 ` Andi Shyti 2026-08-28 18:21 ` Danilo Krummrich 2026-08-28 19:17 ` Matthew Brost 2026-08-28 19:34 ` Matthew Brost 2026-08-27 12:38 ` [PATCH 02/10] dma-buf/tests: Convert st-dma-resv tests to use dma_resv_alloc Christian König 2026-08-27 13:08 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 03/10] drm/gem: Add helper for drm_gem_object resv assignment Christian König 2026-08-27 13:22 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 04/10] drm/gem: Convert drm_gem_private_object_init to return error code Christian König 2026-08-27 13:19 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 05/10] drm/gem: Use dynamic allocation for GEM object dma_resv Christian König 2026-08-27 13:13 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 06/10] drm/mode_config: Use dma_resv_alloc for lockdep annotation Christian König 2026-08-27 13:02 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 07/10] drm/xe: " Christian König 2026-08-27 13:04 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 08/10] drm/i915/gt: Use dma_resv_alloc for VM reservation objects Christian König 2026-08-27 13:11 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 09/10] drm/ttm/tests: Use dma_resv_alloc in test files Christian König 2026-08-27 13:12 ` sashiko-bot 2026-08-27 12:38 ` [PATCH 10/10] dma-buf: Inline dma_resv_init and remove allocated flag Christian König 2026-08-27 13:31 ` sashiko-bot 2026-08-27 13:39 ` ✗ Fi.CI.BUILD: failure for series starting with [01/10] dma-buf: Add reference counting to dma_resv Patchwork
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox