Hi, Maxime
On 9/5/23 15:16, Maxime Ripard wrote:
On Tue, Sep 05, 2023 at 02:32:38PM +0200, Thomas Hellström wrote:
Hi,
On 9/5/23 14:05, Maxime Ripard wrote:
Hi,
On Tue, Sep 05, 2023 at 10:58:31AM +0200, Thomas Hellström wrote:
Check that object freeing from within drm_exec_fini() works as expected
and doesn't generate any warnings.
Cc: Christian König <christian.koenig@xxxxxxx>
Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/tests/drm_exec_test.c | 47 +++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_exec_test.c b/drivers/gpu/drm/tests/drm_exec_test.c
index 563949d777dd..294c25f49cc7 100644
--- a/drivers/gpu/drm/tests/drm_exec_test.c
+++ b/drivers/gpu/drm/tests/drm_exec_test.c
@@ -170,6 +170,52 @@ static void test_prepare_array(struct kunit *test)
drm_gem_private_object_fini(&gobj2);
}
+static const struct drm_gem_object_funcs put_funcs = {
+ .free = (void *)kfree,
+};
+
+/*
+ * Check that freeing objects from within drm_exec_fini()
+ * behaves as expected.
+ */
+static void test_early_put(struct kunit *test)
+{
+ struct drm_exec_priv *priv = test->priv;
+ struct drm_gem_object *gobj1;
+ struct drm_gem_object *gobj2;
+ struct drm_gem_object *array[2];
+ struct drm_exec exec;
+ int ret;
+
+ gobj1 = kzalloc(sizeof(*gobj1), GFP_KERNEL);
+ KUNIT_EXPECT_NOT_ERR_OR_NULL(test, gobj1);
+ if (!gobj1)
+ return;
+
+ gobj2 = kzalloc(sizeof(*gobj2), GFP_KERNEL);
+ KUNIT_EXPECT_NOT_ERR_OR_NULL(test, gobj2);
+ if (!gobj2) {
+ kfree(gobj1);
+ return;
+ }
+
+ gobj1->funcs = &put_funcs;
+ gobj2->funcs = &put_funcs;
+ array[0] = gobj1;
+ array[1] = gobj2;
+ drm_gem_private_object_init(priv->drm, gobj1, PAGE_SIZE);
+ drm_gem_private_object_init(priv->drm, gobj2, PAGE_SIZE);
+
+ drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);
+ drm_exec_until_all_locked(&exec)
+ ret = drm_exec_prepare_array(&exec, array, ARRAY_SIZE(array),
+ 1);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ drm_gem_object_put(gobj1);
+ drm_gem_object_put(gobj2);
+ drm_exec_fini(&exec);
It doesn't look like you actually check that "freeing objects from
within drm_exec_fini() behaves as expected." What is the expectation
here, and how is it checked?
Hm. Good question, I've been manually checking dmesg for lockdep splats. Is
there a way to automate that?
I'm not familiar with the drm_exec API, but judging by the code I'd
assume you want to check that gobj1 and gobj2 are actually freed using
kfree?
Actually not. What's important here is that the call to drm_exec_fini(),
which puts the last references to gobj1 and gobj2 doesn't trigger any
lockdep splats, like the one in the commit message of patch 3/3. So to
make more sense, the test could perhaps be conditioned on
CONFIG_DEBUG_LOCK_ALLOC. Still it would require manual checking of
dmesg() after being run.
/Thomas
