Re: [PATCH 3/4] drm/vc4: tests: Stop allocating the state in test init

From: Maíra Canal
Date: Sun Mar 30 2025 - 14:10:38 EST


Hi Maxime,

On 28/03/25 12:32, Maxime Ripard wrote:
On Sun, Mar 23, 2025 at 03:48:17PM -0300, Maíra Canal wrote:
Hi Maxime,

On 18/03/25 11:17, Maxime Ripard wrote:
The vc4-pv-muxing-combinations and vc5-pv-muxing-combinations test
suites use a common test init function which, in part, allocates the
drm atomic state the test will use.

That allocation relies on drm_kunit_helper_atomic_state_alloc(), and
thus requires a struct drm_modeset_acquire_ctx. This context will then
be stored in the allocated state->acquire_ctx field.

However, the context is local to the test init function, and is cleared
as soon as drm_kunit_helper_atomic_state_alloc() is done. We thus end up
with an dangling pointer to a cleared context in state->acquire_ctx for
our test to consumes.

We should really allocate the context and the state in the test
functions, so we can also control when we're done with it.

Fixes: 30188df0c387 ("drm/tests: Drop drm_kunit_helper_acquire_ctx_alloc()")
Signed-off-by: Maxime Ripard <mripard@xxxxxxxxxx>
---
drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c | 41 +++++++++++++++++---------
1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
index 992e8f5c5c6ea8d92338a8fe739fa1115ff85338..52c04ef33206bf4f9e21e3c8b7cea932824a67fa 100644
--- a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
+++ b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
@@ -18,11 +18,10 @@
#include "vc4_mock.h"
struct pv_muxing_priv {
struct vc4_dev *vc4;
- struct drm_atomic_state *state;

Can't we add `struct drm_modeset_acquire_ctx` here? Then, we can be sure
that the context exists during the entire test case.

Also, we can add `drm_modeset_drop_locks()` and
`drm_modeset_acquire_fini()` to a exit function in the kunit suite.

That's what we were doing before, but the kunit functions and exit
functions are run in a separate thread by design, which then cause
lockdep to complain.

It's not great, but we can't really change either :/


Thanks for the explanation, Maxime. In that case,

Reviewed-by: Maíra Canal <mcanal@xxxxxxxxxx>

Best Regards,
- Maíra

Maxime