2020-05-29 21:51:00

by Alan Maguire

[permalink] [raw]
Subject: [PATCH v4 kunit-next 0/2] kunit: extend kunit resources API

A recent RFC patch set [1] suggests some additional functionality
may be needed around kunit resources. It seems to require

1. support for resources without allocation
2. support for lookup of such resources
3. support for access to resources across multiple kernel threads

The proposed changes here are designed to address these needs.
The idea is we first generalize the API to support adding
resources with static data; then from there we support named
resources. The latter support is needed because if we are
in a different thread context and only have the "struct kunit *"
to work with, we need a way to identify a resource in lookup.

[1] https://lkml.org/lkml/2020/2/26/1286

Changes since v3:
- removed unused "init" field from "struct kunit_resources" (Brendan)

Changes since v2:
- moved a few functions relating to resource retrieval in patches
1 and 2 into include/kunit/test.h and defined as "static inline";
this allows built-in consumers to use these functions when KUnit
is built as a module

Changes since v1:
- reformatted longer parameter lists to have one parameter per-line
(Brendan, patch 1)
- fixed phrasing in various comments to clarify allocation of memory
and added comment to kunit resource tests to clarify why
kunit_put_resource() is used there (Brendan, patch 1)
- changed #define to static inline function (Brendan, patch 2)
- simplified kunit_add_named_resource() to use more of existing
code for non-named resource (Brendan, patch 2)

Alan Maguire (2):
kunit: generalize kunit_resource API beyond allocated resources
kunit: add support for named resources

Alan Maguire (2):
kunit: generalize kunit_resource API beyond allocated resources
kunit: add support for named resources

include/kunit/test.h | 210 +++++++++++++++++++++++++++++++++++++++-------
lib/kunit/kunit-test.c | 111 +++++++++++++++++++-----
lib/kunit/string-stream.c | 14 ++--
lib/kunit/test.c | 171 ++++++++++++++++++++++---------------
4 files changed, 380 insertions(+), 126 deletions(-)

--
1.8.3.1


2020-05-29 21:51:13

by Alan Maguire

[permalink] [raw]
Subject: [PATCH v4 kunit-next 2/2] 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 <[email protected]>
Reviewed-by: Brendan Higgins <[email protected]>
---
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 f9b914e..59f3144 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -72,9 +72,15 @@
* return kunit_alloc_resource(test, kunit_kmalloc_init,
* kunit_kmalloc_free, &params);
* }
+ *
+ * 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.
@@ -399,6 +420,19 @@ static inline bool kunit_resource_instance_match(struct kunit *test,
}

/**
+ * 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.
@@ -427,6 +461,19 @@ static inline bool kunit_resource_instance_match(struct kunit *test,
}

/**
+ * 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.
@@ -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 03f3eca..69f9024 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 void kunit_resource_test_exit(struct kunit *test)
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 569b7f9..c360372 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,
--
1.8.3.1

2020-06-05 21:23:46

by Brendan Higgins

[permalink] [raw]
Subject: Re: [PATCH v4 kunit-next 0/2] kunit: extend kunit resources API

On Fri, May 29, 2020 at 2:46 PM Alan Maguire <[email protected]> wrote:
>
> A recent RFC patch set [1] suggests some additional functionality
> may be needed around kunit resources. It seems to require
>
> 1. support for resources without allocation
> 2. support for lookup of such resources
> 3. support for access to resources across multiple kernel threads
>
> The proposed changes here are designed to address these needs.
> The idea is we first generalize the API to support adding
> resources with static data; then from there we support named
> resources. The latter support is needed because if we are
> in a different thread context and only have the "struct kunit *"
> to work with, we need a way to identify a resource in lookup.
>
> [1] https://lkml.org/lkml/2020/2/26/1286
>
> Changes since v3:
> - removed unused "init" field from "struct kunit_resources" (Brendan)

Shuah, it looks like you haven't sent a PR to Linus yet. Would you
mind picking this up for 5.8?

2020-06-09 20:42:00

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH v4 kunit-next 0/2] kunit: extend kunit resources API

On 6/5/20 3:20 PM, Brendan Higgins wrote:
> On Fri, May 29, 2020 at 2:46 PM Alan Maguire <[email protected]> wrote:
>>
>> A recent RFC patch set [1] suggests some additional functionality
>> may be needed around kunit resources. It seems to require
>>
>> 1. support for resources without allocation
>> 2. support for lookup of such resources
>> 3. support for access to resources across multiple kernel threads
>>
>> The proposed changes here are designed to address these needs.
>> The idea is we first generalize the API to support adding
>> resources with static data; then from there we support named
>> resources. The latter support is needed because if we are
>> in a different thread context and only have the "struct kunit *"
>> to work with, we need a way to identify a resource in lookup.
>>
>> [1] https://lkml.org/lkml/2020/2/26/1286
>>
>> Changes since v3:
>> - removed unused "init" field from "struct kunit_resources" (Brendan)
>
> Shuah, it looks like you haven't sent a PR to Linus yet. Would you
> mind picking this up for 5.8?
>

Applied to linux-kselftest kunit branch for second update for
Linux 5.8-rc1

thanks,
-- Shuah