linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/4] kunit: Add kunit_add_action() to defer a call until test exit
@ 2023-05-25  4:21 David Gow
  2023-05-25  4:21 ` [PATCH v3 2/4] kunit: executor_test: Use kunit_add_action() David Gow
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: David Gow @ 2023-05-25  4:21 UTC (permalink / raw)
  To: Matti Vaittinen, Daniel Latypov, Maxime Ripard, Benjamin Berg,
	Brendan Higgins, Stephen Boyd, Shuah Khan, Rae Moar
  Cc: David Gow, Greg Kroah-Hartman, Rafael J . Wysocki,
	Heikki Krogerus, Jonathan Cameron, Sadiya Kazi, Jonathan Corbet,
	linux-doc, kunit-dev, linux-kselftest, linux-kernel,
	Benjamin Berg

Many uses of the KUnit resource system are intended to simply defer
calling a function until the test exits (be it due to success or
failure). The existing kunit_alloc_resource() function is often used for
this, but was awkward to use (requiring passing NULL init functions, etc),
and returned a resource without incrementing its reference count, which
-- while okay for this use-case -- could cause problems in others.

Instead, introduce a simple kunit_add_action() API: a simple function
(returning nothing, accepting a single void* argument) can be scheduled
to be called when the test exits. Deferred actions are called in the
opposite order to that which they were registered.

This mimics the devres API, devm_add_action(), and also provides
kunit_remove_action(), to cancel a deferred action, and
kunit_release_action() to trigger one early.

This is implemented as a resource under the hood, so the ordering
between resource cleanup and deferred functions is maintained.

Reviewed-by: Benjamin Berg <benjamin.berg@intel.com>
Reviewed-by: Maxime Ripard <maxime@cerno.tech>
Tested-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: David Gow <davidgow@google.com>
---

No changes since v2:
https://lore.kernel.org/linux-kselftest/20230518083849.2631178-1-davidgow@google.com/

Changes since v1:
https://lore.kernel.org/linux-kselftest/20230421084226.2278282-2-davidgow@google.com/
- Some small documentation updates (Thanks Daniel)
- Reinstate a typedef for the action function.
  - This time, it's called kunit_action_t
  - Thanks Maxime!

Changes since RFC v2:
https://lore.kernel.org/linux-kselftest/20230331080411.981038-2-davidgow@google.com/
- Got rid of internal_gfp
  - everything uses GFP_KERNEL now
  - This includes kunit_kzalloc() and friends, which still allocate the
    returned memory with the provided GFP, but use GFP_KERNEL for
    internal bookkeeping data.
  - Thanks Maxime & Benjamin!
- Got rid of cancellation tokens.
  - kunit_add_action() now returns 0 on success, otherwise an error
  - Note that this can quite easily lead to a memory leak, so look at
    kunit_add_action_or_reset()
  - Thanks Maxime & Benjamin!
- Added kunit_add_action_or_reset
  - Matches devm_add_action_or_reset()
  - Returns 0 on success.
  - Thanks Maxime & Benjamin!
- Got rid of the kunit_defer_func_t typedef.
  - I liked it, but it is probably pushing the boundaries of kernel
    style.
  - Use (void (*)(void *)) instead.

Changes since RFC v1:
https://lore.kernel.org/linux-kselftest/20230325043104.3761770-2-davidgow@google.com/
- Rename functions to better match the devm_* APIs. (Thanks Maxime)
- Embed the kunit_resource in struct kunit_action_ctx to avoid an extra
  allocation (Thanks Benjamin)
- Use 'struct kunit_action_ctx' as the type for cancellation tokens
  (Thanks Benjamin)
- Add tests.


---
 include/kunit/resource.h | 92 +++++++++++++++++++++++++++++++++++++
 lib/kunit/kunit-test.c   | 88 ++++++++++++++++++++++++++++++++++-
 lib/kunit/resource.c     | 99 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 278 insertions(+), 1 deletion(-)

diff --git a/include/kunit/resource.h b/include/kunit/resource.h
index c0d88b318e90..b64eb783b1bc 100644
--- a/include/kunit/resource.h
+++ b/include/kunit/resource.h
@@ -387,4 +387,96 @@ static inline int kunit_destroy_named_resource(struct kunit *test,
  */
 void kunit_remove_resource(struct kunit *test, struct kunit_resource *res);
 
+/* A 'deferred action' function to be used with kunit_add_action. */
+typedef void (kunit_action_t)(void *);
+
+/**
+ * kunit_add_action() - Call a function when the test ends.
+ * @test: Test case to associate the action with.
+ * @func: The function to run on test exit
+ * @ctx: Data passed into @func
+ *
+ * Defer the execution of a function until the test exits, either normally or
+ * due to a failure.  @ctx is passed as additional context. All functions
+ * registered with kunit_add_action() will execute in the opposite order to that
+ * they were registered in.
+ *
+ * This is useful for cleaning up allocated memory and resources, as these
+ * functions are called even if the test aborts early due to, e.g., a failed
+ * assertion.
+ *
+ * See also: devm_add_action() for the devres equivalent.
+ *
+ * Returns:
+ *   0 on success, an error if the action could not be deferred.
+ */
+int kunit_add_action(struct kunit *test, kunit_action_t *action, void *ctx);
+
+/**
+ * kunit_add_action_or_reset() - Call a function when the test ends.
+ * @test: Test case to associate the action with.
+ * @func: The function to run on test exit
+ * @ctx: Data passed into @func
+ *
+ * Defer the execution of a function until the test exits, either normally or
+ * due to a failure.  @ctx is passed as additional context. All functions
+ * registered with kunit_add_action() will execute in the opposite order to that
+ * they were registered in.
+ *
+ * This is useful for cleaning up allocated memory and resources, as these
+ * functions are called even if the test aborts early due to, e.g., a failed
+ * assertion.
+ *
+ * If the action cannot be created (e.g., due to the system being out of memory),
+ * then action(ctx) will be called immediately, and an error will be returned.
+ *
+ * See also: devm_add_action_or_reset() for the devres equivalent.
+ *
+ * Returns:
+ *   0 on success, an error if the action could not be deferred.
+ */
+int kunit_add_action_or_reset(struct kunit *test, kunit_action_t *action,
+			      void *ctx);
+
+/**
+ * kunit_remove_action() - Cancel a matching deferred action.
+ * @test: Test case the action is associated with.
+ * @func: The deferred function to cancel.
+ * @ctx: The context passed to the deferred function to trigger.
+ *
+ * Prevent an action deferred via kunit_add_action() from executing when the
+ * test terminates.
+ *
+ * If the function/context pair was deferred multiple times, only the most
+ * recent one will be cancelled.
+ *
+ * See also: devm_remove_action() for the devres equivalent.
+ */
+void kunit_remove_action(struct kunit *test,
+			 kunit_action_t *action,
+			 void *ctx);
+
+/**
+ * kunit_release_action() - Run a matching action call immediately.
+ * @test: Test case the action is associated with.
+ * @func: The deferred function to trigger.
+ * @ctx: The context passed to the deferred function to trigger.
+ *
+ * Execute a function deferred via kunit_add_action()) immediately, rather than
+ * when the test ends.
+ *
+ * If the function/context pair was deferred multiple times, it will only be
+ * executed once here. The most recent deferral will no longer execute when
+ * the test ends.
+ *
+ * kunit_release_action(test, func, ctx);
+ * is equivalent to
+ * func(ctx);
+ * kunit_remove_action(test, func, ctx);
+ *
+ * See also: devm_release_action() for the devres equivalent.
+ */
+void kunit_release_action(struct kunit *test,
+			  kunit_action_t *action,
+			  void *ctx);
 #endif /* _KUNIT_RESOURCE_H */
diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c
index 42e44caa1bdd..83d8e90ca7a2 100644
--- a/lib/kunit/kunit-test.c
+++ b/lib/kunit/kunit-test.c
@@ -112,7 +112,7 @@ struct kunit_test_resource_context {
 	struct kunit test;
 	bool is_resource_initialized;
 	int allocate_order[2];
-	int free_order[2];
+	int free_order[4];
 };
 
 static int fake_resource_init(struct kunit_resource *res, void *context)
@@ -403,6 +403,88 @@ static void kunit_resource_test_named(struct kunit *test)
 	KUNIT_EXPECT_TRUE(test, list_empty(&test->resources));
 }
 
+static void increment_int(void *ctx)
+{
+	int *i = (int *)ctx;
+	(*i)++;
+}
+
+static void kunit_resource_test_action(struct kunit *test)
+{
+	int num_actions = 0;
+
+	kunit_add_action(test, increment_int, &num_actions);
+	KUNIT_EXPECT_EQ(test, num_actions, 0);
+	kunit_cleanup(test);
+	KUNIT_EXPECT_EQ(test, num_actions, 1);
+
+	/* Once we've cleaned up, the action queue is empty. */
+	kunit_cleanup(test);
+	KUNIT_EXPECT_EQ(test, num_actions, 1);
+
+	/* Check the same function can be deferred multiple times. */
+	kunit_add_action(test, increment_int, &num_actions);
+	kunit_add_action(test, increment_int, &num_actions);
+	kunit_cleanup(test);
+	KUNIT_EXPECT_EQ(test, num_actions, 3);
+}
+static void kunit_resource_test_remove_action(struct kunit *test)
+{
+	int num_actions = 0;
+
+	kunit_add_action(test, increment_int, &num_actions);
+	KUNIT_EXPECT_EQ(test, num_actions, 0);
+
+	kunit_remove_action(test, increment_int, &num_actions);
+	kunit_cleanup(test);
+	KUNIT_EXPECT_EQ(test, num_actions, 0);
+}
+static void kunit_resource_test_release_action(struct kunit *test)
+{
+	int num_actions = 0;
+
+	kunit_add_action(test, increment_int, &num_actions);
+	KUNIT_EXPECT_EQ(test, num_actions, 0);
+	/* Runs immediately on trigger. */
+	kunit_release_action(test, increment_int, &num_actions);
+	KUNIT_EXPECT_EQ(test, num_actions, 1);
+
+	/* Doesn't run again on test exit. */
+	kunit_cleanup(test);
+	KUNIT_EXPECT_EQ(test, num_actions, 1);
+}
+static void action_order_1(void *ctx)
+{
+	struct kunit_test_resource_context *res_ctx = (struct kunit_test_resource_context *)ctx;
+
+	KUNIT_RESOURCE_TEST_MARK_ORDER(res_ctx, free_order, 1);
+	kunit_log(KERN_INFO, current->kunit_test, "action_order_1");
+}
+static void action_order_2(void *ctx)
+{
+	struct kunit_test_resource_context *res_ctx = (struct kunit_test_resource_context *)ctx;
+
+	KUNIT_RESOURCE_TEST_MARK_ORDER(res_ctx, free_order, 2);
+	kunit_log(KERN_INFO, current->kunit_test, "action_order_2");
+}
+static void kunit_resource_test_action_ordering(struct kunit *test)
+{
+	struct kunit_test_resource_context *ctx = test->priv;
+
+	kunit_add_action(test, action_order_1, ctx);
+	kunit_add_action(test, action_order_2, ctx);
+	kunit_add_action(test, action_order_1, ctx);
+	kunit_add_action(test, action_order_2, ctx);
+	kunit_remove_action(test, action_order_1, ctx);
+	kunit_release_action(test, action_order_2, ctx);
+	kunit_cleanup(test);
+
+	/* [2 is triggered] [2], [(1 is cancelled)] [1] */
+	KUNIT_EXPECT_EQ(test, ctx->free_order[0], 2);
+	KUNIT_EXPECT_EQ(test, ctx->free_order[1], 2);
+	KUNIT_EXPECT_EQ(test, ctx->free_order[2], 1);
+}
+
 static int kunit_resource_test_init(struct kunit *test)
 {
 	struct kunit_test_resource_context *ctx =
@@ -434,6 +516,10 @@ static struct kunit_case kunit_resource_test_cases[] = {
 	KUNIT_CASE(kunit_resource_test_proper_free_ordering),
 	KUNIT_CASE(kunit_resource_test_static),
 	KUNIT_CASE(kunit_resource_test_named),
+	KUNIT_CASE(kunit_resource_test_action),
+	KUNIT_CASE(kunit_resource_test_remove_action),
+	KUNIT_CASE(kunit_resource_test_release_action),
+	KUNIT_CASE(kunit_resource_test_action_ordering),
 	{}
 };
 
diff --git a/lib/kunit/resource.c b/lib/kunit/resource.c
index c414df922f34..f0209252b179 100644
--- a/lib/kunit/resource.c
+++ b/lib/kunit/resource.c
@@ -77,3 +77,102 @@ int kunit_destroy_resource(struct kunit *test, kunit_resource_match_t match,
 	return 0;
 }
 EXPORT_SYMBOL_GPL(kunit_destroy_resource);
+
+struct kunit_action_ctx {
+	struct kunit_resource res;
+	kunit_action_t *func;
+	void *ctx;
+};
+
+static void __kunit_action_free(struct kunit_resource *res)
+{
+	struct kunit_action_ctx *action_ctx = container_of(res, struct kunit_action_ctx, res);
+
+	action_ctx->func(action_ctx->ctx);
+}
+
+
+int kunit_add_action(struct kunit *test, void (*action)(void *), void *ctx)
+{
+	struct kunit_action_ctx *action_ctx;
+
+	KUNIT_ASSERT_NOT_NULL_MSG(test, action, "Tried to action a NULL function!");
+
+	action_ctx = kzalloc(sizeof(*action_ctx), GFP_KERNEL);
+	if (!action_ctx)
+		return -ENOMEM;
+
+	action_ctx->func = action;
+	action_ctx->ctx = ctx;
+
+	action_ctx->res.should_kfree = true;
+	/* As init is NULL, this cannot fail. */
+	__kunit_add_resource(test, NULL, __kunit_action_free, &action_ctx->res, action_ctx);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(kunit_add_action);
+
+int kunit_add_action_or_reset(struct kunit *test, void (*action)(void *),
+			      void *ctx)
+{
+	int res = kunit_add_action(test, action, ctx);
+
+	if (res)
+		action(ctx);
+	return res;
+}
+EXPORT_SYMBOL_GPL(kunit_add_action_or_reset);
+
+static bool __kunit_action_match(struct kunit *test,
+				struct kunit_resource *res, void *match_data)
+{
+	struct kunit_action_ctx *match_ctx = (struct kunit_action_ctx *)match_data;
+	struct kunit_action_ctx *res_ctx = container_of(res, struct kunit_action_ctx, res);
+
+	/* Make sure this is a free function. */
+	if (res->free != __kunit_action_free)
+		return false;
+
+	/* Both the function and context data should match. */
+	return (match_ctx->func == res_ctx->func) && (match_ctx->ctx == res_ctx->ctx);
+}
+
+void kunit_remove_action(struct kunit *test,
+			kunit_action_t *action,
+			void *ctx)
+{
+	struct kunit_action_ctx match_ctx;
+	struct kunit_resource *res;
+
+	match_ctx.func = action;
+	match_ctx.ctx = ctx;
+
+	res = kunit_find_resource(test, __kunit_action_match, &match_ctx);
+	if (res) {
+		/* Remove the free function so we don't run the action. */
+		res->free = NULL;
+		kunit_remove_resource(test, res);
+		kunit_put_resource(res);
+	}
+}
+EXPORT_SYMBOL_GPL(kunit_remove_action);
+
+void kunit_release_action(struct kunit *test,
+			 kunit_action_t *action,
+			 void *ctx)
+{
+	struct kunit_action_ctx match_ctx;
+	struct kunit_resource *res;
+
+	match_ctx.func = action;
+	match_ctx.ctx = ctx;
+
+	res = kunit_find_resource(test, __kunit_action_match, &match_ctx);
+	if (res) {
+		kunit_remove_resource(test, res);
+		/* We have to put() this here, else free won't be called. */
+		kunit_put_resource(res);
+	}
+}
+EXPORT_SYMBOL_GPL(kunit_release_action);
-- 
2.41.0.rc0.172.g3f132b7071-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v3 2/4] kunit: executor_test: Use kunit_add_action()
  2023-05-25  4:21 [PATCH v3 1/4] kunit: Add kunit_add_action() to defer a call until test exit David Gow
@ 2023-05-25  4:21 ` David Gow
  2023-05-25  4:21 ` [PATCH v3 3/4] kunit: kmalloc_array: " David Gow
  2023-05-25  4:21 ` [PATCH v3 4/4] Documentation: kunit: Add usage notes for kunit_add_action() David Gow
  2 siblings, 0 replies; 4+ messages in thread
