Re: [PATCH 3/5] drm/tests: shmem: Hold reservation lock around vmap/vunmap

[Boris Brezillon <boris.brezillon@collabora.com>]

On Fri, 12 Dec 2025 17:00:34 +0100
Thomas Zimmermann <tzimmermann@suse.de> wrote:

> Acquire and release the GEM object's reservation lock around vmap and
> vunmap operations. The tests use vmap_locked, which led to errors such
> as show below.
> 
> [  122.292030] WARNING: CPU: 3 PID: 1413 at drivers/gpu/drm/drm_gem_shmem_helper.c:390 drm_gem_shmem_vmap_locked+0x3a3/0x6f0
> 
> [  122.468066] WARNING: CPU: 3 PID: 1413 at drivers/gpu/drm/drm_gem_shmem_helper.c:293 drm_gem_shmem_pin_locked+0x1fe/0x350
> 
> [  122.563504] WARNING: CPU: 3 PID: 1413 at drivers/gpu/drm/drm_gem_shmem_helper.c:234 drm_gem_shmem_get_pages_locked+0x23c/0x370
> 
> [  122.662248] WARNING: CPU: 2 PID: 1413 at drivers/gpu/drm/drm_gem_shmem_helper.c:452 drm_gem_shmem_vunmap_locked+0x101/0x330
> 
> Only export the new vmap/vunmap helpers for Kunit tests. These are
> not interfaces for regular drivers.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 954907f7147d ("drm/shmem-helper: Refactor locked/unlocked functions")
> Cc: dri-devel@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v6.16+

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/gpu/drm/drm_gem_shmem_helper.c     | 33 ++++++++++++++++++++++
>  drivers/gpu/drm/tests/drm_gem_shmem_test.c |  6 ++--
>  include/drm/drm_gem_shmem_helper.h         |  9 ++++++
>  3 files changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index dc94a27710e5..06ef4e5adb7d 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -15,6 +15,8 @@
>  #include <asm/set_memory.h>
>  #endif
>  
> +#include <kunit/visibility.h>
> +
>  #include <drm/drm.h>
>  #include <drm/drm_device.h>
>  #include <drm/drm_drv.h>
> @@ -893,6 +895,37 @@ struct drm_gem_object *drm_gem_shmem_prime_import_no_map(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_no_map);
>  
> +/*
> + * Kunit helpers
> + */
> +
> +#if IS_ENABLED(CONFIG_KUNIT)
> +int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem, struct iosys_map *map)
> +{
> +	struct drm_gem_object *obj = &shmem->base;
> +	int ret;
> +
> +	ret = dma_resv_lock_interruptible(obj->resv, NULL);
> +	if (ret)
> +		return ret;
> +	ret = drm_gem_shmem_vmap_locked(shmem, map);
> +	dma_resv_unlock(obj->resv);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_IF_KUNIT(drm_gem_shmem_vmap);
> +
> +void drm_gem_shmem_vunmap(struct drm_gem_shmem_object *shmem, struct iosys_map *map)
> +{
> +	struct drm_gem_object *obj = &shmem->base;
> +
> +	dma_resv_lock_interruptible(obj->resv, NULL);
> +	drm_gem_shmem_vunmap_locked(shmem, map);
> +	dma_resv_unlock(obj->resv);
> +}
> +EXPORT_SYMBOL_IF_KUNIT(drm_gem_shmem_vunmap);
> +#endif
> +
>  MODULE_DESCRIPTION("DRM SHMEM memory-management helpers");
>  MODULE_IMPORT_NS("DMA_BUF");
>  MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/tests/drm_gem_shmem_test.c b/drivers/gpu/drm/tests/drm_gem_shmem_test.c
> index 1d50bab51ef3..3e7c6f20fbcc 100644
> --- a/drivers/gpu/drm/tests/drm_gem_shmem_test.c
> +++ b/drivers/gpu/drm/tests/drm_gem_shmem_test.c
> @@ -19,6 +19,8 @@
>  #include <drm/drm_gem_shmem_helper.h>
>  #include <drm/drm_kunit_helpers.h>
>  
> +MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
> +
>  #define TEST_SIZE		SZ_1M
>  #define TEST_BYTE		0xae
>  
> @@ -176,7 +178,7 @@ static void drm_gem_shmem_test_vmap(struct kunit *test)
>  	ret = kunit_add_action_or_reset(test, drm_gem_shmem_free_wrapper, shmem);
>  	KUNIT_ASSERT_EQ(test, ret, 0);
>  
> -	ret = drm_gem_shmem_vmap_locked(shmem, &map);
> +	ret = drm_gem_shmem_vmap(shmem, &map);
>  	KUNIT_ASSERT_EQ(test, ret, 0);
>  	KUNIT_ASSERT_NOT_NULL(test, shmem->vaddr);
>  	KUNIT_ASSERT_FALSE(test, iosys_map_is_null(&map));
> @@ -186,7 +188,7 @@ static void drm_gem_shmem_test_vmap(struct kunit *test)
>  	for (i = 0; i < TEST_SIZE; i++)
>  		KUNIT_EXPECT_EQ(test, iosys_map_rd(&map, i, u8), TEST_BYTE);
>  
> -	drm_gem_shmem_vunmap_locked(shmem, &map);
> +	drm_gem_shmem_vunmap(shmem, &map);
>  	KUNIT_EXPECT_NULL(test, shmem->vaddr);
>  	KUNIT_EXPECT_EQ(test, refcount_read(&shmem->vmap_use_count), 0);
>  }
> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
> index 589f7bfe7506..6924ee226655 100644
> --- a/include/drm/drm_gem_shmem_helper.h
> +++ b/include/drm/drm_gem_shmem_helper.h
> @@ -303,4 +303,13 @@ struct drm_gem_object *drm_gem_shmem_prime_import_no_map(struct drm_device *dev,
>  	.gem_prime_import       = drm_gem_shmem_prime_import_no_map, \
>  	.dumb_create            = drm_gem_shmem_dumb_create
>  
> +/*
> + * Kunit helpers
> + */
> +
> +#if IS_ENABLED(CONFIG_KUNIT)
> +int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem, struct iosys_map *map);
> +void drm_gem_shmem_vunmap(struct drm_gem_shmem_object *shmem, struct iosys_map *map);
> +#endif
> +
>  #endif /* __DRM_GEM_SHMEM_HELPER_H__ */