diff mbox series

[REPOST] drm/ttm/tests: Let ttm_bo_test consider different ww_mutex implementation.

Message ID 20240613064716.WxAIvb9K@linutronix.de (mailing list archive)
State New, archived
Headers show
Series [REPOST] drm/ttm/tests: Let ttm_bo_test consider different ww_mutex implementation. | expand

Commit Message

Sebastian Andrzej Siewior June 13, 2024, 6:47 a.m. UTC
PREEMPT_RT has a different locking implementation for ww_mutex. The
base mutex of struct ww_mutex is declared as struct WW_MUTEX_BASE. The
latter is defined as `mutex' for non-PREEMPT_RT builds and `rt_mutex'
for PREEMPT_RT builds.

Using mutex_lock() directly on the base mutex in
ttm_bo_reserve_deadlock() leads to compile error on PREEMPT_RT.

The locking-selftest has its own defines to deal with this and it is
probably best to defines the needed one within the test program since
their usefulness is limited outside of well known selftests.

Provide ww_mutex_base_lock() which points to the correct function for
PREEMPT_RT and non-PREEMPT_RT builds.

Fixes: 995279d280d1e ("drm/ttm/tests: Add tests for ttm_bo functions")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
Repost of https://lore.kernel.org/r/20240404102534.QTa80QPY@linutronix.de

 drivers/gpu/drm/ttm/tests/ttm_bo_test.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Christian König June 13, 2024, 12:33 p.m. UTC | #1
Am 13.06.24 um 08:47 schrieb Sebastian Andrzej Siewior:
> PREEMPT_RT has a different locking implementation for ww_mutex. The
> base mutex of struct ww_mutex is declared as struct WW_MUTEX_BASE. The
> latter is defined as `mutex' for non-PREEMPT_RT builds and `rt_mutex'
> for PREEMPT_RT builds.
>
> Using mutex_lock() directly on the base mutex in
> ttm_bo_reserve_deadlock() leads to compile error on PREEMPT_RT.
>
> The locking-selftest has its own defines to deal with this and it is
> probably best to defines the needed one within the test program since
> their usefulness is limited outside of well known selftests.
>
> Provide ww_mutex_base_lock() which points to the correct function for
> PREEMPT_RT and non-PREEMPT_RT builds.

In general good that somebody is looking into this, but I can't even 
judge why ww_mutex would use rt_mutex in the first place.

So I don't feel well reviewing this.

Regards,
Christian.

>
> Fixes: 995279d280d1e ("drm/ttm/tests: Add tests for ttm_bo functions")
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> Repost of https://lore.kernel.org/r/20240404102534.QTa80QPY@linutronix.de
>
>   drivers/gpu/drm/ttm/tests/ttm_bo_test.c |    8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> --- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
> +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
> @@ -18,6 +18,12 @@
>   
>   #define BO_SIZE		SZ_8K
>   
> +#ifdef CONFIG_PREEMPT_RT
> +#define ww_mutex_base_lock(b)			rt_mutex_lock(b)
> +#else
> +#define ww_mutex_base_lock(b)			mutex_lock(b)
> +#endif
> +
>   struct ttm_bo_test_case {
>   	const char *description;
>   	bool interruptible;
> @@ -142,7 +148,7 @@ static void ttm_bo_reserve_deadlock(stru
>   	bo2 = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
>   
>   	ww_acquire_init(&ctx1, &reservation_ww_class);
> -	mutex_lock(&bo2->base.resv->lock.base);
> +	ww_mutex_base_lock(&bo2->base.resv->lock.base);
>   
>   	/* The deadlock will be caught by WW mutex, don't warn about it */
>   	lock_release(&bo2->base.resv->lock.base.dep_map, 1);
Sebastian Andrzej Siewior June 13, 2024, 12:58 p.m. UTC | #2
On 2024-06-13 14:33:51 [+0200], Christian König wrote:
> > Provide ww_mutex_base_lock() which points to the correct function for
> > PREEMPT_RT and non-PREEMPT_RT builds.
> 
> In general good that somebody is looking into this, but I can't even judge
> why ww_mutex would use rt_mutex in the first place.

as noted in commit message, this is already done in
lib/locking-selftest.c
The base mutex of struct ww_mutex is WW_MUTEX_BASE which is defined as

| #ifndef CONFIG_PREEMPT_RT
| #define WW_MUTEX_BASE                   mutex
…
| #else
| #define WW_MUTEX_BASE                   rt_mutex
…
| #endif

this is all in-tree.

> So I don't feel well reviewing this.
> 
> Regards,
> Christian.

Sebastian
diff mbox series

Patch

--- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
@@ -18,6 +18,12 @@ 
 
 #define BO_SIZE		SZ_8K
 
+#ifdef CONFIG_PREEMPT_RT
+#define ww_mutex_base_lock(b)			rt_mutex_lock(b)
+#else
+#define ww_mutex_base_lock(b)			mutex_lock(b)
+#endif
+
 struct ttm_bo_test_case {
 	const char *description;
 	bool interruptible;
@@ -142,7 +148,7 @@  static void ttm_bo_reserve_deadlock(stru
 	bo2 = ttm_bo_kunit_init(test, test->priv, BO_SIZE);
 
 	ww_acquire_init(&ctx1, &reservation_ww_class);
-	mutex_lock(&bo2->base.resv->lock.base);
+	ww_mutex_base_lock(&bo2->base.resv->lock.base);
 
 	/* The deadlock will be caught by WW mutex, don't warn about it */
 	lock_release(&bo2->base.resv->lock.base.dep_map, 1);