From: David Gow @ 2023-05-25  4:21 UTC (permalink / raw)
  To: Matti Vaittinen, Daniel Latypov, Maxime Ripard, Benjamin Berg,
	Brendan Higgins, Stephen Boyd, Shuah Khan, Rae Moar
  Cc: David Gow, Greg Kroah-Hartman, Rafael J . Wysocki,
	Heikki Krogerus, Jonathan Cameron, Sadiya Kazi, Jonathan Corbet,
	linux-doc, kunit-dev, linux-kselftest, linux-kernel,
	Benjamin Berg

Now we have the kunit_add_action() function, we can use it to implement
kfree_at_end() and free_subsuite_at_end() without the need for extra
helper functions.

Reviewed-by: Benjamin Berg <benjamin.berg@intel.com>
Reviewed-by: Maxime Ripard <maxime@cerno.tech>
Tested-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: David Gow <davidgow@google.com>
---

No changes since v2:
https://lore.kernel.org/linux-kselftest/20230518083849.2631178-2-davidgow@google.com/

Changes since v1:
https://lore.kernel.org/linux-kselftest/20230421084226.2278282-3-davidgow@google.com/
- Use the kunit_action_t typedef

Changes since RFCv2:
https://lore.kernel.org/linux-kselftest/20230331080411.981038-3-davidgow@google.com/
- Don't use the no-longer-extant kunit_defer_func_t typedef.
- Don't pass a GFP pointer in.


