On 4/2/26 18:13, Bence Csókás wrote:

[Sie erhalten nicht häufig E-Mails von bence.csokas@arm.com. Weitere Informationen, warum dies wichtig ist, finden Sie unter https://aka.ms/LearnAboutSenderIdentification ]

Hi,

I just came across this commit while researching something else. Original patch had too few context lines, so I here's the diff with `-U10`.

On 3/18/25 20:22, Daniel Almeida wrote:

...

From: Asahi Lina lina@asahilina.net

Since commit 21aa27ddc582 ("drm/shmem-helper: Switch to reservation lock"), the drm_gem_shmem_vmap and drm_gem_shmem_vunmap functions require that the caller holds the DMA reservation lock for the object. Add lockdep assertions to help validate this.

There were already lockdep assertions...

Good point, I completely missed that.

...

Signed-off-by: Asahi Lina lina@asahilina.net Signed-off-by: Daniel Almeida daniel.almeida@collabora.com Reviewed-by: Christian König christian.koenig@amd.com Signed-off-by: Lyude Paul lyude@redhat.com Link: https://lore.kernel.org/r/20250318-drm-gem-shmem-v1-1-64b96511a84f@collabora...

---

drivers/gpu/drm/drm_gem_shmem_helper.c | 4 ++++   1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index aa43265f4f4f..0b41f0346bad 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -341,20 +341,22 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_unpin);    *    * Returns:    * 0 on success or a negative error code on failure.    */   int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem,                             struct iosys_map *map)   {       struct drm_gem_object *obj = &shmem->base;       int ret = 0;

+     dma_resv_assert_held(obj->resv);

if (drm_gem_is_imported(obj)) {               ret = dma_buf_vmap(obj->dma_buf, map);       } else {               pgprot_t prot = PAGE_KERNEL;

dma_resv_assert_held(shmem->base.resv);

... right here, and

...

if (refcount_inc_not_zero(&shmem->vmap_use_count)) {                       iosys_map_set_vaddr(map, shmem->vaddr);                       return 0; @@ -401,20 +403,22 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_vmap_locked);    * drops to zero.    *    * This function hides the differences between dma-buf imported and natively    * allocated objects.    */   void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem,                                struct iosys_map *map)   {       struct drm_gem_object *obj = &shmem->base;

+     dma_resv_assert_held(obj->resv);

if (drm_gem_is_imported(obj)) {               dma_buf_vunmap(obj->dma_buf, map);       } else {               dma_resv_assert_held(shmem->base.resv);

...here.

...

if (refcount_dec_and_test(&shmem->vmap_use_count)) {                       vunmap(shmem->vaddr);                       shmem->vaddr = NULL;

drm_gem_shmem_unpin_locked(shmem);

Or were those insufficient for some reason? If so, should we keep both of them, or should the older ones have been removed?

The dma_buf_vmap()/dma_buf_vunmap() functions require the caller to be holding the reservation lock as well.

So it kind of makes sense to move the assertions to the beginning of the functions.

Regards, Christian.

Bence