Background: Currently, a reader looking at kunit/test.h will find the file is quite long, and the first meaty comment is a doc comment about struct kunit_resource.
Most users will not ever use the KUnit resource API directly. They'll use kunit_kmalloc() and friends, or decide it's simpler to do cleanups via labels (it often can be) instead of figuring out how to use the API.
It's also logically separate from everything else in test.h. Removing it from the file doesn't cause any compilation errors (since struct kunit has `struct list_head resources` to store them).
This commit: Let's move it into a kunit/resource.h file and give it a separate page in the docs, kunit/api/resource.rst.
We include resource.h at the bottom of test.h since * don't want to force existing users to add a new include if they use the API * it accesses `lock` inside `struct kunit` in a inline func * so we can't just forward declare, and the alternatives require uninlining the func, adding hepers to lock/unlock, or other more invasive changes.
Now the first big comment in test.h is about kunit_case, which is a lot more relevant to what a new user wants to know.
A side effect of this is git blame won't properly track history by default, users need to run $ git blame -L ,1 -C17 include/kunit/resource.h
Signed-off-by: Daniel Latypov dlatypov@google.com Reviewed-by: David Gow davidgow@google.com Reviewed-by: Brendan Higgins brendanhiggins@google.com --- v1 -> v2: add TODO to make users #include resource.h, drop redundant fwd decl of struct kunit in resource.h --- Documentation/dev-tools/kunit/api/index.rst | 5 + .../dev-tools/kunit/api/resource.rst | 13 + include/kunit/resource.h | 318 ++++++++++++++++++ include/kunit/test.h | 303 +---------------- 4 files changed, 340 insertions(+), 299 deletions(-) create mode 100644 Documentation/dev-tools/kunit/api/resource.rst create mode 100644 include/kunit/resource.h
diff --git a/Documentation/dev-tools/kunit/api/index.rst b/Documentation/dev-tools/kunit/api/index.rst index 3006cadcf44a..45ce04823f9f 100644 --- a/Documentation/dev-tools/kunit/api/index.rst +++ b/Documentation/dev-tools/kunit/api/index.rst @@ -6,6 +6,7 @@ API Reference .. toctree::
test + resource
This section documents the KUnit kernel testing API. It is divided into the following sections: @@ -13,3 +14,7 @@ following sections: Documentation/dev-tools/kunit/api/test.rst
- documents all of the standard testing API + +Documentation/dev-tools/kunit/api/resource.rst + + - documents the KUnit resource API diff --git a/Documentation/dev-tools/kunit/api/resource.rst b/Documentation/dev-tools/kunit/api/resource.rst new file mode 100644 index 000000000000..0a94f831259e --- /dev/null +++ b/Documentation/dev-tools/kunit/api/resource.rst @@ -0,0 +1,13 @@ +.. SPDX-License-Identifier: GPL-2.0 + +============ +Resource API +============ + +This file documents the KUnit resource API. + +Most users won't need to use this API directly, power users can use it to store +state on a per-test basis, register custom cleanup actions, and more. + +.. kernel-doc:: include/kunit/resource.h + :internal: diff --git a/include/kunit/resource.h b/include/kunit/resource.h new file mode 100644 index 000000000000..7ab1fd83972b --- /dev/null +++ b/include/kunit/resource.h @@ -0,0 +1,318 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * KUnit resource API for test managed resources (allocations, etc.). + * + * Copyright (C) 2022, Google LLC. + * Author: Daniel Latypov dlatypov@google.com + */ + +#ifndef _KUNIT_RESOURCE_H +#define _KUNIT_RESOURCE_H + +#include <kunit/test.h> + +#include <linux/kref.h> +#include <linux/list.h> +#include <linux/slab.h> +#include <linux/spinlock.h> + +struct kunit_resource; + +typedef int (*kunit_resource_init_t)(struct kunit_resource *, void *); +typedef void (*kunit_resource_free_t)(struct kunit_resource *); + +/** + * struct kunit_resource - represents a *test managed resource* + * @data: for the user to store arbitrary data. + * @name: optional name + * @free: a user supplied function to free the resource. Populated by + * kunit_resource_alloc(). + * + * Represents a *test managed resource*, a resource which will automatically be + * cleaned up at the end of a test case. + * + * Resources are reference counted so if a resource is retrieved via + * kunit_alloc_and_get_resource() or kunit_find_resource(), we need + * to call kunit_put_resource() to reduce the resource reference count + * when finished with it. Note that kunit_alloc_resource() does not require a + * kunit_resource_put() because it does not retrieve the resource itself. + * + * Example: + * + * .. code-block:: c + * + * struct kunit_kmalloc_params { + * size_t size; + * gfp_t gfp; + * }; + * + * static int kunit_kmalloc_init(struct kunit_resource *res, void *context) + * { + * struct kunit_kmalloc_params *params = context; + * res->data = kmalloc(params->size, params->gfp); + * + * if (!res->data) + * return -ENOMEM; + * + * return 0; + * } + * + * static void kunit_kmalloc_free(struct kunit_resource *res) + * { + * kfree(res->data); + * } + * + * void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp) + * { + * struct kunit_kmalloc_params params; + * + * params.size = size; + * params.gfp = gfp; + * + * return kunit_alloc_resource(test, kunit_kmalloc_init, + * kunit_kmalloc_free, ¶ms); + * } + * + * Resources can also be named, with lookup/removal done on a name + * basis also. kunit_add_named_resource(), kunit_find_named_resource() + * and kunit_destroy_named_resource(). Resource names must be + * unique within the test instance. + */ +struct kunit_resource { + void *data; + const char *name; + kunit_resource_free_t free; + + /* private: internal use only. */ + struct kref refcount; + struct list_head node; +}; + +/* + * Like kunit_alloc_resource() below, but returns the struct kunit_resource + * object that contains the allocation. This is mostly for testing purposes. + */ +struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test, + kunit_resource_init_t init, + kunit_resource_free_t free, + gfp_t internal_gfp, + void *context); + +/** + * kunit_get_resource() - Hold resource for use. Should not need to be used + * by most users as we automatically get resources + * retrieved by kunit_find_resource*(). + * @res: resource + */ +static inline void kunit_get_resource(struct kunit_resource *res) +{ + kref_get(&res->refcount); +} + +/* + * Called when refcount reaches zero via kunit_put_resource(); + * should not be called directly. + */ +static inline void kunit_release_resource(struct kref *kref) +{ + struct kunit_resource *res = container_of(kref, struct kunit_resource, + refcount); + + /* If free function is defined, resource was dynamically allocated. */ + if (res->free) { + res->free(res); + kfree(res); + } +} + +/** + * kunit_put_resource() - When caller is done with retrieved resource, + * kunit_put_resource() should be called to drop + * reference count. The resource list maintains + * a reference count on resources, so if no users + * are utilizing a resource and it is removed from + * the resource list, it will be freed via the + * associated free function (if any). Only + * needs to be used if we alloc_and_get() or + * find() resource. + * @res: resource + */ +static inline void kunit_put_resource(struct kunit_resource *res) +{ + kref_put(&res->refcount, kunit_release_resource); +} + +/** + * kunit_add_resource() - Add a *test managed resource*. + * @test: The test context object. + * @init: a user-supplied function to initialize the result (if needed). If + * none is supplied, the resource data value is simply set to @data. + * If an init function is supplied, @data is passed to it instead. + * @free: a user-supplied function to free the resource (if needed). + * @res: The resource. + * @data: value to pass to init function or set in resource data field. + */ +int kunit_add_resource(struct kunit *test, + kunit_resource_init_t init, + kunit_resource_free_t free, + struct kunit_resource *res, + void *data); + +/** + * kunit_add_named_resource() - Add a named *test managed resource*. + * @test: The test context object. + * @init: a user-supplied function to initialize the resource data, if needed. + * @free: a user-supplied function to free the resource data, if needed. + * @res: The resource. + * @name: name to be set for resource. + * @data: value to pass to init function or set in resource data field. + */ +int kunit_add_named_resource(struct kunit *test, + kunit_resource_init_t init, + kunit_resource_free_t free, + struct kunit_resource *res, + const char *name, + void *data); + +/** + * kunit_alloc_resource() - Allocates a *test managed resource*. + * @test: The test context object. + * @init: a user supplied function to initialize the resource. + * @free: a user supplied function to free the resource. + * @internal_gfp: gfp to use for internal allocations, if unsure, use GFP_KERNEL + * @context: for the user to pass in arbitrary data to the init function. + * + * Allocates a *test managed resource*, a resource which will automatically be + * cleaned up at the end of a test case. See &struct kunit_resource for an + * example. + * + * Note: KUnit needs to allocate memory for a kunit_resource object. You must + * specify an @internal_gfp that is compatible with the use context of your + * resource. + */ +static inline void *kunit_alloc_resource(struct kunit *test, + kunit_resource_init_t init, + kunit_resource_free_t free, + gfp_t internal_gfp, + void *context) +{ + struct kunit_resource *res; + + res = kzalloc(sizeof(*res), internal_gfp); + if (!res) + return NULL; + + if (!kunit_add_resource(test, init, free, res, context)) + return res->data; + + return NULL; +} + +typedef bool (*kunit_resource_match_t)(struct kunit *test, + struct kunit_resource *res, + void *match_data); + +/** + * kunit_resource_instance_match() - Match a resource with the same instance. + * @test: Test case to which the resource belongs. + * @res: The resource. + * @match_data: The resource pointer to match against. + * + * An instance of kunit_resource_match_t that matches a resource whose + * allocation matches @match_data. + */ +static inline bool kunit_resource_instance_match(struct kunit *test, + struct kunit_resource *res, + void *match_data) +{ + return res->data == match_data; +} + +/** + * kunit_resource_name_match() - Match a resource with the same name. + * @test: Test case to which the resource belongs. + * @res: The resource. + * @match_name: The name to match against. + */ +static inline bool kunit_resource_name_match(struct kunit *test, + struct kunit_resource *res, + void *match_name) +{ + return res->name && strcmp(res->name, match_name) == 0; +} + +/** + * kunit_find_resource() - Find a resource using match function/data. + * @test: Test case to which the resource belongs. + * @match: match function to be applied to resources/match data. + * @match_data: data to be used in matching. + */ +static inline struct kunit_resource * +kunit_find_resource(struct kunit *test, + kunit_resource_match_t match, + void *match_data) +{ + struct kunit_resource *res, *found = NULL; + unsigned long flags; + + spin_lock_irqsave(&test->lock, flags); + + list_for_each_entry_reverse(res, &test->resources, node) { + if (match(test, res, (void *)match_data)) { + found = res; + kunit_get_resource(found); + break; + } + } + + spin_unlock_irqrestore(&test->lock, flags); + + return found; +} + +/** + * kunit_find_named_resource() - Find a resource using match name. + * @test: Test case to which the resource belongs. + * @name: match name. + */ +static inline struct kunit_resource * +kunit_find_named_resource(struct kunit *test, + const char *name) +{ + return kunit_find_resource(test, kunit_resource_name_match, + (void *)name); +} + +/** + * kunit_destroy_resource() - Find a kunit_resource and destroy it. + * @test: Test case to which the resource belongs. + * @match: Match function. Returns whether a given resource matches @match_data. + * @match_data: Data passed into @match. + * + * RETURNS: + * 0 if kunit_resource is found and freed, -ENOENT if not found. + */ +int kunit_destroy_resource(struct kunit *test, + kunit_resource_match_t match, + void *match_data); + +static inline int kunit_destroy_named_resource(struct kunit *test, + const char *name) +{ + return kunit_destroy_resource(test, kunit_resource_name_match, + (void *)name); +} + +/** + * kunit_remove_resource() - remove resource from resource list associated with + * test. + * @test: The test context object. + * @res: The resource to be removed. + * + * Note that the resource will not be immediately freed since it is likely + * the caller has a reference to it via alloc_and_get() or find(); + * in this case a final call to kunit_put_resource() is required. + */ +void kunit_remove_resource(struct kunit *test, struct kunit_resource *res); + +#endif /* _KUNIT_RESOURCE_H */ diff --git a/include/kunit/test.h b/include/kunit/test.h index 00b9ff7783ab..d04f9e2fd5e7 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -27,78 +27,6 @@
#include <asm/rwonce.h>
-struct kunit_resource; - -typedef int (*kunit_resource_init_t)(struct kunit_resource *, void *); -typedef void (*kunit_resource_free_t)(struct kunit_resource *); - -/** - * struct kunit_resource - represents a *test managed resource* - * @data: for the user to store arbitrary data. - * @name: optional name - * @free: a user supplied function to free the resource. Populated by - * kunit_resource_alloc(). - * - * Represents a *test managed resource*, a resource which will automatically be - * cleaned up at the end of a test case. - * - * Resources are reference counted so if a resource is retrieved via - * kunit_alloc_and_get_resource() or kunit_find_resource(), we need - * to call kunit_put_resource() to reduce the resource reference count - * when finished with it. Note that kunit_alloc_resource() does not require a - * kunit_resource_put() because it does not retrieve the resource itself. - * - * Example: - * - * .. code-block:: c - * - * struct kunit_kmalloc_params { - * size_t size; - * gfp_t gfp; - * }; - * - * static int kunit_kmalloc_init(struct kunit_resource *res, void *context) - * { - * struct kunit_kmalloc_params *params = context; - * res->data = kmalloc(params->size, params->gfp); - * - * if (!res->data) - * return -ENOMEM; - * - * return 0; - * } - * - * static void kunit_kmalloc_free(struct kunit_resource *res) - * { - * kfree(res->data); - * } - * - * void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp) - * { - * struct kunit_kmalloc_params params; - * - * params.size = size; - * params.gfp = gfp; - * - * return kunit_alloc_resource(test, kunit_kmalloc_init, - * kunit_kmalloc_free, ¶ms); - * } - * - * Resources can also be named, with lookup/removal done on a name - * basis also. kunit_add_named_resource(), kunit_find_named_resource() - * and kunit_destroy_named_resource(). Resource names must be - * unique within the test instance. - */ -struct kunit_resource { - void *data; - const char *name; - kunit_resource_free_t free; - - /* private: internal use only. */ - struct kref refcount; - struct list_head node; -}; - struct kunit;
/* Size of log associated with test. */ @@ -385,233 +313,6 @@ static inline int kunit_run_all_tests(void)
enum kunit_status kunit_suite_has_succeeded(struct kunit_suite *suite);
-/* - * Like kunit_alloc_resource() below, but returns the struct kunit_resource - * object that contains the allocation. This is mostly for testing purposes. - */ -struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test, - kunit_resource_init_t init, - kunit_resource_free_t free, - gfp_t internal_gfp, - void *context); - -/** - * kunit_get_resource() - Hold resource for use. Should not need to be used - * by most users as we automatically get resources - * retrieved by kunit_find_resource*(). - * @res: resource - */ -static inline void kunit_get_resource(struct kunit_resource *res) -{ - kref_get(&res->refcount); -} - -/* - * Called when refcount reaches zero via kunit_put_resources(); - * should not be called directly. - */ -static inline void kunit_release_resource(struct kref *kref) -{ - struct kunit_resource *res = container_of(kref, struct kunit_resource, - refcount); - - /* If free function is defined, resource was dynamically allocated. */ - if (res->free) { - res->free(res); - kfree(res); - } -} - -/** - * kunit_put_resource() - When caller is done with retrieved resource, - * kunit_put_resource() should be called to drop - * reference count. The resource list maintains - * a reference count on resources, so if no users - * are utilizing a resource and it is removed from - * the resource list, it will be freed via the - * associated free function (if any). Only - * needs to be used if we alloc_and_get() or - * find() resource. - * @res: resource - */ -static inline void kunit_put_resource(struct kunit_resource *res) -{ - kref_put(&res->refcount, kunit_release_resource); -} - -/** - * kunit_add_resource() - Add a *test managed resource*. - * @test: The test context object. - * @init: a user-supplied function to initialize the result (if needed). If - * none is supplied, the resource data value is simply set to @data. - * If an init function is supplied, @data is passed to it instead. - * @free: a user-supplied function to free the resource (if needed). - * @res: The resource. - * @data: value to pass to init function or set in resource data field. - */ -int kunit_add_resource(struct kunit *test, - kunit_resource_init_t init, - kunit_resource_free_t free, - struct kunit_resource *res, - void *data); - -/** - * kunit_add_named_resource() - Add a named *test managed resource*. - * @test: The test context object. - * @init: a user-supplied function to initialize the resource data, if needed. - * @free: a user-supplied function to free the resource data, if needed. - * @res: The resource. - * @name: name to be set for resource. - * @data: value to pass to init function or set in resource data field. - */ -int kunit_add_named_resource(struct kunit *test, - kunit_resource_init_t init, - kunit_resource_free_t free, - struct kunit_resource *res, - const char *name, - void *data); - -/** - * kunit_alloc_resource() - Allocates a *test managed resource*. - * @test: The test context object. - * @init: a user supplied function to initialize the resource. - * @free: a user supplied function to free the resource. - * @internal_gfp: gfp to use for internal allocations, if unsure, use GFP_KERNEL - * @context: for the user to pass in arbitrary data to the init function. - * - * Allocates a *test managed resource*, a resource which will automatically be - * cleaned up at the end of a test case. See &struct kunit_resource for an - * example. - * - * Note: KUnit needs to allocate memory for a kunit_resource object. You must - * specify an @internal_gfp that is compatible with the use context of your - * resource. - */ -static inline void *kunit_alloc_resource(struct kunit *test, - kunit_resource_init_t init, - kunit_resource_free_t free, - gfp_t internal_gfp, - void *context) -{ - struct kunit_resource *res; - - res = kzalloc(sizeof(*res), internal_gfp); - if (!res) - return NULL; - - if (!kunit_add_resource(test, init, free, res, context)) - return res->data; - - return NULL; -} - -typedef bool (*kunit_resource_match_t)(struct kunit *test, - struct kunit_resource *res, - void *match_data); - -/** - * kunit_resource_instance_match() - Match a resource with the same instance. - * @test: Test case to which the resource belongs. - * @res: The resource. - * @match_data: The resource pointer to match against. - * - * An instance of kunit_resource_match_t that matches a resource whose - * allocation matches @match_data. - */ -static inline bool kunit_resource_instance_match(struct kunit *test, - struct kunit_resource *res, - void *match_data) -{ - return res->data == match_data; -} - -/** - * kunit_resource_name_match() - Match a resource with the same name. - * @test: Test case to which the resource belongs. - * @res: The resource. - * @match_name: The name to match against. - */ -static inline bool kunit_resource_name_match(struct kunit *test, - struct kunit_resource *res, - void *match_name) -{ - return res->name && strcmp(res->name, match_name) == 0; -} - -/** - * kunit_find_resource() - Find a resource using match function/data. - * @test: Test case to which the resource belongs. - * @match: match function to be applied to resources/match data. - * @match_data: data to be used in matching. - */ -static inline struct kunit_resource * -kunit_find_resource(struct kunit *test, - kunit_resource_match_t match, - void *match_data) -{ - struct kunit_resource *res, *found = NULL; - unsigned long flags; - - spin_lock_irqsave(&test->lock, flags); - - list_for_each_entry_reverse(res, &test->resources, node) { - if (match(test, res, (void *)match_data)) { - found = res; - kunit_get_resource(found); - break; - } - } - - spin_unlock_irqrestore(&test->lock, flags); - - return found; -} - -/** - * kunit_find_named_resource() - Find a resource using match name. - * @test: Test case to which the resource belongs. - * @name: match name. - */ -static inline struct kunit_resource * -kunit_find_named_resource(struct kunit *test, - const char *name) -{ - return kunit_find_resource(test, kunit_resource_name_match, - (void *)name); -} - -/** - * kunit_destroy_resource() - Find a kunit_resource and destroy it. - * @test: Test case to which the resource belongs. - * @match: Match function. Returns whether a given resource matches @match_data. - * @match_data: Data passed into @match. - * - * RETURNS: - * 0 if kunit_resource is found and freed, -ENOENT if not found. - */ -int kunit_destroy_resource(struct kunit *test, - kunit_resource_match_t match, - void *match_data); - -static inline int kunit_destroy_named_resource(struct kunit *test, - const char *name) -{ - return kunit_destroy_resource(test, kunit_resource_name_match, - (void *)name); -} - -/** - * kunit_remove_resource() - remove resource from resource list associated with - * test. - * @test: The test context object. - * @res: The resource to be removed. - * - * Note that the resource will not be immediately freed since it is likely - * the caller has a reference to it via alloc_and_get() or find(); - * in this case a final call to kunit_put_resource() is required. - */ -void kunit_remove_resource(struct kunit *test, struct kunit_resource *res); - /** * kunit_kmalloc_array() - Like kmalloc_array() except the allocation is *test managed*. * @test: The test context object. @@ -1526,4 +1227,8 @@ do { \ return NULL; \ }
+// TODO(dlatypov@google.com): consider eventually migrating users to explicitly +// include resource.h themselves if they need it. +#include <kunit/resource.h> + #endif /* _KUNIT_TEST_H */
base-commit: b14ffae378aa1db993e62b01392e70d1e585fb23
We've split out the declarations from include/kunit/test.h into resource.h. This patch splits out the definitions as well for consistency.
A side effect of this is git blame won't properly track history by default, users need to run $ git blame -L ,1 -C13 lib/kunit/resource.c
Signed-off-by: Daniel Latypov dlatypov@google.com --- lib/kunit/Makefile | 1 + lib/kunit/resource.c | 126 +++++++++++++++++++++++++++++++++++++++++++ lib/kunit/test.c | 116 +-------------------------------------- 3 files changed, 128 insertions(+), 115 deletions(-) create mode 100644 lib/kunit/resource.c
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile index c49f4ffb6273..29aff6562b42 100644 --- a/lib/kunit/Makefile +++ b/lib/kunit/Makefile @@ -1,6 +1,7 @@ obj-$(CONFIG_KUNIT) += kunit.o
kunit-objs += test.o \ + resource.o \ string-stream.o \ assert.o \ try-catch.o \ diff --git a/lib/kunit/resource.c b/lib/kunit/resource.c new file mode 100644 index 000000000000..b8bced246217 --- /dev/null +++ b/lib/kunit/resource.c @@ -0,0 +1,126 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * KUnit resource API for test managed resources (allocations, etc.). + * + * Copyright (C) 2022, Google LLC. + * Author: Daniel Latypov dlatypov@google.com + */ + +#include <kunit/resource.h> +#include <kunit/test.h> +#include <linux/kref.h> + +/* + * Used for static resources and when a kunit_resource * has been created by + * kunit_alloc_resource(). When an init function is supplied, @data is passed + * into the init function; otherwise, we simply set the resource data field to + * the data value passed in. + */ +int kunit_add_resource(struct kunit *test, + kunit_resource_init_t init, + kunit_resource_free_t free, + struct kunit_resource *res, + void *data) +{ + int ret = 0; + unsigned long flags; + + res->free = free; + kref_init(&res->refcount); + + if (init) { + ret = init(res, data); + if (ret) + return ret; + } else { + res->data = data; + } + + spin_lock_irqsave(&test->lock, flags); + list_add_tail(&res->node, &test->resources); + /* refcount for list is established by kref_init() */ + spin_unlock_irqrestore(&test->lock, flags); + + return ret; +} +EXPORT_SYMBOL_GPL(kunit_add_resource); + +int kunit_add_named_resource(struct kunit *test, + kunit_resource_init_t init, + kunit_resource_free_t free, + struct kunit_resource *res, + const char *name, + void *data) +{ + struct kunit_resource *existing; + + if (!name) + return -EINVAL; + + existing = kunit_find_named_resource(test, name); + if (existing) { + kunit_put_resource(existing); + return -EEXIST; + } + + res->name = name; + + return kunit_add_resource(test, init, free, res, data); +} +EXPORT_SYMBOL_GPL(kunit_add_named_resource); + +struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test, + kunit_resource_init_t init, + kunit_resource_free_t free, + gfp_t internal_gfp, + void *data) +{ + struct kunit_resource *res; + int ret; + + res = kzalloc(sizeof(*res), internal_gfp); + if (!res) + return NULL; + + ret = kunit_add_resource(test, init, free, res, data); + if (!ret) { + /* + * bump refcount for get; kunit_resource_put() should be called + * when done. + */ + kunit_get_resource(res); + return res; + } + return NULL; +} +EXPORT_SYMBOL_GPL(kunit_alloc_and_get_resource); + +void kunit_remove_resource(struct kunit *test, struct kunit_resource *res) +{ + unsigned long flags; + + spin_lock_irqsave(&test->lock, flags); + list_del(&res->node); + spin_unlock_irqrestore(&test->lock, flags); + kunit_put_resource(res); +} +EXPORT_SYMBOL_GPL(kunit_remove_resource); + +int kunit_destroy_resource(struct kunit *test, kunit_resource_match_t match, + void *match_data) +{ + struct kunit_resource *res = kunit_find_resource(test, match, + match_data); + + if (!res) + return -ENOENT; + + kunit_remove_resource(test, res); + + /* We have a reference also via _find(); drop it. */ + kunit_put_resource(res); + + return 0; +} +EXPORT_SYMBOL_GPL(kunit_destroy_resource); + diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 3bca3bf5c15b..0f66c13d126e 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -6,10 +6,10 @@ * Author: Brendan Higgins brendanhiggins@google.com */
+#include <kunit/resource.h> #include <kunit/test.h> #include <kunit/test-bug.h> #include <linux/kernel.h> -#include <linux/kref.h> #include <linux/moduleparam.h> #include <linux/sched/debug.h> #include <linux/sched.h> @@ -592,120 +592,6 @@ void __kunit_test_suites_exit(struct kunit_suite **suites) } EXPORT_SYMBOL_GPL(__kunit_test_suites_exit);
-/* - * Used for static resources and when a kunit_resource * has been created by - * kunit_alloc_resource(). When an init function is supplied, @data is passed - * into the init function; otherwise, we simply set the resource data field to - * the data value passed in. - */ -int kunit_add_resource(struct kunit *test, - kunit_resource_init_t init, - kunit_resource_free_t free, - struct kunit_resource *res, - void *data) -{ - int ret = 0; - unsigned long flags; - - res->free = free; - kref_init(&res->refcount); - - if (init) { - ret = init(res, data); - if (ret) - return ret; - } else { - res->data = data; - } - - spin_lock_irqsave(&test->lock, flags); - list_add_tail(&res->node, &test->resources); - /* refcount for list is established by kref_init() */ - spin_unlock_irqrestore(&test->lock, flags); - - return ret; -} -EXPORT_SYMBOL_GPL(kunit_add_resource); - -int kunit_add_named_resource(struct kunit *test, - kunit_resource_init_t init, - kunit_resource_free_t free, - struct kunit_resource *res, - const char *name, - void *data) -{ - struct kunit_resource *existing; - - if (!name) - return -EINVAL; - - existing = kunit_find_named_resource(test, name); - if (existing) { - kunit_put_resource(existing); - return -EEXIST; - } - - res->name = name; - - return kunit_add_resource(test, init, free, res, data); -} -EXPORT_SYMBOL_GPL(kunit_add_named_resource); - -struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test, - kunit_resource_init_t init, - kunit_resource_free_t free, - gfp_t internal_gfp, - void *data) -{ - struct kunit_resource *res; - int ret; - - res = kzalloc(sizeof(*res), internal_gfp); - if (!res) - return NULL; - - ret = kunit_add_resource(test, init, free, res, data); - if (!ret) { - /* - * bump refcount for get; kunit_resource_put() should be called - * when done. - */ - kunit_get_resource(res); - return res; - } - return NULL; -} -EXPORT_SYMBOL_GPL(kunit_alloc_and_get_resource); - -void kunit_remove_resource(struct kunit *test, struct kunit_resource *res) -{ - unsigned long flags; - - spin_lock_irqsave(&test->lock, flags); - list_del(&res->node); - spin_unlock_irqrestore(&test->lock, flags); - kunit_put_resource(res); -} -EXPORT_SYMBOL_GPL(kunit_remove_resource); - -int kunit_destroy_resource(struct kunit *test, kunit_resource_match_t match, - void *match_data) -{ - struct kunit_resource *res = kunit_find_resource(test, match, - match_data); - - if (!res) - return -ENOENT; - - kunit_remove_resource(test, res); - - /* We have a reference also via _find(); drop it. */ - kunit_put_resource(res); - - return 0; -} -EXPORT_SYMBOL_GPL(kunit_destroy_resource); - struct kunit_kmalloc_array_params { size_t n; size_t size;
On Sat, Mar 26, 2022 at 8:20 AM Daniel Latypov dlatypov@google.com wrote:
We've split out the declarations from include/kunit/test.h into resource.h. This patch splits out the definitions as well for consistency.
A side effect of this is git blame won't properly track history by default, users need to run $ git blame -L ,1 -C13 lib/kunit/resource.c
Signed-off-by: Daniel Latypov dlatypov@google.com
Looks good and works fine here. I'm going to try to rebase most of the other resource system stuff I'm working on on top of these (which will likely end up moving a bunch of code _again_, but is probably the least terrible of all the available options).
One nitpick (newline at end of file) below, otherwise this is good.
Reviewed-by: David Gow davidgow@google.com
Cheers, -- David
lib/kunit/Makefile | 1 + lib/kunit/resource.c | 126 +++++++++++++++++++++++++++++++++++++++++++ lib/kunit/test.c | 116 +-------------------------------------- 3 files changed, 128 insertions(+), 115 deletions(-) create mode 100644 lib/kunit/resource.c
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile index c49f4ffb6273..29aff6562b42 100644 --- a/lib/kunit/Makefile +++ b/lib/kunit/Makefile @@ -1,6 +1,7 @@ obj-$(CONFIG_KUNIT) += kunit.o
kunit-objs += test.o \
resource.o \ string-stream.o \ assert.o \ try-catch.o \diff --git a/lib/kunit/resource.c b/lib/kunit/resource.c new file mode 100644 index 000000000000..b8bced246217 --- /dev/null +++ b/lib/kunit/resource.c @@ -0,0 +1,126 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- KUnit resource API for test managed resources (allocations, etc.).
- Copyright (C) 2022, Google LLC.
- Author: Daniel Latypov dlatypov@google.com
- */
+#include <kunit/resource.h> +#include <kunit/test.h> +#include <linux/kref.h>
+/*
- Used for static resources and when a kunit_resource * has been created by
- kunit_alloc_resource(). When an init function is supplied, @data is passed
- into the init function; otherwise, we simply set the resource data field to
- the data value passed in.
- */
+int kunit_add_resource(struct kunit *test,
kunit_resource_init_t init,kunit_resource_free_t free,struct kunit_resource *res,void *data)+{
int ret = 0;unsigned long flags;res->free = free;kref_init(&res->refcount);if (init) {ret = init(res, data);if (ret)return ret;} else {res->data = data;}spin_lock_irqsave(&test->lock, flags);list_add_tail(&res->node, &test->resources);/* refcount for list is established by kref_init() */spin_unlock_irqrestore(&test->lock, flags);return ret;+} +EXPORT_SYMBOL_GPL(kunit_add_resource);
+int kunit_add_named_resource(struct kunit *test,
kunit_resource_init_t init,kunit_resource_free_t free,struct kunit_resource *res,const char *name,void *data)+{
struct kunit_resource *existing;if (!name)return -EINVAL;existing = kunit_find_named_resource(test, name);if (existing) {kunit_put_resource(existing);return -EEXIST;}res->name = name;return kunit_add_resource(test, init, free, res, data);+} +EXPORT_SYMBOL_GPL(kunit_add_named_resource);
+struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test,
kunit_resource_init_t init,kunit_resource_free_t free,gfp_t internal_gfp,void *data)+{
struct kunit_resource *res;int ret;res = kzalloc(sizeof(*res), internal_gfp);if (!res)return NULL;ret = kunit_add_resource(test, init, free, res, data);if (!ret) {/** bump refcount for get; kunit_resource_put() should be called* when done.*/kunit_get_resource(res);return res;}return NULL;+} +EXPORT_SYMBOL_GPL(kunit_alloc_and_get_resource);
+void kunit_remove_resource(struct kunit *test, struct kunit_resource *res) +{
unsigned long flags;spin_lock_irqsave(&test->lock, flags);list_del(&res->node);spin_unlock_irqrestore(&test->lock, flags);kunit_put_resource(res);+} +EXPORT_SYMBOL_GPL(kunit_remove_resource);
+int kunit_destroy_resource(struct kunit *test, kunit_resource_match_t match,
void *match_data)+{
struct kunit_resource *res = kunit_find_resource(test, match,match_data);if (!res)return -ENOENT;kunit_remove_resource(test, res);/* We have a reference also via _find(); drop it. */kunit_put_resource(res);return 0;+} +EXPORT_SYMBOL_GPL(kunit_destroy_resource);
.git/rebase-apply/patch:151: new blank line at EOF.
+
diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 3bca3bf5c15b..0f66c13d126e 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -6,10 +6,10 @@
- Author: Brendan Higgins brendanhiggins@google.com
*/
+#include <kunit/resource.h> #include <kunit/test.h> #include <kunit/test-bug.h> #include <linux/kernel.h> -#include <linux/kref.h> #include <linux/moduleparam.h> #include <linux/sched/debug.h> #include <linux/sched.h> @@ -592,120 +592,6 @@ void __kunit_test_suites_exit(struct kunit_suite **suites) } EXPORT_SYMBOL_GPL(__kunit_test_suites_exit);
-/*
- Used for static resources and when a kunit_resource * has been created by
- kunit_alloc_resource(). When an init function is supplied, @data is passed
- into the init function; otherwise, we simply set the resource data field to
- the data value passed in.
- */
-int kunit_add_resource(struct kunit *test,
kunit_resource_init_t init,kunit_resource_free_t free,struct kunit_resource *res,void *data)-{
int ret = 0;unsigned long flags;res->free = free;kref_init(&res->refcount);if (init) {ret = init(res, data);if (ret)return ret;} else {res->data = data;}spin_lock_irqsave(&test->lock, flags);list_add_tail(&res->node, &test->resources);/* refcount for list is established by kref_init() */spin_unlock_irqrestore(&test->lock, flags);return ret;-} -EXPORT_SYMBOL_GPL(kunit_add_resource);
-int kunit_add_named_resource(struct kunit *test,
kunit_resource_init_t init,kunit_resource_free_t free,struct kunit_resource *res,const char *name,void *data)-{
struct kunit_resource *existing;if (!name)return -EINVAL;existing = kunit_find_named_resource(test, name);if (existing) {kunit_put_resource(existing);return -EEXIST;}res->name = name;return kunit_add_resource(test, init, free, res, data);-} -EXPORT_SYMBOL_GPL(kunit_add_named_resource);
-struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test,
kunit_resource_init_t init,kunit_resource_free_t free,gfp_t internal_gfp,void *data)-{
struct kunit_resource *res;int ret;res = kzalloc(sizeof(*res), internal_gfp);if (!res)return NULL;ret = kunit_add_resource(test, init, free, res, data);if (!ret) {/** bump refcount for get; kunit_resource_put() should be called* when done.*/kunit_get_resource(res);return res;}return NULL;-} -EXPORT_SYMBOL_GPL(kunit_alloc_and_get_resource);
-void kunit_remove_resource(struct kunit *test, struct kunit_resource *res) -{
unsigned long flags;spin_lock_irqsave(&test->lock, flags);list_del(&res->node);spin_unlock_irqrestore(&test->lock, flags);kunit_put_resource(res);-} -EXPORT_SYMBOL_GPL(kunit_remove_resource);
-int kunit_destroy_resource(struct kunit *test, kunit_resource_match_t match,
void *match_data)-{
struct kunit_resource *res = kunit_find_resource(test, match,match_data);if (!res)return -ENOENT;kunit_remove_resource(test, res);/* We have a reference also via _find(); drop it. */kunit_put_resource(res);return 0;-} -EXPORT_SYMBOL_GPL(kunit_destroy_resource);
struct kunit_kmalloc_array_params { size_t n; size_t size; -- 2.35.1.1021.g381101b075-goog
On Fri, Mar 25, 2022 at 11:27 PM David Gow davidgow@google.com wrote:
On Sat, Mar 26, 2022 at 8:20 AM Daniel Latypov dlatypov@google.com wrote:
We've split out the declarations from include/kunit/test.h into resource.h. This patch splits out the definitions as well for consistency.
A side effect of this is git blame won't properly track history by default, users need to run $ git blame -L ,1 -C13 lib/kunit/resource.c
Signed-off-by: Daniel Latypov dlatypov@google.com
Looks good and works fine here. I'm going to try to rebase most of the other resource system stuff I'm working on on top of these (which will likely end up moving a bunch of code _again_, but is probably the least terrible of all the available options).
One nitpick (newline at end of file) below, otherwise this is good.
Huh, I had thought checkpatch --strict would catch this. I guess not.
Fixing this and sending out a v3.
Reviewed-by: David Gow davidgow@google.com
On Fri, Mar 25, 2022 at 8:20 PM Daniel Latypov dlatypov@google.com wrote:
We've split out the declarations from include/kunit/test.h into resource.h. This patch splits out the definitions as well for consistency.
A side effect of this is git blame won't properly track history by default, users need to run $ git blame -L ,1 -C13 lib/kunit/resource.c
Signed-off-by: Daniel Latypov dlatypov@google.com
Reviewed-by: Brendan Higgins brendanhiggins@google.com
linux-kselftest-mirror@lists.linaro.org
-
Brendan Higgins -
Daniel Latypov -
David Gow