---
 lib/kunit/executor_test.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c
index 0cea31c27b23..ce6749af374d 100644
--- a/lib/kunit/executor_test.c
+++ b/lib/kunit/executor_test.c
@@ -125,11 +125,6 @@ kunit_test_suites(&executor_test_suite);
 
 /* Test helpers */
 
-static void kfree_res_free(struct kunit_resource *res)
-{
-	kfree(res->data);
-}
-
 /* Use the resource API to register a call to kfree(to_free).
  * Since we never actually use the resource, it's safe to use on const data.
  */
@@ -138,8 +133,10 @@ static void kfree_at_end(struct kunit *test, const void *to_free)
 	/* kfree() handles NULL already, but avoid allocating a no-op cleanup. */
 	if (IS_ERR_OR_NULL(to_free))
 		return;
-	kunit_alloc_resource(test, NULL, kfree_res_free, GFP_KERNEL,
-			     (void *)to_free);
+
+	kunit_add_action(test,
+			(kunit_action_t *)kfree,
+			(void *)to_free);
 }
 
 static struct kunit_suite *alloc_fake_suite(struct kunit *test,
-- 
2.41.0.rc0.172.g3f132b7071-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v3 3/4] kunit: kmalloc_array: Use kunit_add_action()
  2023-05-25  4:21 [PATCH v3 1/4] kunit: Add kunit_add_action() to defer a call until test exit David Gow
  2023-05-25  4:21 ` [PATCH v3 2/4] kunit: executor_test: Use kunit_add_action() David Gow
@ 2023-05-25  4:21 ` David Gow
  2023-05-25  4:21 ` [PATCH v3 4/4] Documentation: kunit: Add usage notes for kunit_add_action() David Gow
  2 siblings, 0 replies; 4+ messages in thread
From: David Gow @ 2023-05-25  4:21 UTC (permalink / raw)
  To: Matti Vaittinen, Daniel Latypov, Maxime Ripard, Benjamin Berg,
	Brendan Higgins, Stephen Boyd, Shuah Khan, Rae Moar
  Cc: David Gow, Greg Kroah-Hartman, Rafael J . Wysocki,
	Heikki Krogerus, Jonathan Cameron, Sadiya Kazi, Jonathan Corbet,
	linux-doc, kunit-dev, linux-kselftest, linux-kernel,
	Benjamin Berg

The kunit_add_action() function is much simpler and cleaner to use that
the full KUnit resource API for simple things like the
kunit_kmalloc_array() functionality.

Replacing it allows us to get rid of a number of helper functions, and
leaves us with no uses of kunit_alloc_resource(), which has some
usability problems and is going to have its behaviour modified in an
upcoming patch.

Note that we need to use kunit_defer_trigger_all() to implement
kunit_kfree().

Reviewed-by: Benjamin Berg <benjamin.berg@intel.com>
Reviewed-by: Maxime Ripard <maxime@cerno.tech>
Tested-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: David Gow <davidgow@google.com>
---

No changes since v2:
https://lore.kernel.org/linux-kselftest/20230518083849.2631178-3-davidgow@google.com/

Changes since v1:
https://lore.kernel.org/linux-kselftest/20230421084226.2278282-4-davidgow@google.com/
- Use the kunit_action_t typedef.
  - This fixes some spurious checkpatch warnings.

Changes since RFCv2:
https://lore.kernel.org/linux-kselftest/20230331080411.981038-4-davidgow@google.com/
- Update to match changes in the the action API.
- Always allocate the action context with GFP_KERNEL.
- Update documentation to note that this will cause GFP_KERNEL
  allocations, regardless of the gfp argument passed in.

---
 include/kunit/test.h | 10 +++++++--
 lib/kunit/test.c     | 48 +++++++++-----------------------------------
 2 files changed, 17 insertions(+), 41 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 3028a1a3fcad..2f23d6efa505 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -324,8 +324,11 @@ enum kunit_status kunit_suite_has_succeeded(struct kunit_suite *suite);
  * @gfp: flags passed to underlying kmalloc().
  *
  * Just like `kmalloc_array(...)`, except the allocation is managed by the test case
- * and is automatically cleaned up after the test case concludes. See &struct
- * kunit_resource for more information.
+ * and is automatically cleaned up after the test case concludes. See kunit_add_action()
+ * for more information.
+ *
+ * Note that some internal context data is also allocated with GFP_KERNEL,
+ * regardless of the gfp passed in.
  */
 void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp);
 
@@ -336,6 +339,9 @@ void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp);
  * @gfp: flags passed to underlying kmalloc().
  *
  * See kmalloc() and kunit_kmalloc_array() for more information.
+ *
+ * Note that some internal context data is also allocated with GFP_KERNEL,
+ * regardless of the gfp passed in.
  */
 static inline void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp)
 {
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index f5e4ceffd282..d3fb93a23ccc 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -752,58 +752,28 @@ static struct notifier_block kunit_mod_nb = {
 };
 #endif
 
-struct kunit_kmalloc_array_params {
-	size_t n;
-	size_t size;
-	gfp_t gfp;
-};
-
-static int kunit_kmalloc_array_init(struct kunit_resource *res, void *context)
+void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp)
 {
-	struct kunit_kmalloc_array_params *params = context;
+	void *data;
 
-	res->data = kmalloc_array(params->n, params->size, params->gfp);
-	if (!res->data)
-		return -ENOMEM;
+	data = kmalloc_array(n, size, gfp);
 
-	return 0;
-}
+	if (!data)
+		return NULL;
 
-static void kunit_kmalloc_array_free(struct kunit_resource *res)
-{
-	kfree(res->data);
-}
+	if (kunit_add_action_or_reset(test, (kunit_action_t *)kfree, data) != 0)
+		return NULL;
 
-void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp)
-{
-	struct kunit_kmalloc_array_params params = {
-		.size = size,
-		.n = n,
-		.gfp = gfp
-	};
-
-	return kunit_alloc_resource(test,
-				    kunit_kmalloc_array_init,
-				    kunit_kmalloc_array_free,
-				    gfp,
-				    &params);
+	return data;
 }
 EXPORT_SYMBOL_GPL(kunit_kmalloc_array);
 
-static inline bool kunit_kfree_match(struct kunit *test,
-				     struct kunit_resource *res, void *match_data)
-{
-	/* Only match resources allocated with kunit_kmalloc() and friends. */
-	return res->free == kunit_kmalloc_array_free && res->data == match_data;
-}
-
 void kunit_kfree(struct kunit *test, const void *ptr)
 {
 	if (!ptr)
 		return;
 
-	if (kunit_destroy_resource(test, kunit_kfree_match, (void *)ptr))
-		KUNIT_FAIL(test, "kunit_kfree: %px already freed or not allocated by kunit", ptr);
+	kunit_release_action(test, (kunit_action_t *)kfree, (void *)ptr);
 }
 EXPORT_SYMBOL_GPL(kunit_kfree);
 
-- 
2.41.0.rc0.172.g3f132b7071-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v3 4/4] Documentation: kunit: Add usage notes for kunit_add_action()
  2023-05-25  4:21 [PATCH v3 1/4] kunit: Add kunit_add_action() to defer a call until test exit David Gow
  2023-05-25  4:21 ` [PATCH v3 2/4] kunit: executor_test: Use kunit_add_action() David Gow
  2023-05-25  4:21 ` [PATCH v3 3/4] kunit: kmalloc_array: " David Gow
@ 2023-05-25  4:21 ` David Gow
  2 siblings, 0 replies; 4+ messages in thread
