* [PATCH] lib: use of kunit in test_parman.c
@ 2021-07-27 22:58 José Aquiles Guedes de Rezende
2021-07-28 0:04 ` Daniel Latypov
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: José Aquiles Guedes de Rezende @ 2021-07-27 22:58 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, linux-kernel, brendanhiggins, dlatypov, davidgow,
linux-kselftest, ~lkcamp/patches,
José Aquiles Guedes de Rezende,
Matheus Henrique de Souza Silva
Convert the parman test module to use the KUnit test framework.
This makes thetest clearer by leveraging KUnit's assertion macros
and test case definitions,as well as helps standardize on a testing framework.
Co-developed-by: Matheus Henrique de Souza Silva <matheushenriquedesouzasilva@protonmail.com>
Signed-off-by: Matheus Henrique de Souza Silva <matheushenriquedesouzasilva@protonmail.com>
Signed-off-by: José Aquiles Guedes de Rezende <jjoseaquiless@gmail.com>
---
lib/test_parman.c | 145 +++++++++++++++++++---------------------------
1 file changed, 60 insertions(+), 85 deletions(-)
diff --git a/lib/test_parman.c b/lib/test_parman.c
index 35e32243693c..bd5010f0a412 100644
--- a/lib/test_parman.c
+++ b/lib/test_parman.c
@@ -41,6 +41,8 @@
#include <linux/err.h>
#include <linux/random.h>
#include <linux/parman.h>
+#include <linux/sched.h>
+#include <kunit/test.h>
#define TEST_PARMAN_PRIO_SHIFT 7 /* defines number of prios for testing */
#define TEST_PARMAN_PRIO_COUNT BIT(TEST_PARMAN_PRIO_SHIFT)
@@ -91,12 +93,14 @@ struct test_parman {
static int test_parman_resize(void *priv, unsigned long new_count)
{
+ struct kunit *test = current->kunit_test;
struct test_parman *test_parman = priv;
struct test_parman_item **prio_array;
unsigned long old_count;
prio_array = krealloc(test_parman->prio_array,
ITEM_PTRS_SIZE(new_count), GFP_KERNEL);
+ KUNIT_EXPECT_NOT_ERR_OR_NULL(test, prio_array);
if (new_count == 0)
return 0;
if (!prio_array)
@@ -214,42 +218,39 @@ static void test_parman_items_fini(struct test_parman *test_parman)
}
}
-static struct test_parman *test_parman_create(const struct parman_ops *ops)
+static int test_parman_create(struct kunit *test)
{
struct test_parman *test_parman;
int err;
- test_parman = kzalloc(sizeof(*test_parman), GFP_KERNEL);
- if (!test_parman)
- return ERR_PTR(-ENOMEM);
+ test_parman = kunit_kzalloc(test, sizeof(*test_parman), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_parman);
+
err = test_parman_resize(test_parman, TEST_PARMAN_BASE_COUNT);
- if (err)
- goto err_resize;
- test_parman->parman = parman_create(ops, test_parman);
- if (!test_parman->parman) {
- err = -ENOMEM;
- goto err_parman_create;
- }
+ KUNIT_ASSERT_EQ(test, err, 0);
+
+ test_parman->parman = parman_create(&test_parman_lsort_ops, test_parman);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_parman->parman);
+
test_parman_rnd_init(test_parman);
test_parman_prios_init(test_parman);
test_parman_items_init(test_parman);
test_parman->run_budget = TEST_PARMAN_RUN_BUDGET;
- return test_parman;
-
-err_parman_create:
- test_parman_resize(test_parman, 0);
-err_resize:
- kfree(test_parman);
- return ERR_PTR(err);
+ test->priv = test_parman;
+ return 0;
}
-static void test_parman_destroy(struct test_parman *test_parman)
+static void test_parman_destroy(struct kunit *test)
{
+ struct test_parman *test_parman = test->priv;
+
+ if (!test_parman)
+ return;
test_parman_items_fini(test_parman);
test_parman_prios_fini(test_parman);
parman_destroy(test_parman->parman);
test_parman_resize(test_parman, 0);
- kfree(test_parman);
+ kunit_kfree(test, test_parman);
}
static bool test_parman_run_check_budgets(struct test_parman *test_parman)
@@ -265,8 +266,9 @@ static bool test_parman_run_check_budgets(struct test_parman *test_parman)
return true;
}
-static int test_parman_run(struct test_parman *test_parman)
+static void test_parman_run(struct kunit *test)
{
+ struct test_parman *test_parman = test->priv;
unsigned int i = test_parman_rnd_get(test_parman);
int err;
@@ -281,8 +283,8 @@ static int test_parman_run(struct test_parman *test_parman)
err = parman_item_add(test_parman->parman,
&item->prio->parman_prio,
&item->parman_item);
- if (err)
- return err;
+ KUNIT_ASSERT_EQ(test, err, 0);
+
test_parman->prio_array[item->parman_item.index] = item;
test_parman->used_items++;
} else {
@@ -294,22 +296,19 @@ static int test_parman_run(struct test_parman *test_parman)
}
item->used = !item->used;
}
- return 0;
}
-static int test_parman_check_array(struct test_parman *test_parman,
- bool gaps_allowed)
+static void test_parman_check_array(struct kunit *test, bool gaps_allowed)
{
unsigned int last_unused_items = 0;
unsigned long last_priority = 0;
unsigned int used_items = 0;
int i;
+ struct test_parman *test_parman = test->priv;
- if (test_parman->prio_array_limit < TEST_PARMAN_BASE_COUNT) {
- pr_err("Array limit is lower than the base count (%lu < %lu)\n",
- test_parman->prio_array_limit, TEST_PARMAN_BASE_COUNT);
- return -EINVAL;
- }
+ KUNIT_ASSERT_GE_MSG(test, test_parman->prio_array_limit, TEST_PARMAN_BASE_COUNT,
+ "Array limit is lower than the base count (%lu < %lu)\n",
+ test_parman->prio_array_limit, TEST_PARMAN_BASE_COUNT);
for (i = 0; i < test_parman->prio_array_limit; i++) {
struct test_parman_item *item = test_parman->prio_array[i];
@@ -318,77 +317,53 @@ static int test_parman_check_array(struct test_parman *test_parman,
last_unused_items++;
continue;
}
- if (last_unused_items && !gaps_allowed) {
- pr_err("Gap found in array even though they are forbidden\n");
- return -EINVAL;
- }
+
+ KUNIT_ASSERT_FALSE_MSG(test, last_unused_items && !gaps_allowed,
+ "Gap found in array even though they are forbidden\n");
last_unused_items = 0;
used_items++;
- if (item->prio->priority < last_priority) {
- pr_err("Item belongs under higher priority then the last one (current: %lu, previous: %lu)\n",
- item->prio->priority, last_priority);
- return -EINVAL;
- }
- last_priority = item->prio->priority;
+ KUNIT_ASSERT_GE_MSG(test, item->prio->priority, last_priority,
+ "Item belongs under higher priority then the last one (current: %lu, previous: %lu)\n",
+ item->prio->priority, last_priority);
- if (item->parman_item.index != i) {
- pr_err("Item has different index in compare to where it actually is (%lu != %d)\n",
- item->parman_item.index, i);
- return -EINVAL;
- }
- }
+ last_priority = item->prio->priority;
- if (used_items != test_parman->used_items) {
- pr_err("Number of used items in array does not match (%u != %u)\n",
- used_items, test_parman->used_items);
- return -EINVAL;
- }
+ KUNIT_ASSERT_EQ_MSG(test, item->parman_item.index, (unsigned long)i,
+ "Item has different index in compare to where it actually is (%lu != %d)\n",
+ item->parman_item.index, i);
- if (last_unused_items >= TEST_PARMAN_RESIZE_STEP_COUNT) {
- pr_err("Number of unused item at the end of array is bigger than resize step (%u >= %lu)\n",
- last_unused_items, TEST_PARMAN_RESIZE_STEP_COUNT);
- return -EINVAL;
}
- pr_info("Priority array check successful\n");
+ KUNIT_ASSERT_EQ_MSG(test, used_items, test_parman->used_items,
+ "Number of used items in array does not match (%u != %u)\n",
+ used_items, test_parman->used_items);
- return 0;
+ KUNIT_ASSERT_LT_MSG(test, (unsigned long)last_unused_items, TEST_PARMAN_RESIZE_STEP_COUNT,
+ "Number of unused item at the end of array is bigger than resize step (%u >= %lu)\n",
+ last_unused_items, TEST_PARMAN_RESIZE_STEP_COUNT);
}
-static int test_parman_lsort(void)
+static void test_parman_lsort(struct kunit *test)
{
- struct test_parman *test_parman;
- int err;
-
- test_parman = test_parman_create(&test_parman_lsort_ops);
- if (IS_ERR(test_parman))
- return PTR_ERR(test_parman);
-
- err = test_parman_run(test_parman);
- if (err)
- goto out;
-
- err = test_parman_check_array(test_parman, false);
- if (err)
- goto out;
-out:
- test_parman_destroy(test_parman);
- return err;
+ test_parman_run(test);
+ test_parman_check_array(test, false);
}
-static int __init test_parman_init(void)
-{
- return test_parman_lsort();
-}
+static struct kunit_case parman_test_case[] = {
+ KUNIT_CASE(test_parman_lsort),
+ {}
+};
-static void __exit test_parman_exit(void)
-{
-}
+static struct kunit_suite parman_test_suite = {
+ .name = "parman",
+ .init = test_parman_create,
+ .exit = test_parman_destroy,
+ .test_cases = parman_test_case,
+};
-module_init(test_parman_init);
-module_exit(test_parman_exit);
+kunit_test_suite(parman_test_suite);
MODULE_LICENSE("Dual BSD/GPL");
MODULE_AUTHOR("Jiri Pirko <jiri@mellanox.com>");
--
2.32.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] lib: use of kunit in test_parman.c
2021-07-27 22:58 [PATCH] lib: use of kunit in test_parman.c José Aquiles Guedes de Rezende
@ 2021-07-28 0:04 ` Daniel Latypov
2021-07-28 1:37 ` David Gow
2021-07-28 4:46 ` kernel test robot
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Latypov @ 2021-07-28 0:04 UTC (permalink / raw)
To: José Aquiles Guedes de Rezende
Cc: Jiri Pirko, netdev, linux-kernel, brendanhiggins, davidgow,
linux-kselftest, ~lkcamp/patches,
Matheus Henrique de Souza Silva
On Tue, Jul 27, 2021 at 4:04 PM José Aquiles Guedes de Rezende
<jjoseaquiless@gmail.com> wrote:
>
> Convert the parman test module to use the KUnit test framework.
> This makes thetest clearer by leveraging KUnit's assertion macros
nit: s/thetest/the test
> and test case definitions,as well as helps standardize on a testing framework.
>
> Co-developed-by: Matheus Henrique de Souza Silva <matheushenriquedesouzasilva@protonmail.com>
> Signed-off-by: Matheus Henrique de Souza Silva <matheushenriquedesouzasilva@protonmail.com>
> Signed-off-by: José Aquiles Guedes de Rezende <jjoseaquiless@gmail.com>
Acked-by: Daniel Latypov <dlatypov@google.com>
I just briefly looked over the usage of KUnit a bit and left some suggestions.
It's nice to see the use of current->kunit_test in an ops struct.
I wrote that up as a potential use case in commit 359a376081d4
("kunit: support failure from dynamic analysis tools"), and I think
this is the first example of it being used as such :)
> ---
> lib/test_parman.c | 145 +++++++++++++++++++---------------------------
> 1 file changed, 60 insertions(+), 85 deletions(-)
>
> diff --git a/lib/test_parman.c b/lib/test_parman.c
> index 35e32243693c..bd5010f0a412 100644
> --- a/lib/test_parman.c
> +++ b/lib/test_parman.c
> @@ -41,6 +41,8 @@
> #include <linux/err.h>
> #include <linux/random.h>
> #include <linux/parman.h>
> +#include <linux/sched.h>
> +#include <kunit/test.h>
>
> #define TEST_PARMAN_PRIO_SHIFT 7 /* defines number of prios for testing */
> #define TEST_PARMAN_PRIO_COUNT BIT(TEST_PARMAN_PRIO_SHIFT)
> @@ -91,12 +93,14 @@ struct test_parman {
>
> static int test_parman_resize(void *priv, unsigned long new_count)
> {
> + struct kunit *test = current->kunit_test;
> struct test_parman *test_parman = priv;
> struct test_parman_item **prio_array;
> unsigned long old_count;
>
> prio_array = krealloc(test_parman->prio_array,
> ITEM_PTRS_SIZE(new_count), GFP_KERNEL);
> + KUNIT_EXPECT_NOT_ERR_OR_NULL(test, prio_array);
> if (new_count == 0)
> return 0;
> if (!prio_array)
> @@ -214,42 +218,39 @@ static void test_parman_items_fini(struct test_parman *test_parman)
> }
> }
>
> -static struct test_parman *test_parman_create(const struct parman_ops *ops)
> +static int test_parman_create(struct kunit *test)
> {
> struct test_parman *test_parman;
> int err;
>
> - test_parman = kzalloc(sizeof(*test_parman), GFP_KERNEL);
> - if (!test_parman)
> - return ERR_PTR(-ENOMEM);
> + test_parman = kunit_kzalloc(test, sizeof(*test_parman), GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_parman);
> +
> err = test_parman_resize(test_parman, TEST_PARMAN_BASE_COUNT);
Would it be clearer to do
KUNIT_ASSERT_EQ(test, 0, test_parman_resize(test_parman,
TEST_PARMAN_BASE_COUNT));
or change the line below to:
KUNIT_ASSERT_EQ_MSG(test, err, 0, "test_parman_resize failed");
Otherwise, if the test fails there, the error message isn't too clear.
> - if (err)
> - goto err_resize;
> - test_parman->parman = parman_create(ops, test_parman);
> - if (!test_parman->parman) {
> - err = -ENOMEM;
> - goto err_parman_create;
> - }
> + KUNIT_ASSERT_EQ(test, err, 0);
> +
> + test_parman->parman = parman_create(&test_parman_lsort_ops, test_parman);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_parman->parman);
Hmm, this won't call `test_parman_resize(test_parman, 0)` on error
like it did before.
Unfortunately, KUnit is a bit clunky for cases like this right now,
where there's cleanups that need to happen but aren't handle already
by the resource API (the underlying thing behind kunit_kzalloc that
frees the mem).
We could do something like
if (IS_ERR_OR_NULL(test_parman->parman)) {
// we can use KUNIT_FAIL to just directly fail the test, or use the
assert macro for the autogenerated failure message
KUNIT_FAIL(test, "....") / KUNIT_EXPECT_NOT_ERR_OR_NULL(test, ...);
goto err_parman_create;
}
> +
> test_parman_rnd_init(test_parman);
> test_parman_prios_init(test_parman);
> test_parman_items_init(test_parman);
> test_parman->run_budget = TEST_PARMAN_RUN_BUDGET;
> - return test_parman;
> -
> -err_parman_create:
> - test_parman_resize(test_parman, 0);
> -err_resize:
> - kfree(test_parman);
> - return ERR_PTR(err);
> + test->priv = test_parman;
> + return 0;
> }
>
> -static void test_parman_destroy(struct test_parman *test_parman)
> +static void test_parman_destroy(struct kunit *test)
> {
> + struct test_parman *test_parman = test->priv;
> +
> + if (!test_parman)
> + return;
> test_parman_items_fini(test_parman);
> test_parman_prios_fini(test_parman);
> parman_destroy(test_parman->parman);
> test_parman_resize(test_parman, 0);
> - kfree(test_parman);
> + kunit_kfree(test, test_parman);
This works as-is, but FYI, it isn't necessary as we've used
kunit_kzalloc() to allocate it above.
When the test exists, it'll automatically call this function, basically.
> }
>
> static bool test_parman_run_check_budgets(struct test_parman *test_parman)
> @@ -265,8 +266,9 @@ static bool test_parman_run_check_budgets(struct test_parman *test_parman)
> return true;
> }
>
> -static int test_parman_run(struct test_parman *test_parman)
> +static void test_parman_run(struct kunit *test)
> {
> + struct test_parman *test_parman = test->priv;
> unsigned int i = test_parman_rnd_get(test_parman);
> int err;
>
> @@ -281,8 +283,8 @@ static int test_parman_run(struct test_parman *test_parman)
> err = parman_item_add(test_parman->parman,
> &item->prio->parman_prio,
> &item->parman_item);
> - if (err)
> - return err;
> + KUNIT_ASSERT_EQ(test, err, 0);
Echoing my suggestion above, we can either do
KUNIT_ASSERT_EQ(test, 0, parman_item_add(...));
or something like
KUNIT_ASSERT_EQ_MSG(test, err, 0, "parman_item_add failed")
> +
> test_parman->prio_array[item->parman_item.index] = item;
> test_parman->used_items++;
> } else {
> @@ -294,22 +296,19 @@ static int test_parman_run(struct test_parman *test_parman)
> }
> item->used = !item->used;
> }
> - return 0;
> }
>
> -static int test_parman_check_array(struct test_parman *test_parman,
> - bool gaps_allowed)
> +static void test_parman_check_array(struct kunit *test, bool gaps_allowed)
> {
> unsigned int last_unused_items = 0;
> unsigned long last_priority = 0;
> unsigned int used_items = 0;
> int i;
> + struct test_parman *test_parman = test->priv;
>
> - if (test_parman->prio_array_limit < TEST_PARMAN_BASE_COUNT) {
> - pr_err("Array limit is lower than the base count (%lu < %lu)\n",
> - test_parman->prio_array_limit, TEST_PARMAN_BASE_COUNT);
> - return -EINVAL;
> - }
> + KUNIT_ASSERT_GE_MSG(test, test_parman->prio_array_limit, TEST_PARMAN_BASE_COUNT,
> + "Array limit is lower than the base count (%lu < %lu)\n",
> + test_parman->prio_array_limit, TEST_PARMAN_BASE_COUNT);
>
> for (i = 0; i < test_parman->prio_array_limit; i++) {
> struct test_parman_item *item = test_parman->prio_array[i];
> @@ -318,77 +317,53 @@ static int test_parman_check_array(struct test_parman *test_parman,
> last_unused_items++;
> continue;
> }
> - if (last_unused_items && !gaps_allowed) {
> - pr_err("Gap found in array even though they are forbidden\n");
> - return -EINVAL;
> - }
> +
> + KUNIT_ASSERT_FALSE_MSG(test, last_unused_items && !gaps_allowed,
> + "Gap found in array even though they are forbidden\n");
FYI, you don't need the \n for these.
KUnit will put one automatically.
Ditto for the other instances.
>
> last_unused_items = 0;
> used_items++;
>
> - if (item->prio->priority < last_priority) {
> - pr_err("Item belongs under higher priority then the last one (current: %lu, previous: %lu)\n",
> - item->prio->priority, last_priority);
> - return -EINVAL;
> - }
> - last_priority = item->prio->priority;
> + KUNIT_ASSERT_GE_MSG(test, item->prio->priority, last_priority,
> + "Item belongs under higher priority then the last one (current: %lu, previous: %lu)\n",
> + item->prio->priority, last_priority);
>
> - if (item->parman_item.index != i) {
> - pr_err("Item has different index in compare to where it actually is (%lu != %d)\n",
> - item->parman_item.index, i);
> - return -EINVAL;
> - }
> - }
> + last_priority = item->prio->priority;
>
> - if (used_items != test_parman->used_items) {
> - pr_err("Number of used items in array does not match (%u != %u)\n",
> - used_items, test_parman->used_items);
> - return -EINVAL;
> - }
> + KUNIT_ASSERT_EQ_MSG(test, item->parman_item.index, (unsigned long)i,
> + "Item has different index in compare to where it actually is (%lu != %d)\n",
> + item->parman_item.index, i);
Note: the cast to `unsigned long` here shouldn't be necessary anymore,
I don't think.
See 6e62dfa6d14f ("kunit: Do not typecheck binary assertions").
>
> - if (last_unused_items >= TEST_PARMAN_RESIZE_STEP_COUNT) {
> - pr_err("Number of unused item at the end of array is bigger than resize step (%u >= %lu)\n",
> - last_unused_items, TEST_PARMAN_RESIZE_STEP_COUNT);
> - return -EINVAL;
> }
>
> - pr_info("Priority array check successful\n");
> + KUNIT_ASSERT_EQ_MSG(test, used_items, test_parman->used_items,
> + "Number of used items in array does not match (%u != %u)\n",
> + used_items, test_parman->used_items);
>
> - return 0;
> + KUNIT_ASSERT_LT_MSG(test, (unsigned long)last_unused_items, TEST_PARMAN_RESIZE_STEP_COUNT,
> + "Number of unused item at the end of array is bigger than resize step (%u >= %lu)\n",
> + last_unused_items, TEST_PARMAN_RESIZE_STEP_COUNT);
ditto here, I think the casting can be dropped now.
> }
>
> -static int test_parman_lsort(void)
> +static void test_parman_lsort(struct kunit *test)
> {
> - struct test_parman *test_parman;
> - int err;
> -
> - test_parman = test_parman_create(&test_parman_lsort_ops);
> - if (IS_ERR(test_parman))
> - return PTR_ERR(test_parman);
> -
> - err = test_parman_run(test_parman);
> - if (err)
> - goto out;
> -
> - err = test_parman_check_array(test_parman, false);
> - if (err)
> - goto out;
> -out:
> - test_parman_destroy(test_parman);
> - return err;
> + test_parman_run(test);
> + test_parman_check_array(test, false);
> }
>
> -static int __init test_parman_init(void)
> -{
> - return test_parman_lsort();
> -}
> +static struct kunit_case parman_test_case[] = {
> + KUNIT_CASE(test_parman_lsort),
> + {}
> +};
>
> -static void __exit test_parman_exit(void)
> -{
> -}
> +static struct kunit_suite parman_test_suite = {
> + .name = "parman",
> + .init = test_parman_create,
> + .exit = test_parman_destroy,
> + .test_cases = parman_test_case,
> +};
>
> -module_init(test_parman_init);
> -module_exit(test_parman_exit);
> +kunit_test_suite(parman_test_suite);
>
> MODULE_LICENSE("Dual BSD/GPL");
> MODULE_AUTHOR("Jiri Pirko <jiri@mellanox.com>");
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] lib: use of kunit in test_parman.c
2021-07-27 22:58 [PATCH] lib: use of kunit in test_parman.c José Aquiles Guedes de Rezende
2021-07-28 0:04 ` Daniel Latypov
@ 2021-07-28 1:37 ` David Gow
2021-07-28 4:46 ` kernel test robot
2 siblings, 0 replies; 4+ messages in thread
From: David Gow @ 2021-07-28 1:37 UTC (permalink / raw)
To: José Aquiles Guedes de Rezende
Cc: Jiri Pirko, Networking, Linux Kernel Mailing List,
Brendan Higgins, Daniel Latypov,
open list:KERNEL SELFTEST FRAMEWORK, ~lkcamp/patches,
Matheus Henrique de Souza Silva
On Wed, Jul 28, 2021 at 7:04 AM José Aquiles Guedes de Rezende
<jjoseaquiless@gmail.com> wrote:
>
> Convert the parman test module to use the KUnit test framework.
> This makes thetest clearer by leveraging KUnit's assertion macros
> and test case definitions,as well as helps standardize on a testing framework.
>
> Co-developed-by: Matheus Henrique de Souza Silva <matheushenriquedesouzasilva@protonmail.com>
> Signed-off-by: Matheus Henrique de Souza Silva <matheushenriquedesouzasilva@protonmail.com>
> Signed-off-by: José Aquiles Guedes de Rezende <jjoseaquiless@gmail.com>
> ---
Thanks for porting this! A few more notes from the KUnit side.
> lib/test_parman.c | 145 +++++++++++++++++++---------------------------
> 1 file changed, 60 insertions(+), 85 deletions(-)
This seems to be missing changes to lib/Kconfig.debug: you'll want to
modify the TEST_PARMAN config item to
- depend on KUNIT
- only appear if !KUNIT_ALL_TESTS
- default KUNIT_ALL_TESTS
It might also be nice to:
- select PARMAN (it's otherwise extremely unlikely a config will
actually pick up the test)
- maybe rename TEST_PARMAN to PARMAN_KUNIT_TEST (this is optional,
since this is a port of an existing test, but does make it clearer, so
it really depends on what existing usage looks like)
>
> diff --git a/lib/test_parman.c b/lib/test_parman.c
> index 35e32243693c..bd5010f0a412 100644
> --- a/lib/test_parman.c
> +++ b/lib/test_parman.c
The rest of this looks okay to me, but you should check out Daniel's
comments in his email, too.
Cheers,
-- David
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] lib: use of kunit in test_parman.c
2021-07-27 22:58 [PATCH] lib: use of kunit in test_parman.c José Aquiles Guedes de Rezende
2021-07-28 0:04 ` Daniel Latypov
2021-07-28 1:37 ` David Gow
@ 2021-07-28 4:46 ` kernel test robot
2 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2021-07-28 4:46 UTC (permalink / raw)
To: José Aquiles Guedes de Rezende, Jiri Pirko
Cc: clang-built-linux, kbuild-all, netdev, linux-kernel,
brendanhiggins, dlatypov, davidgow, linux-kselftest,
~lkcamp/patches, José Aquiles Guedes de Rezende,
Matheus Henrique de Souza Silva
[-- Attachment #1: Type: text/plain, Size: 2939 bytes --]
Hi "José,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on ipvs/master]
[also build test ERROR on linux/master linus/master v5.14-rc3 next-20210727]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Jos-Aquiles-Guedes-de-Rezende/lib-use-of-kunit-in-test_parman-c/20210728-070506
base: https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
config: powerpc64-buildonly-randconfig-r006-20210727 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project c49df15c278857adecd12db6bb1cdc96885f7079)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc64 cross compiling tool for clang build
# apt-get install binutils-powerpc64-linux-gnu
# https://github.com/0day-ci/linux/commit/28790f5c89e89dc96adf7bd4c748ddd505a52391
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jos-Aquiles-Guedes-de-Rezende/lib-use-of-kunit-in-test_parman-c/20210728-070506
git checkout 28790f5c89e89dc96adf7bd4c748ddd505a52391
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir ARCH=powerpc SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> lib/test_parman.c:96:32: error: no member named 'kunit_test' in 'struct task_struct'
struct kunit *test = current->kunit_test;
~~~~~~~ ^
1 error generated.
vim +96 lib/test_parman.c
93
94 static int test_parman_resize(void *priv, unsigned long new_count)
95 {
> 96 struct kunit *test = current->kunit_test;
97 struct test_parman *test_parman = priv;
98 struct test_parman_item **prio_array;
99 unsigned long old_count;
100
101 prio_array = krealloc(test_parman->prio_array,
102 ITEM_PTRS_SIZE(new_count), GFP_KERNEL);
103 KUNIT_EXPECT_NOT_ERR_OR_NULL(test, prio_array);
104 if (new_count == 0)
105 return 0;
106 if (!prio_array)
107 return -ENOMEM;
108 old_count = test_parman->prio_array_limit;
109 if (new_count > old_count)
110 memset(&prio_array[old_count], 0,
111 ITEM_PTRS_SIZE(new_count - old_count));
112 test_parman->prio_array = prio_array;
113 test_parman->prio_array_limit = new_count;
114 return 0;
115 }
116
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 41642 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-28 4:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-27 22:58 [PATCH] lib: use of kunit in test_parman.c José Aquiles Guedes de Rezende
2021-07-28 0:04 ` Daniel Latypov
2021-07-28 1:37 ` David Gow
2021-07-28 4:46 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).