From d4cdd146d0db900b2eb6c2d28cba719b3bf0a928 Mon Sep 17 00:00:00 2001 From: Alan Maguire Date: Fri, 29 May 2020 22:46:20 +0100 Subject: kunit: generalize kunit_resource API beyond allocated resources In its original form, the kunit resources API - consisting the struct kunit_resource and associated functions - was focused on adding allocated resources during test operation that would be automatically cleaned up on test completion. The recent RFC patch proposing converting KASAN tests to KUnit [1] showed another potential model - where outside of test context, but with a pointer to the test state, we wish to access/update test-related data, but expressly want to avoid allocations. It turns out we can generalize the kunit_resource to support static resources where the struct kunit_resource * is passed in and initialized for us. As part of this work, we also change the "allocation" field to the more general "data" name, as instead of associating an allocation, we can associate a pointer to static data. Static data is distinguished by a NULL free functions. A test is added to cover using kunit_add_resource() with a static resource and data. Finally we also make use of the kernel's krefcount interfaces to manage reference counting of KUnit resources. The motivation for this is simple; if we have kernel threads accessing and using resources (say via kunit_find_resource()) we need to ensure we do not remove said resources (or indeed free them if they were dynamically allocated) until the reference count reaches zero. A new function - kunit_put_resource() - is added to handle this, and it should be called after a thread using kunit_find_resource() is finished with the retrieved resource. We ensure that the functions needed to look up, use and drop reference count are "static inline"-defined so that they can be used by builtin code as well as modules in the case that KUnit is built as a module. A cosmetic change here also; I've tried moving to kunit_[action]_resource() as the format of function names for consistency and readability. [1] https://lkml.org/lkml/2020/2/26/1286 Signed-off-by: Alan Maguire Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan --- include/kunit/test.h | 156 +++++++++++++++++++++++++++++++++++++--------- lib/kunit/kunit-test.c | 74 ++++++++++++++++------ lib/kunit/string-stream.c | 14 ++--- lib/kunit/test.c | 153 ++++++++++++++++++++++++--------------------- 4 files changed, 268 insertions(+), 129 deletions(-) diff --git a/include/kunit/test.h b/include/kunit/test.h index 47e61e1d5337..f9b914ebfd75 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -15,6 +15,7 @@ #include #include #include +#include struct kunit_resource; @@ -23,13 +24,19 @@ typedef void (*kunit_resource_free_t)(struct kunit_resource *); /** * struct kunit_resource - represents a *test managed resource* - * @allocation: for the user to store arbitrary data. + * @data: for the user to store arbitrary data. * @free: a user supplied function to free the resource. Populated by - * kunit_alloc_resource(). + * 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 @@ -42,9 +49,9 @@ typedef void (*kunit_resource_free_t)(struct kunit_resource *); * static int kunit_kmalloc_init(struct kunit_resource *res, void *context) * { * struct kunit_kmalloc_params *params = context; - * res->allocation = kmalloc(params->size, params->gfp); + * res->data = kmalloc(params->size, params->gfp); * - * if (!res->allocation) + * if (!res->data) * return -ENOMEM; * * return 0; @@ -52,30 +59,26 @@ typedef void (*kunit_resource_free_t)(struct kunit_resource *); * * static void kunit_kmalloc_free(struct kunit_resource *res) * { - * kfree(res->allocation); + * kfree(res->data); * } * * void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp) * { * struct kunit_kmalloc_params params; - * struct kunit_resource *res; * * params.size = size; * params.gfp = gfp; * - * res = kunit_alloc_resource(test, kunit_kmalloc_init, + * return kunit_alloc_resource(test, kunit_kmalloc_init, * kunit_kmalloc_free, ¶ms); - * if (res) - * return res->allocation; - * - * return NULL; * } */ struct kunit_resource { - void *allocation; - kunit_resource_free_t free; + void *data; /* private: internal use only. */ + kunit_resource_free_t free; + struct kref refcount; struct list_head node; }; @@ -283,6 +286,64 @@ struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test, 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). + * @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_alloc_resource() - Allocates a *test managed resource*. * @test: The test context object. @@ -295,7 +356,7 @@ struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test, * cleaned up at the end of a test case. See &struct kunit_resource for an * example. * - * NOTE: KUnit needs to allocate memory for each kunit_resource object. You must + * 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. */ @@ -307,54 +368,89 @@ static inline void *kunit_alloc_resource(struct kunit *test, { struct kunit_resource *res; - res = kunit_alloc_and_get_resource(test, init, free, internal_gfp, - context); + res = kzalloc(sizeof(*res), internal_gfp); + if (!res) + return NULL; - if (res) - return res->allocation; + if (!kunit_add_resource(test, init, free, res, context)) + return res->data; return NULL; } typedef bool (*kunit_resource_match_t)(struct kunit *test, - const void *res, + 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 data stored in kunit_resource->allocation. + * @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, - const void *res, + struct kunit_resource *res, void *match_data) { - return res == match_data; + return res->data == match_data; } /** - * kunit_resource_destroy() - Find a kunit_resource and destroy it. + * 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; + + spin_lock(&test->lock); + + 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(&test->lock); + + return found; +} + +/** + * 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. - * @free: Must match free on the kunit_resource to free. * @match_data: Data passed into @match. * - * Free the latest kunit_resource of @test for which @free matches the - * kunit_resource_free_t associated with the resource and for which @match - * returns true. - * * RETURNS: * 0 if kunit_resource is found and freed, -ENOENT if not found. */ -int kunit_resource_destroy(struct kunit *test, +int kunit_destroy_resource(struct kunit *test, kunit_resource_match_t match, - kunit_resource_free_t free, void *match_data); +/** + * 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() - Like kmalloc() except the allocation is *test managed*. * @test: The test context object. diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c index 4f3d36a72f8f..03f3ecaa1ef9 100644 --- a/lib/kunit/kunit-test.c +++ b/lib/kunit/kunit-test.c @@ -118,14 +118,14 @@ static int fake_resource_init(struct kunit_resource *res, void *context) { struct kunit_test_resource_context *ctx = context; - res->allocation = &ctx->is_resource_initialized; + res->data = &ctx->is_resource_initialized; ctx->is_resource_initialized = true; return 0; } static void fake_resource_free(struct kunit_resource *res) { - bool *is_resource_initialized = res->allocation; + bool *is_resource_initialized = res->data; *is_resource_initialized = false; } @@ -154,11 +154,21 @@ static void kunit_resource_test_alloc_resource(struct kunit *test) KUNIT_ASSERT_NOT_ERR_OR_NULL(test, res); KUNIT_EXPECT_PTR_EQ(test, &ctx->is_resource_initialized, - (bool *) res->allocation); + (bool *)res->data); KUNIT_EXPECT_TRUE(test, list_is_last(&res->node, &ctx->test.resources)); KUNIT_EXPECT_PTR_EQ(test, free, res->free); + + kunit_put_resource(res); } +/* + * Note: tests below use kunit_alloc_and_get_resource(), so as a consequence + * they have a reference to the associated resource that they must release + * via kunit_put_resource(). In normal operation, users will only + * have to do this for cases where they use kunit_find_resource(), and the + * kunit_alloc_resource() function will be used (which does not take a + * resource reference). + */ static void kunit_resource_test_destroy_resource(struct kunit *test) { struct kunit_test_resource_context *ctx = test->priv; @@ -169,11 +179,12 @@ static void kunit_resource_test_destroy_resource(struct kunit *test) GFP_KERNEL, ctx); + kunit_put_resource(res); + KUNIT_ASSERT_FALSE(test, - kunit_resource_destroy(&ctx->test, + kunit_destroy_resource(&ctx->test, kunit_resource_instance_match, - res->free, - res->allocation)); + res->data)); KUNIT_EXPECT_FALSE(test, ctx->is_resource_initialized); KUNIT_EXPECT_TRUE(test, list_empty(&ctx->test.resources)); @@ -191,6 +202,7 @@ static void kunit_resource_test_cleanup_resources(struct kunit *test) fake_resource_free, GFP_KERNEL, ctx); + kunit_put_resource(resources[i]); } kunit_cleanup(&ctx->test); @@ -221,14 +233,14 @@ static int fake_resource_2_init(struct kunit_resource *res, void *context) KUNIT_RESOURCE_TEST_MARK_ORDER(ctx, allocate_order, 2); - res->allocation = ctx; + res->data = ctx; return 0; } static void fake_resource_2_free(struct kunit_resource *res) { - struct kunit_test_resource_context *ctx = res->allocation; + struct kunit_test_resource_context *ctx = res->data; KUNIT_RESOURCE_TEST_MARK_ORDER(ctx, free_order, 2); } @@ -236,23 +248,26 @@ static void fake_resource_2_free(struct kunit_resource *res) static int fake_resource_1_init(struct kunit_resource *res, void *context) { struct kunit_test_resource_context *ctx = context; + struct kunit_resource *res2; - kunit_alloc_and_get_resource(&ctx->test, - fake_resource_2_init, - fake_resource_2_free, - GFP_KERNEL, - ctx); + res2 = kunit_alloc_and_get_resource(&ctx->test, + fake_resource_2_init, + fake_resource_2_free, + GFP_KERNEL, + ctx); KUNIT_RESOURCE_TEST_MARK_ORDER(ctx, allocate_order, 1); - res->allocation = ctx; + res->data = ctx; + + kunit_put_resource(res2); return 0; } static void fake_resource_1_free(struct kunit_resource *res) { - struct kunit_test_resource_context *ctx = res->allocation; + struct kunit_test_resource_context *ctx = res->data; KUNIT_RESOURCE_TEST_MARK_ORDER(ctx, free_order, 1); } @@ -265,13 +280,14 @@ static void fake_resource_1_free(struct kunit_resource *res) static void kunit_resource_test_proper_free_ordering(struct kunit *test) { struct kunit_test_resource_context *ctx = test->priv; + struct kunit_resource *res; /* fake_resource_1 allocates a fake_resource_2 in its init. */ - kunit_alloc_and_get_resource(&ctx->test, - fake_resource_1_init, - fake_resource_1_free, - GFP_KERNEL, - ctx); + res = kunit_alloc_and_get_resource(&ctx->test, + fake_resource_1_init, + fake_resource_1_free, + GFP_KERNEL, + ctx); /* * Since fake_resource_2_init calls KUNIT_RESOURCE_TEST_MARK_ORDER @@ -281,6 +297,8 @@ static void kunit_resource_test_proper_free_ordering(struct kunit *test) KUNIT_EXPECT_EQ(test, ctx->allocate_order[0], 2); KUNIT_EXPECT_EQ(test, ctx->allocate_order[1], 1); + kunit_put_resource(res); + kunit_cleanup(&ctx->test); /* @@ -292,6 +310,21 @@ static void kunit_resource_test_proper_free_ordering(struct kunit *test) KUNIT_EXPECT_EQ(test, ctx->free_order[1], 2); } +static void kunit_resource_test_static(struct kunit *test) +{ + struct kunit_test_resource_context ctx; + struct kunit_resource res; + + KUNIT_EXPECT_EQ(test, kunit_add_resource(test, NULL, NULL, &res, &ctx), + 0); + + KUNIT_EXPECT_PTR_EQ(test, res.data, (void *)&ctx); + + kunit_cleanup(test); + + KUNIT_EXPECT_TRUE(test, list_empty(&test->resources)); +} + static int kunit_resource_test_init(struct kunit *test) { struct kunit_test_resource_context *ctx = @@ -320,6 +353,7 @@ static struct kunit_case kunit_resource_test_cases[] = { KUNIT_CASE(kunit_resource_test_destroy_resource), KUNIT_CASE(kunit_resource_test_cleanup_resources), KUNIT_CASE(kunit_resource_test_proper_free_ordering), + KUNIT_CASE(kunit_resource_test_static), {} }; diff --git a/lib/kunit/string-stream.c b/lib/kunit/string-stream.c index 350392013c14..141789ca8949 100644 --- a/lib/kunit/string-stream.c +++ b/lib/kunit/string-stream.c @@ -33,14 +33,14 @@ static int string_stream_fragment_init(struct kunit_resource *res, if (!frag->fragment) return -ENOMEM; - res->allocation = frag; + res->data = frag; return 0; } static void string_stream_fragment_free(struct kunit_resource *res) { - struct string_stream_fragment *frag = res->allocation; + struct string_stream_fragment *frag = res->data; list_del(&frag->node); kunit_kfree(frag->test, frag->fragment); @@ -65,9 +65,8 @@ static struct string_stream_fragment *alloc_string_stream_fragment( static int string_stream_fragment_destroy(struct string_stream_fragment *frag) { - return kunit_resource_destroy(frag->test, + return kunit_destroy_resource(frag->test, kunit_resource_instance_match, - string_stream_fragment_free, frag); } @@ -179,7 +178,7 @@ static int string_stream_init(struct kunit_resource *res, void *context) if (!stream) return -ENOMEM; - res->allocation = stream; + res->data = stream; stream->gfp = ctx->gfp; stream->test = ctx->test; INIT_LIST_HEAD(&stream->fragments); @@ -190,7 +189,7 @@ static int string_stream_init(struct kunit_resource *res, void *context) static void string_stream_free(struct kunit_resource *res) { - struct string_stream *stream = res->allocation; + struct string_stream *stream = res->data; string_stream_clear(stream); } @@ -211,8 +210,7 @@ struct string_stream *alloc_string_stream(struct kunit *test, gfp_t gfp) int string_stream_destroy(struct string_stream *stream) { - return kunit_resource_destroy(stream->test, + return kunit_destroy_resource(stream->test, kunit_resource_instance_match, - string_stream_free, stream); } diff --git a/lib/kunit/test.c b/lib/kunit/test.c index ccb2ffad8dcf..569b7f9f17e4 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -8,6 +8,7 @@ #include #include +#include #include #include "debugfs.h" @@ -406,90 +407,92 @@ void __kunit_test_suites_exit(struct kunit_suite **suites) } EXPORT_SYMBOL_GPL(__kunit_test_suites_exit); -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) +/* + * 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) { - struct kunit_resource *res; - int ret; + int ret = 0; - res = kzalloc(sizeof(*res), internal_gfp); - if (!res) - return NULL; + res->free = free; + kref_init(&res->refcount); - ret = init(res, context); - if (ret) - return NULL; + if (init) { + ret = init(res, data); + if (ret) + return ret; + } else { + res->data = data; + } - res->free = free; spin_lock(&test->lock); list_add_tail(&res->node, &test->resources); + /* refcount for list is established by kref_init() */ spin_unlock(&test->lock); - return res; -} -EXPORT_SYMBOL_GPL(kunit_alloc_and_get_resource); - -static void kunit_resource_free(struct kunit *test, struct kunit_resource *res) -{ - res->free(res); - kfree(res); + return ret; } +EXPORT_SYMBOL_GPL(kunit_add_resource); -static struct kunit_resource *kunit_resource_find(struct kunit *test, - kunit_resource_match_t match, - kunit_resource_free_t free, - void *match_data) +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 *resource; + struct kunit_resource *res; + int ret; - lockdep_assert_held(&test->lock); + res = kzalloc(sizeof(*res), internal_gfp); + if (!res) + return NULL; - list_for_each_entry_reverse(resource, &test->resources, node) { - if (resource->free != free) - continue; - if (match(test, resource->allocation, match_data)) - return resource; + 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); -static struct kunit_resource *kunit_resource_remove( - struct kunit *test, - kunit_resource_match_t match, - kunit_resource_free_t free, - void *match_data) +void kunit_remove_resource(struct kunit *test, struct kunit_resource *res) { - struct kunit_resource *resource; - spin_lock(&test->lock); - resource = kunit_resource_find(test, match, free, match_data); - if (resource) - list_del(&resource->node); + list_del(&res->node); spin_unlock(&test->lock); - - return resource; + kunit_put_resource(res); } +EXPORT_SYMBOL_GPL(kunit_remove_resource); -int kunit_resource_destroy(struct kunit *test, - kunit_resource_match_t match, - kunit_resource_free_t free, +int kunit_destroy_resource(struct kunit *test, kunit_resource_match_t match, void *match_data) { - struct kunit_resource *resource; - - resource = kunit_resource_remove(test, match, free, match_data); + struct kunit_resource *res = kunit_find_resource(test, match, + match_data); - if (!resource) + if (!res) return -ENOENT; - kunit_resource_free(test, resource); + kunit_remove_resource(test, res); + + /* We have a reference also via _find(); drop it. */ + kunit_put_resource(res); + return 0; } -EXPORT_SYMBOL_GPL(kunit_resource_destroy); +EXPORT_SYMBOL_GPL(kunit_destroy_resource); struct kunit_kmalloc_params { size_t size; @@ -500,8 +503,8 @@ static int kunit_kmalloc_init(struct kunit_resource *res, void *context) { struct kunit_kmalloc_params *params = context; - res->allocation = kmalloc(params->size, params->gfp); - if (!res->allocation) + res->data = kmalloc(params->size, params->gfp); + if (!res->data) return -ENOMEM; return 0; @@ -509,7 +512,7 @@ static int kunit_kmalloc_init(struct kunit_resource *res, void *context) static void kunit_kmalloc_free(struct kunit_resource *res) { - kfree(res->allocation); + kfree(res->data); } void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp) @@ -529,20 +532,25 @@ EXPORT_SYMBOL_GPL(kunit_kmalloc); void kunit_kfree(struct kunit *test, const void *ptr) { - int rc; + struct kunit_resource *res; - rc = kunit_resource_destroy(test, - kunit_resource_instance_match, - kunit_kmalloc_free, - (void *)ptr); + res = kunit_find_resource(test, kunit_resource_instance_match, + (void *)ptr); + + /* + * Removing the resource from the list of resources drops the + * reference count to 1; the final put will trigger the free. + */ + kunit_remove_resource(test, res); + + kunit_put_resource(res); - WARN_ON(rc); } EXPORT_SYMBOL_GPL(kunit_kfree); void kunit_cleanup(struct kunit *test) { - struct kunit_resource *resource; + struct kunit_resource *res; /* * test->resources is a stack - each allocation must be freed in the @@ -559,13 +567,16 @@ void kunit_cleanup(struct kunit *test) spin_unlock(&test->lock); break; } - resource = list_last_entry(&test->resources, - struct kunit_resource, - node); - list_del(&resource->node); + res = list_last_entry(&test->resources, + struct kunit_resource, + node); + /* + * Need to unlock here as a resource may remove another + * resource, and this can't happen if the test->lock + * is held. + */ spin_unlock(&test->lock); - - kunit_resource_free(test, resource); + kunit_remove_resource(test, res); } } EXPORT_SYMBOL_GPL(kunit_cleanup); -- cgit v1.2.3 From 725aca9585956676687c4cb803e88f770b0df2b2 Mon Sep 17 00:00:00 2001 From: Alan Maguire Date: Fri, 29 May 2020 22:46:21 +0100 Subject: kunit: add support for named resources The kunit resources API allows for custom initialization and cleanup code (init/fini); here a new resource add function sets the "struct kunit_resource" "name" field, and calls the standard add function. Having a simple way to name resources is useful in cases such as multithreaded tests where a set of resources are shared among threads; a pointer to the "struct kunit *" test state then is all that is needed to retrieve and use named resources. Support is provided to add, find and destroy named resources; the latter two are simply wrappers that use a "match-by-name" callback. If an attempt to add a resource with a name that already exists is made kunit_add_named_resource() will return -EEXIST. Signed-off-by: Alan Maguire Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan --- include/kunit/test.h | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++ lib/kunit/kunit-test.c | 37 ++++++++++++++++++++++++++++++++++ lib/kunit/test.c | 24 ++++++++++++++++++++++ 3 files changed, 115 insertions(+) diff --git a/include/kunit/test.h b/include/kunit/test.h index f9b914ebfd75..59f3144f009a 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -72,9 +72,15 @@ typedef void (*kunit_resource_free_t)(struct kunit_resource *); * 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; /* optional name */ /* private: internal use only. */ kunit_resource_free_t free; @@ -344,6 +350,21 @@ int kunit_add_resource(struct kunit *test, 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. + * @name_data: name and data to be set for 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); + /** * kunit_alloc_resource() - Allocates a *test managed resource*. * @test: The test context object. @@ -398,6 +419,19 @@ static inline bool kunit_resource_instance_match(struct kunit *test, 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. @@ -426,6 +460,19 @@ kunit_find_resource(struct kunit *test, 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. @@ -439,6 +486,13 @@ 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. diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c index 03f3ecaa1ef9..69f902440a0e 100644 --- a/lib/kunit/kunit-test.c +++ b/lib/kunit/kunit-test.c @@ -325,6 +325,42 @@ static void kunit_resource_test_static(struct kunit *test) KUNIT_EXPECT_TRUE(test, list_empty(&test->resources)); } +static void kunit_resource_test_named(struct kunit *test) +{ + struct kunit_resource res1, res2, *found = NULL; + struct kunit_test_resource_context ctx; + + KUNIT_EXPECT_EQ(test, + kunit_add_named_resource(test, NULL, NULL, &res1, + "resource_1", &ctx), + 0); + KUNIT_EXPECT_PTR_EQ(test, res1.data, (void *)&ctx); + + KUNIT_EXPECT_EQ(test, + kunit_add_named_resource(test, NULL, NULL, &res1, + "resource_1", &ctx), + -EEXIST); + + KUNIT_EXPECT_EQ(test, + kunit_add_named_resource(test, NULL, NULL, &res2, + "resource_2", &ctx), + 0); + + found = kunit_find_named_resource(test, "resource_1"); + + KUNIT_EXPECT_PTR_EQ(test, found, &res1); + + if (found) + kunit_put_resource(&res1); + + KUNIT_EXPECT_EQ(test, kunit_destroy_named_resource(test, "resource_2"), + 0); + + kunit_cleanup(test); + + KUNIT_EXPECT_TRUE(test, list_empty(&test->resources)); +} + static int kunit_resource_test_init(struct kunit *test) { struct kunit_test_resource_context *ctx = @@ -354,6 +390,7 @@ static struct kunit_case kunit_resource_test_cases[] = { KUNIT_CASE(kunit_resource_test_cleanup_resources), KUNIT_CASE(kunit_resource_test_proper_free_ordering), KUNIT_CASE(kunit_resource_test_static), + KUNIT_CASE(kunit_resource_test_named), {} }; diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 569b7f9f17e4..c36037200310 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -441,6 +441,30 @@ int kunit_add_resource(struct kunit *test, } 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, -- cgit v1.2.3 From 39f65da0920349deb2be14bf8c83490b8775e5d3 Mon Sep 17 00:00:00 2001 From: David Gow Date: Mon, 6 Jul 2020 21:31:20 -0700 Subject: Documentation: kunit: Remove references to --defconfig The --defconfig option in kunit_tool was removed in [1], but the getting started and kunit_tool documentation still encouraged its use. Update those documents to reflect that it's no-longer required, and is the default behaviour if no .kunitconfig is found. Also update a couple of places where .kunitconfig is still referred to as kunitconfig (this was changed in [2]). [1]: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit-fixes&id=9bdf64b35117cc10813d24e1842cd8ee40ecbf19 [2]: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit-fixes&id=14ee5cfd4512ee3d1e0047d8751450dcc6544070 Signed-off-by: David Gow Signed-off-by: Shuah Khan --- Documentation/dev-tools/kunit/kunit-tool.rst | 17 +++++------------ Documentation/dev-tools/kunit/start.rst | 2 +- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/Documentation/dev-tools/kunit/kunit-tool.rst b/Documentation/dev-tools/kunit/kunit-tool.rst index 949af2da81e5..29ae2fee8123 100644 --- a/Documentation/dev-tools/kunit/kunit-tool.rst +++ b/Documentation/dev-tools/kunit/kunit-tool.rst @@ -19,13 +19,13 @@ compiles the kernel as a standalone Linux executable that can be run like any other program directly inside of a host operating system. To be clear, it does not require any virtualization support: it is just a regular program. -What is a kunitconfig? -====================== +What is a .kunitconfig? +======================= It's just a defconfig that kunit_tool looks for in the base directory. kunit_tool uses it to generate a .config as you might expect. In addition, it verifies that the generated .config contains the CONFIG options in the -kunitconfig; the reason it does this is so that it is easy to be sure that a +.kunitconfig; the reason it does this is so that it is easy to be sure that a CONFIG that enables a test actually ends up in the .config. How do I use kunit_tool? @@ -46,16 +46,9 @@ However, you most likely want to use it with the following options: - ``--timeout`` sets a maximum amount of time to allow tests to run. - ``--jobs`` sets the number of threads to use to build the kernel. -If you just want to use the defconfig that ships with the kernel, you can -append the ``--defconfig`` flag as well: - -.. code-block:: bash - - ./tools/testing/kunit/kunit.py run --timeout=30 --jobs=`nproc --all` --defconfig - .. note:: - This command is particularly helpful for getting started because it - just works. No kunitconfig needs to be present. + This command will work even without a .kunitconfig file: if no + .kunitconfig is present, a default one will be used instead. For a list of all the flags supported by kunit_tool, you can run: diff --git a/Documentation/dev-tools/kunit/start.rst b/Documentation/dev-tools/kunit/start.rst index bb112cf70624..d23385e3e159 100644 --- a/Documentation/dev-tools/kunit/start.rst +++ b/Documentation/dev-tools/kunit/start.rst @@ -18,7 +18,7 @@ The wrapper can be run with: .. code-block:: bash - ./tools/testing/kunit/kunit.py run --defconfig + ./tools/testing/kunit/kunit.py run For more information on this wrapper (also called kunit_tool) check out the :doc:`kunit-tool` page. -- cgit v1.2.3 From 5a9fcad71caa970f30aef99134a1cd19bc4b8eea Mon Sep 17 00:00:00 2001 From: Will Chen Date: Wed, 8 Jul 2020 14:35:43 -0700 Subject: kunit: capture stderr on all make subprocess calls Direct stderr to subprocess.STDOUT so error messages get included in the subprocess.CalledProcessError exceptions output field. This results in more meaningful error messages for the user. This is already being done in the make_allyesconfig method. Do the same for make_mrproper, make_olddefconfig, and make methods. With this, failures on unclean trees [1] will give users an error message that includes: "The source tree is not clean, please run 'make ARCH=um mrproper'" [1] https://bugzilla.kernel.org/show_bug.cgi?id=205219 Signed-off-by: Will Chen Reviewed-by: Brendan Higgins Tested-by: Brendan Higgins Signed-off-by: Shuah Khan --- tools/testing/kunit/kunit_kernel.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index 63dbda2d029f..e20e2056cb38 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -34,7 +34,7 @@ class LinuxSourceTreeOperations(object): def make_mrproper(self): try: - subprocess.check_output(['make', 'mrproper']) + subprocess.check_output(['make', 'mrproper'], stderr=subprocess.STDOUT) except OSError as e: raise ConfigError('Could not call make command: ' + e) except subprocess.CalledProcessError as e: @@ -47,7 +47,7 @@ class LinuxSourceTreeOperations(object): if build_dir: command += ['O=' + build_dir] try: - subprocess.check_output(command, stderr=subprocess.PIPE) + subprocess.check_output(command, stderr=subprocess.STDOUT) except OSError as e: raise ConfigError('Could not call make command: ' + e) except subprocess.CalledProcessError as e: @@ -77,7 +77,7 @@ class LinuxSourceTreeOperations(object): if build_dir: command += ['O=' + build_dir] try: - subprocess.check_output(command) + subprocess.check_output(command, stderr=subprocess.STDOUT) except OSError as e: raise BuildError('Could not call execute make: ' + e) except subprocess.CalledProcessError as e: -- cgit v1.2.3 From 6816fe61bda8c819c368ad2002cd27172ecb79de Mon Sep 17 00:00:00 2001 From: Brendan Higgins Date: Tue, 14 Jul 2020 13:41:29 -0700 Subject: kunit: tool: fix broken default args in unit tests Commit ddbd60c779b4 ("kunit: use --build_dir=.kunit as default") changed the default build directory for KUnit tests, but failed to update associated unit tests for kunit_tool, so update them. Fixes: ddbd60c779b4 ("kunit: use --build_dir=.kunit as default") Signed-off-by: Brendan Higgins Signed-off-by: Shuah Khan --- tools/testing/kunit/kunit_tool_test.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py index 5bb7b118ebd9..f0da3f565577 100755 --- a/tools/testing/kunit/kunit_tool_test.py +++ b/tools/testing/kunit/kunit_tool_test.py @@ -247,14 +247,14 @@ class KUnitMainTest(unittest.TestCase): def test_build_passes_args_pass(self): kunit.main(['build'], self.linux_source_mock) assert self.linux_source_mock.build_reconfig.call_count == 0 - self.linux_source_mock.build_um_kernel.assert_called_once_with(False, 8, '', None) + self.linux_source_mock.build_um_kernel.assert_called_once_with(False, 8, '.kunit', None) assert self.linux_source_mock.run_kernel.call_count == 0 def test_exec_passes_args_pass(self): kunit.main(['exec'], self.linux_source_mock) assert self.linux_source_mock.build_reconfig.call_count == 0 assert self.linux_source_mock.run_kernel.call_count == 1 - self.linux_source_mock.run_kernel.assert_called_once_with(build_dir='', timeout=300) + self.linux_source_mock.run_kernel.assert_called_once_with(build_dir='.kunit', timeout=300) self.print_mock.assert_any_call(StrContains('Testing complete.')) def test_run_passes_args_pass(self): @@ -262,7 +262,7 @@ class KUnitMainTest(unittest.TestCase): assert self.linux_source_mock.build_reconfig.call_count == 1 assert self.linux_source_mock.run_kernel.call_count == 1 self.linux_source_mock.run_kernel.assert_called_once_with( - build_dir='', timeout=300) + build_dir='.kunit', timeout=300) self.print_mock.assert_any_call(StrContains('Testing complete.')) def test_exec_passes_args_fail(self): @@ -302,7 +302,7 @@ class KUnitMainTest(unittest.TestCase): def test_exec_timeout(self): timeout = 3453 kunit.main(['exec', '--timeout', str(timeout)], self.linux_source_mock) - self.linux_source_mock.run_kernel.assert_called_once_with(build_dir='', timeout=timeout) + self.linux_source_mock.run_kernel.assert_called_once_with(build_dir='.kunit', timeout=timeout) self.print_mock.assert_any_call(StrContains('Testing complete.')) def test_run_timeout(self): @@ -310,7 +310,7 @@ class KUnitMainTest(unittest.TestCase): kunit.main(['run', '--timeout', str(timeout)], self.linux_source_mock) assert self.linux_source_mock.build_reconfig.call_count == 1 self.linux_source_mock.run_kernel.assert_called_once_with( - build_dir='', timeout=timeout) + build_dir='.kunit', timeout=timeout) self.print_mock.assert_any_call(StrContains('Testing complete.')) def test_run_builddir(self): -- cgit v1.2.3 From d43c7fb05765152d4d4a39a8ef957c4ea14d8847 Mon Sep 17 00:00:00 2001 From: Brendan Higgins Date: Tue, 14 Jul 2020 13:41:30 -0700 Subject: kunit: tool: fix improper treatment of file location Commit 01397e822af4 ("kunit: Fix TabError, remove defconfig code and handle when there is no kunitconfig") and commit 45ba7a893ad8 ("kunit: kunit_tool: Separate out config/build/exec/parse") introduced two closely related issues which built off of each other: they excessively created the build directory when not present and modified a constant (constants in Python only exist by convention). Together these issues broken a number of unit tests for KUnit tool, so fix them. Fixed up commit log to fic checkpatch commit description style error. Shuah Khan Fixes: 01397e822af4 ("kunit: Fix TabError, remove defconfig code and handle when there is no kunitconfig") Fixes: 45ba7a893ad8 ("kunit: kunit_tool: Separate out config/build/exec/parse") Signed-off-by: Brendan Higgins Signed-off-by: Shuah Khan --- tools/testing/kunit/kunit.py | 24 ------------------------ tools/testing/kunit/kunit_tool_test.py | 4 ++-- 2 files changed, 2 insertions(+), 26 deletions(-) diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index 787b6d4ad716..2ece17e9eab5 100755 --- a/tools/testing/kunit/kunit.py +++ b/tools/testing/kunit/kunit.py @@ -238,12 +238,6 @@ def main(argv, linux=None): if cli_args.subcommand == 'run': if not os.path.exists(cli_args.build_dir): os.mkdir(cli_args.build_dir) - kunit_kernel.kunitconfig_path = os.path.join( - cli_args.build_dir, - kunit_kernel.kunitconfig_path) - - if not os.path.exists(kunit_kernel.kunitconfig_path): - create_default_kunitconfig() if not linux: linux = kunit_kernel.LinuxSourceTree() @@ -261,12 +255,6 @@ def main(argv, linux=None): if cli_args.build_dir: if not os.path.exists(cli_args.build_dir): os.mkdir(cli_args.build_dir) - kunit_kernel.kunitconfig_path = os.path.join( - cli_args.build_dir, - kunit_kernel.kunitconfig_path) - - if not os.path.exists(kunit_kernel.kunitconfig_path): - create_default_kunitconfig() if not linux: linux = kunit_kernel.LinuxSourceTree() @@ -283,12 +271,6 @@ def main(argv, linux=None): if cli_args.build_dir: if not os.path.exists(cli_args.build_dir): os.mkdir(cli_args.build_dir) - kunit_kernel.kunitconfig_path = os.path.join( - cli_args.build_dir, - kunit_kernel.kunitconfig_path) - - if not os.path.exists(kunit_kernel.kunitconfig_path): - create_default_kunitconfig() if not linux: linux = kunit_kernel.LinuxSourceTree() @@ -307,12 +289,6 @@ def main(argv, linux=None): if cli_args.build_dir: if not os.path.exists(cli_args.build_dir): os.mkdir(cli_args.build_dir) - kunit_kernel.kunitconfig_path = os.path.join( - cli_args.build_dir, - kunit_kernel.kunitconfig_path) - - if not os.path.exists(kunit_kernel.kunitconfig_path): - create_default_kunitconfig() if not linux: linux = kunit_kernel.LinuxSourceTree() diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py index f0da3f565577..ebedfc57a39b 100755 --- a/tools/testing/kunit/kunit_tool_test.py +++ b/tools/testing/kunit/kunit_tool_test.py @@ -240,7 +240,7 @@ class KUnitMainTest(unittest.TestCase): pass def test_config_passes_args_pass(self): - kunit.main(['config'], self.linux_source_mock) + kunit.main(['config', '--build_dir=.kunit'], self.linux_source_mock) assert self.linux_source_mock.build_reconfig.call_count == 1 assert self.linux_source_mock.run_kernel.call_count == 0 @@ -315,7 +315,7 @@ class KUnitMainTest(unittest.TestCase): def test_run_builddir(self): build_dir = '.kunit' - kunit.main(['run', '--build_dir', build_dir], self.linux_source_mock) + kunit.main(['run', '--build_dir=.kunit'], self.linux_source_mock) assert self.linux_source_mock.build_reconfig.call_count == 1 self.linux_source_mock.run_kernel.assert_called_once_with( build_dir=build_dir, timeout=300) -- cgit v1.2.3