From: David Gow @ 2023-05-25  4:21 UTC (permalink / raw)
  To: Matti Vaittinen, Daniel Latypov, Maxime Ripard, Benjamin Berg,
	Brendan Higgins, Stephen Boyd, Shuah Khan, Rae Moar
  Cc: David Gow, Greg Kroah-Hartman, Rafael J . Wysocki,
	Heikki Krogerus, Jonathan Cameron, Sadiya Kazi, Jonathan Corbet,
	linux-doc, kunit-dev, linux-kselftest, linux-kernel

Add some basic documentation for kunit_add_action() and related
deferred action functions.

Reviewed-by: Rae Moar <rmoar@google.com>
Signed-off-by: David Gow <davidgow@google.com>
---

Changes since v2:
https://lore.kernel.org/linux-kselftest/20230518083849.2631178-4-davidgow@google.com/
- Fix a couple of typos (Thanks Bagas, Rae)
- Add Rae's Reviewed-by.

This patch is new in v2.

---
 Documentation/dev-tools/kunit/usage.rst | 51 +++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
index 46957d1cbcbb..c27e1646ecd9 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -615,6 +615,57 @@ For example:
 		KUNIT_ASSERT_STREQ(test, buffer, "");
 	}
 
+Registering Cleanup Actions
+---------------------------
+
+If you need to perform some cleanup beyond simple use of ``kunit_kzalloc``,
+you can register a custom "deferred action", which is a cleanup function
+run when the test exits (whether cleanly, or via a failed assertion).
+
+Actions are simple functions with no return value, and a single ``void*``
+context argument, and fulfill the same role as "cleanup" functions in Python
+and Go tests, "defer" statements in languages which support them, and
+(in some cases) destructors in RAII languages.
+
+These are very useful for unregistering things from global lists, closing
+files or other resources, or freeing resources.
+
+For example:
+
+.. code-block:: C
+
+	static void cleanup_device(void *ctx)
+	{
+		struct device *dev = (struct device *)ctx;
+
+		device_unregister(dev);
+	}
+
+	void example_device_test(struct kunit *test)
+	{
+		struct my_device dev;
+
+		device_register(&dev);
+
+		kunit_add_action(test, &cleanup_device, &dev);
+	}
+
+Note that, for functions like device_unregister which only accept a single
+pointer-sized argument, it's possible to directly cast that function to
+a ``kunit_action_t`` rather than writing a wrapper function, for example:
+
+.. code-block:: C
+
+	kunit_add_action(test, (kunit_action_t *)&device_unregister, &dev);
+
+``kunit_add_action`` can fail if, for example, the system is out of memory.
+You can use ``kunit_add_action_or_reset`` instead which runs the action
+immediately if it cannot be deferred.
+
+If you need more control over when the cleanup function is called, you
+can trigger it early using ``kunit_release_action``, or cancel it entirely
+with ``kunit_remove_action``.
+
 
 Testing Static Functions
 ------------------------
-- 
2.41.0.rc0.172.g3f132b7071-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-05-25  4:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-25  4:21 [PATCH v3 1/4] kunit: Add kunit_add_action() to defer a call until test exit David Gow
2023-05-25  4:21 ` [PATCH v3 2/4] kunit: executor_test: Use kunit_add_action() David Gow
2023-05-25  4:21 ` [PATCH v3 3/4] kunit: kmalloc_array: " David Gow
2023-05-25  4:21 ` [PATCH v3 4/4] Documentation: kunit: Add usage notes for kunit_add_action() David Gow

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).