Upcoming Intel Xe driver will need to have a more fine-grained control
over DRM managed actions - namely, the ability to release a given
action, triggering it manually at a different point in time than the
final drm_dev_put().
This series adds a drmm_release_action function (which is similar to
devres devm_release_action) and a simple test that uses it.
v1 -> v2:
- Split the test changes (Maxime)
- Simplify priv lifetime management (Maxime)
v2 -> v3:
- Order tests alphabetically (Maxime)
- Add comments explaining the intention behind the tests and the reason
why DRM device can't be embedded inside test priv (Maxime)
- Bring back priv lifetime management from v1 to avoid use-after-free
v3 -> v4:
- Split test changes into smaller patches (Maxime)
- Remove the waitqueue usage in tests
- Rename the test suite to match other DRM tests
Michał Winiarski (6):
drm/managed: Add drmm_release_action
drm/tests: managed: Rename the suite name to match other DRM tests
drm/tests: managed: Remove the waitqueue usage
drm/tests: managed: Add comments and expect fail messages
drm/tests: managed: Extract device initialization into test init
drm/tests: managed: Add a simple test for drmm_managed_release
drivers/gpu/drm/drm_managed.c | 39 +++++++++++
drivers/gpu/drm/tests/drm_managed_test.c | 84 +++++++++++++++++-------
include/drm/drm_managed.h | 4 ++
3 files changed, 105 insertions(+), 22 deletions(-)
--
2.43.0
Similar to devres equivalent, it allows to call the "release" action
directly and remove the resource from the managed resources list.
Signed-off-by: Michał Winiarski <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/drm_managed.c | 39 +++++++++++++++++++++++++++++++++++
include/drm/drm_managed.h | 4 ++++
2 files changed, 43 insertions(+)
diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
index bcd111404b128..7646f67bda4e4 100644
--- a/drivers/gpu/drm/drm_managed.c
+++ b/drivers/gpu/drm/drm_managed.c
@@ -176,6 +176,45 @@ int __drmm_add_action_or_reset(struct drm_device *dev,
}
EXPORT_SYMBOL(__drmm_add_action_or_reset);
+/**
+ * drmm_release_action - release a managed action from a &drm_device
+ * @dev: DRM device
+ * @action: function which would be called when @dev is released
+ * @data: opaque pointer, passed to @action
+ *
+ * This function calls the @action previously added by drmm_add_action()
+ * immediately.
+ * The @action is removed from the list of cleanup actions for @dev,
+ * which means that it won't be called in the final drm_dev_put().
+ */
+void drmm_release_action(struct drm_device *dev,
+ drmres_release_t action,
+ void *data)
+{
+ struct drmres *dr_match = NULL, *dr;
+ unsigned long flags;
+
+ spin_lock_irqsave(&dev->managed.lock, flags);
+ list_for_each_entry_reverse(dr, &dev->managed.resources, node.entry) {
+ if (dr->node.release == action) {
+ if (!data || (data && *(void **)dr->data == data)) {
+ dr_match = dr;
+ del_dr(dev, dr_match);
+ break;
+ }
+ }
+ }
+ spin_unlock_irqrestore(&dev->managed.lock, flags);
+
+ if (WARN_ON(!dr_match))
+ return;
+
+ action(dev, data);
+
+ free_dr(dr_match);
+}
+EXPORT_SYMBOL(drmm_release_action);
+
/**
* drmm_kmalloc - &drm_device managed kmalloc()
* @dev: DRM device
diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
index ad08f834af408..f547b09ca0239 100644
--- a/include/drm/drm_managed.h
+++ b/include/drm/drm_managed.h
@@ -45,6 +45,10 @@ int __must_check __drmm_add_action_or_reset(struct drm_device *dev,
drmres_release_t action,
void *data, const char *name);
+void drmm_release_action(struct drm_device *dev,
+ drmres_release_t action,
+ void *data);
+
void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp) __malloc;
/**
--
2.43.0
DRM tests use "_" rather than "-" as word separator. Rename the test
suite to match other tests.
Signed-off-by: Michał Winiarski <[email protected]>
---
drivers/gpu/drm/tests/drm_managed_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tests/drm_managed_test.c b/drivers/gpu/drm/tests/drm_managed_test.c
index 1652dca11d30c..659af5abb8014 100644
--- a/drivers/gpu/drm/tests/drm_managed_test.c
+++ b/drivers/gpu/drm/tests/drm_managed_test.c
@@ -61,7 +61,7 @@ static struct kunit_case drm_managed_tests[] = {
};
static struct kunit_suite drm_managed_test_suite = {
- .name = "drm-test-managed",
+ .name = "drm_test_managed",
.test_cases = drm_managed_tests
};
--
2.43.0
DRM managed release (drm_managed_release) is called as part of devres
release (devres_release_all), which is not async.
The release action should have already been executed once
drm_kunit_helper_free_device exits, meaning that there's no need to use
a waitqueue - we can just inspect the "action_done" state directly.
Signed-off-by: Michał Winiarski <[email protected]>
---
drivers/gpu/drm/tests/drm_managed_test.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_managed_test.c b/drivers/gpu/drm/tests/drm_managed_test.c
index 659af5abb8014..e4790ae838ba7 100644
--- a/drivers/gpu/drm/tests/drm_managed_test.c
+++ b/drivers/gpu/drm/tests/drm_managed_test.c
@@ -8,12 +8,8 @@
#include <linux/device.h>
-/* Ought to be enough for anybody */
-#define TEST_TIMEOUT_MS 100
-
struct managed_test_priv {
bool action_done;
- wait_queue_head_t action_wq;
};
static void drm_action(struct drm_device *drm, void *ptr)
@@ -21,7 +17,6 @@ static void drm_action(struct drm_device *drm, void *ptr)
struct managed_test_priv *priv = ptr;
priv->action_done = true;
- wake_up_interruptible(&priv->action_wq);
}
static void drm_test_managed_run_action(struct kunit *test)
@@ -33,7 +28,6 @@ static void drm_test_managed_run_action(struct kunit *test)
priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
- init_waitqueue_head(&priv->action_wq);
dev = drm_kunit_helper_alloc_device(test);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
@@ -50,9 +44,7 @@ static void drm_test_managed_run_action(struct kunit *test)
drm_dev_unregister(drm);
drm_kunit_helper_free_device(test, dev);
- ret = wait_event_interruptible_timeout(priv->action_wq, priv->action_done,
- msecs_to_jiffies(TEST_TIMEOUT_MS));
- KUNIT_EXPECT_GT(test, ret, 0);
+ KUNIT_EXPECT_TRUE(test, priv->action_done);
}
static struct kunit_case drm_managed_tests[] = {
--
2.43.0
Add comments explaining the intention behind the test and certain
implementation details related to device lifetime.
Signed-off-by: Michał Winiarski <[email protected]>
---
drivers/gpu/drm/tests/drm_managed_test.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tests/drm_managed_test.c b/drivers/gpu/drm/tests/drm_managed_test.c
index e4790ae838ba7..986a38c9144a5 100644
--- a/drivers/gpu/drm/tests/drm_managed_test.c
+++ b/drivers/gpu/drm/tests/drm_managed_test.c
@@ -19,6 +19,10 @@ static void drm_action(struct drm_device *drm, void *ptr)
priv->action_done = true;
}
+/*
+ * The test verifies that the release action is called automatically when the
+ * device is released.
+ */
static void drm_test_managed_run_action(struct kunit *test)
{
struct managed_test_priv *priv;
@@ -32,6 +36,11 @@ static void drm_test_managed_run_action(struct kunit *test)
dev = drm_kunit_helper_alloc_device(test);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
+ /*
+ * DRM device can't be embedded in priv, since priv->action_done needs
+ * to remain allocated beyond both parent device and drm_device
+ * lifetime.
+ */
drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0, DRIVER_MODESET);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm);
@@ -44,7 +53,7 @@ static void drm_test_managed_run_action(struct kunit *test)
drm_dev_unregister(drm);
drm_kunit_helper_free_device(test, dev);
- KUNIT_EXPECT_TRUE(test, priv->action_done);
+ KUNIT_EXPECT_TRUE_MSG(test, priv->action_done, "Release action was not called");
}
static struct kunit_case drm_managed_tests[] = {
--
2.43.0
It simplifies the process of extending the test suite with additional
test cases without unnecessary duplication.
Signed-off-by: Michał Winiarski <[email protected]>
---
drivers/gpu/drm/tests/drm_managed_test.c | 37 +++++++++++++++---------
1 file changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_managed_test.c b/drivers/gpu/drm/tests/drm_managed_test.c
index 986a38c9144a5..c1fc1f0aac9b2 100644
--- a/drivers/gpu/drm/tests/drm_managed_test.c
+++ b/drivers/gpu/drm/tests/drm_managed_test.c
@@ -9,6 +9,7 @@
#include <linux/device.h>
struct managed_test_priv {
+ struct drm_device *drm;
bool action_done;
};
@@ -24,11 +25,26 @@ static void drm_action(struct drm_device *drm, void *ptr)
* device is released.
*/
static void drm_test_managed_run_action(struct kunit *test)
+{
+ struct managed_test_priv *priv = test->priv;
+ int ret;
+
+ ret = drmm_add_action_or_reset(priv->drm, drm_action, priv);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ ret = drm_dev_register(priv->drm, 0);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ drm_dev_unregister(priv->drm);
+ drm_kunit_helper_free_device(test, priv->drm->dev);
+
+ KUNIT_EXPECT_TRUE_MSG(test, priv->action_done, "Release action was not called");
+}
+
+static int drm_managed_test_init(struct kunit *test)
{
struct managed_test_priv *priv;
- struct drm_device *drm;
struct device *dev;
- int ret;
priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
@@ -41,19 +57,13 @@ static void drm_test_managed_run_action(struct kunit *test)
* to remain allocated beyond both parent device and drm_device
* lifetime.
*/
- drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0, DRIVER_MODESET);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm);
+ priv->drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*priv->drm), 0,
+ DRIVER_MODESET);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->drm);
- ret = drmm_add_action_or_reset(drm, drm_action, priv);
- KUNIT_EXPECT_EQ(test, ret, 0);
+ test->priv = priv;
- ret = drm_dev_register(drm, 0);
- KUNIT_ASSERT_EQ(test, ret, 0);
-
- drm_dev_unregister(drm);
- drm_kunit_helper_free_device(test, dev);
-
- KUNIT_EXPECT_TRUE_MSG(test, priv->action_done, "Release action was not called");
+ return 0;
}
static struct kunit_case drm_managed_tests[] = {
@@ -63,6 +73,7 @@ static struct kunit_case drm_managed_tests[] = {
static struct kunit_suite drm_managed_test_suite = {
.name = "drm_test_managed",
+ .init = drm_managed_test_init,
.test_cases = drm_managed_tests
};
--
2.43.0
Add a simple test that checks whether the action is indeed called right
away and that it is not called on the final drm_dev_put().
Signed-off-by: Michał Winiarski <[email protected]>
---
drivers/gpu/drm/tests/drm_managed_test.c | 28 ++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_managed_test.c b/drivers/gpu/drm/tests/drm_managed_test.c
index c1fc1f0aac9b2..91863642efc13 100644
--- a/drivers/gpu/drm/tests/drm_managed_test.c
+++ b/drivers/gpu/drm/tests/drm_managed_test.c
@@ -41,6 +41,33 @@ static void drm_test_managed_run_action(struct kunit *test)
KUNIT_EXPECT_TRUE_MSG(test, priv->action_done, "Release action was not called");
}
+/*
+ * The test verifies that the release action is called immediately when
+ * drmm_release_action is called and that it is not called for a second time
+ * when the device is released.
+ */
+static void drm_test_managed_release_action(struct kunit *test)
+{
+ struct managed_test_priv *priv = test->priv;
+ int ret;
+
+ ret = drmm_add_action_or_reset(priv->drm, drm_action, priv);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ ret = drm_dev_register(priv->drm, 0);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ drmm_release_action(priv->drm, drm_action, priv);
+ KUNIT_EXPECT_TRUE_MSG(test, priv->action_done, "Release action was not called");
+ priv->action_done = false;
+
+ drm_dev_unregister(priv->drm);
+ drm_kunit_helper_free_device(test, priv->drm->dev);
+
+ KUNIT_EXPECT_FALSE_MSG(test, priv->action_done,
+ "Unexpected release action call during cleanup");
+}
+
static int drm_managed_test_init(struct kunit *test)
{
struct managed_test_priv *priv;
@@ -67,6 +94,7 @@ static int drm_managed_test_init(struct kunit *test)
}
static struct kunit_case drm_managed_tests[] = {
+ KUNIT_CASE(drm_test_managed_release_action),
KUNIT_CASE(drm_test_managed_run_action),
{}
};
--
2.43.0
On Fri, Jan 05, 2024 at 11:13:20AM +0100, Michał Winiarski wrote:
> DRM tests use "_" rather than "-" as word separator. Rename the test
> suite to match other tests.
>
> Signed-off-by: Michał Winiarski <[email protected]>
> ---
> drivers/gpu/drm/tests/drm_managed_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tests/drm_managed_test.c b/drivers/gpu/drm/tests/drm_managed_test.c
> index 1652dca11d30c..659af5abb8014 100644
> --- a/drivers/gpu/drm/tests/drm_managed_test.c
> +++ b/drivers/gpu/drm/tests/drm_managed_test.c
> @@ -61,7 +61,7 @@ static struct kunit_case drm_managed_tests[] = {
> };
>
> static struct kunit_suite drm_managed_test_suite = {
> - .name = "drm-test-managed",
> + .name = "drm_test_managed",
> .test_cases = drm_managed_tests
> };
I guess if we were to truly follow the trend, it would be drm_managed?
Maxime
On Fri, Jan 05, 2024 at 11:13:21AM +0100, Michał Winiarski wrote:
> DRM managed release (drm_managed_release) is called as part of devres
> release (devres_release_all), which is not async.
> The release action should have already been executed once
> drm_kunit_helper_free_device exits, meaning that there's no need to use
> a waitqueue - we can just inspect the "action_done" state directly.
>
> Signed-off-by: Michał Winiarski <[email protected]>
I disagree, nothing guarantees in the API that it will be executed right
away. Since it might be asynchronous (if something else holds a
reference for example), we need the workqueue.
Fortunately, it turns out that it's actually done right away, which also
means we'll never hit the timeout and thus never stall the test run.
Maxime
On Fri, Jan 05, 2024 at 11:13:22AM +0100, Michał Winiarski wrote:
> Add comments explaining the intention behind the test and certain
> implementation details related to device lifetime.
>
> Signed-off-by: Michał Winiarski <[email protected]>
> ---
> drivers/gpu/drm/tests/drm_managed_test.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tests/drm_managed_test.c b/drivers/gpu/drm/tests/drm_managed_test.c
> index e4790ae838ba7..986a38c9144a5 100644
> --- a/drivers/gpu/drm/tests/drm_managed_test.c
> +++ b/drivers/gpu/drm/tests/drm_managed_test.c
> @@ -19,6 +19,10 @@ static void drm_action(struct drm_device *drm, void *ptr)
> priv->action_done = true;
> }
>
> +/*
> + * The test verifies that the release action is called automatically when the
> + * device is released.
> + */
> static void drm_test_managed_run_action(struct kunit *test)
> {
> struct managed_test_priv *priv;
> @@ -32,6 +36,11 @@ static void drm_test_managed_run_action(struct kunit *test)
> dev = drm_kunit_helper_alloc_device(test);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
>
> + /*
> + * DRM device can't be embedded in priv, since priv->action_done needs
> + * to remain allocated beyond both parent device and drm_device
> + * lifetime.
> + */
> drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0, DRIVER_MODESET);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm);
>
> @@ -44,7 +53,7 @@ static void drm_test_managed_run_action(struct kunit *test)
> drm_dev_unregister(drm);
> drm_kunit_helper_free_device(test, dev);
>
> - KUNIT_EXPECT_TRUE(test, priv->action_done);
> + KUNIT_EXPECT_TRUE_MSG(test, priv->action_done, "Release action was not called");
I'm fine with the other two comments, but I'm not really sure what that
message brings to the table. It should be pretty obvious from the
function name, variable name and comments already.
Maxime
On Fri, Jan 05, 2024 at 11:13:23AM +0100, Michał Winiarski wrote:
> It simplifies the process of extending the test suite with additional
> test cases without unnecessary duplication.
>
> Signed-off-by: Michał Winiarski <[email protected]>
> ---
> drivers/gpu/drm/tests/drm_managed_test.c | 37 +++++++++++++++---------
> 1 file changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/tests/drm_managed_test.c b/drivers/gpu/drm/tests/drm_managed_test.c
> index 986a38c9144a5..c1fc1f0aac9b2 100644
> --- a/drivers/gpu/drm/tests/drm_managed_test.c
> +++ b/drivers/gpu/drm/tests/drm_managed_test.c
> @@ -9,6 +9,7 @@
> #include <linux/device.h>
>
> struct managed_test_priv {
> + struct drm_device *drm;
> bool action_done;
> };
>
> @@ -24,11 +25,26 @@ static void drm_action(struct drm_device *drm, void *ptr)
> * device is released.
> */
> static void drm_test_managed_run_action(struct kunit *test)
> +{
> + struct managed_test_priv *priv = test->priv;
> + int ret;
> +
> + ret = drmm_add_action_or_reset(priv->drm, drm_action, priv);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + ret = drm_dev_register(priv->drm, 0);
> + KUNIT_ASSERT_EQ(test, ret, 0);
> +
> + drm_dev_unregister(priv->drm);
> + drm_kunit_helper_free_device(test, priv->drm->dev);
> +
> + KUNIT_EXPECT_TRUE_MSG(test, priv->action_done, "Release action was not called");
Aside from the message here I already pointed out,
Acked-by: Maxime Ripard <[email protected]>
Maxime
On Fri, Jan 05, 2024 at 11:13:24AM +0100, Michał Winiarski wrote:
> Add a simple test that checks whether the action is indeed called right
> away and that it is not called on the final drm_dev_put().
>
> Signed-off-by: Michał Winiarski <[email protected]>
> ---
> drivers/gpu/drm/tests/drm_managed_test.c | 28 ++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/tests/drm_managed_test.c b/drivers/gpu/drm/tests/drm_managed_test.c
> index c1fc1f0aac9b2..91863642efc13 100644
> --- a/drivers/gpu/drm/tests/drm_managed_test.c
> +++ b/drivers/gpu/drm/tests/drm_managed_test.c
> @@ -41,6 +41,33 @@ static void drm_test_managed_run_action(struct kunit *test)
> KUNIT_EXPECT_TRUE_MSG(test, priv->action_done, "Release action was not called");
> }
>
> +/*
> + * The test verifies that the release action is called immediately when
> + * drmm_release_action is called and that it is not called for a second time
> + * when the device is released.
> + */
> +static void drm_test_managed_release_action(struct kunit *test)
> +{
> + struct managed_test_priv *priv = test->priv;
> + int ret;
> +
> + ret = drmm_add_action_or_reset(priv->drm, drm_action, priv);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + ret = drm_dev_register(priv->drm, 0);
> + KUNIT_ASSERT_EQ(test, ret, 0);
> +
> + drmm_release_action(priv->drm, drm_action, priv);
> + KUNIT_EXPECT_TRUE_MSG(test, priv->action_done, "Release action was not called");
> + priv->action_done = false;
> +
> + drm_dev_unregister(priv->drm);
> + drm_kunit_helper_free_device(test, priv->drm->dev);
> +
> + KUNIT_EXPECT_FALSE_MSG(test, priv->action_done,
> + "Unexpected release action call during cleanup");
> +}
> +
I guess we can have something simpler if we switch action_done to a
counter and just check that the counter didn't increase.
And I think the custom messages should be removed there too.
Maxime
On Wed, Jan 10, 2024 at 04:56:27PM +0100, Maxime Ripard wrote:
> On Fri, Jan 05, 2024 at 11:13:24AM +0100, Michał Winiarski wrote:
> > Add a simple test that checks whether the action is indeed called right
> > away and that it is not called on the final drm_dev_put().
> >
> > Signed-off-by: Michał Winiarski <[email protected]>
> > ---
> > drivers/gpu/drm/tests/drm_managed_test.c | 28 ++++++++++++++++++++++++
> > 1 file changed, 28 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/tests/drm_managed_test.c b/drivers/gpu/drm/tests/drm_managed_test.c
> > index c1fc1f0aac9b2..91863642efc13 100644
> > --- a/drivers/gpu/drm/tests/drm_managed_test.c
> > +++ b/drivers/gpu/drm/tests/drm_managed_test.c
> > @@ -41,6 +41,33 @@ static void drm_test_managed_run_action(struct kunit *test)
> > KUNIT_EXPECT_TRUE_MSG(test, priv->action_done, "Release action was not called");
> > }
> >
> > +/*
> > + * The test verifies that the release action is called immediately when
> > + * drmm_release_action is called and that it is not called for a second time
> > + * when the device is released.
> > + */
> > +static void drm_test_managed_release_action(struct kunit *test)
> > +{
> > + struct managed_test_priv *priv = test->priv;
> > + int ret;
> > +
> > + ret = drmm_add_action_or_reset(priv->drm, drm_action, priv);
> > + KUNIT_EXPECT_EQ(test, ret, 0);
> > +
> > + ret = drm_dev_register(priv->drm, 0);
> > + KUNIT_ASSERT_EQ(test, ret, 0);
> > +
> > + drmm_release_action(priv->drm, drm_action, priv);
> > + KUNIT_EXPECT_TRUE_MSG(test, priv->action_done, "Release action was not called");
> > + priv->action_done = false;
> > +
> > + drm_dev_unregister(priv->drm);
> > + drm_kunit_helper_free_device(test, priv->drm->dev);
> > +
> > + KUNIT_EXPECT_FALSE_MSG(test, priv->action_done,
> > + "Unexpected release action call during cleanup");
> > +}
> > +
>
> I guess we can have something simpler if we switch action_done to a
> counter and just check that the counter didn't increase.
>
> And I think the custom messages should be removed there too.
>
> Maxime
I'll drop the custom messages here and in the previous patch.
I'll also simplify this test in the way you suggested in previous
revision, to not check for release action call on cleanup.
Thanks,
-Michał