Oi Aquiles,
respondendo em português mesmo e só internamente na nossa lista de emails.
Eu vi que você já recebeu vários reviews e até já mandou esse v2, bem legal! :D
Um detalhe é que você não colocou a lista do KUnit em cópia:
kunit-dev@googlegroups.com
Não precisa reenviar só por isso, claro, só estou falando pra você lembrar de
adicionar na v3 caso pedirem pra alterar mais alguma coisa :).
Outra coisa, essas tags que os revisores colocam, quando você reenvia uma nova
versão do patch, você tem que adicionar no seu commit, depois dos
Signed-off-by:, assim outros revisores sabem que tal e tal pessoas já revisaram.
Então no seu caso ficaria assim:
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>
Tested-by: David Gow <davidgow@google.com>
A ideia é dar um amend no commit mesmo e adicionar essas tags no final, e aí pra
v3 em diante elas já vão estar lá e os outros sabem quem já revisou/testou :)
Enfim, era isso, só umas dicas pra quando enviar a v3, mas nada que tenha que
mudar agora. Boa sorte com o restante da revisão e qualquer coisa estamos aqui
pra ajudar :).
Obrigado,
Nícolas
On Wed, Jul 28, 2021 at 01:49:31AM -0300, José Aquiles Guedes de Rezende wrote:
> Convert the parman test module to use the KUnit test framework.> This makes the test 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>> ---> > Changes in v2:> - Rename TEST_PARMAN config item to PARMAN_KUNIT_TEST> and make it work with the kunit framework.> - Change KUNIT_ASSERT_EQ to KUNIT_ASSERT_EQ_MSG.> - Call test_parman_resize(test_parman, 0) when parman_create fail> - Remove kunit_kfree.> - Remove "\n" in error messages> - Remove casts to unsigned long> > lib/Kconfig.debug | 13 +++--> lib/Makefile | 2 +-> lib/test_parman.c | 145 +++++++++++++++++++---------------------------> 3 files changed, 70 insertions(+), 90 deletions(-)> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug> index 831212722924..e68a27e5e5b0 100644> --- a/lib/Kconfig.debug> +++ b/lib/Kconfig.debug> @@ -2231,12 +2231,15 @@ config TEST_HASH> config TEST_IDA> tristate "Perform selftest on IDA functions"> > -config TEST_PARMAN> - tristate "Perform selftest on priority array manager"> - depends on PARMAN> +config PARMAN_KUNIT_TEST> + tristate "Kunit test for priority array manager" if !KUNIT_ALL_TESTS> + select PARMAN> + depends on KUNIT> + default KUNIT_ALL_TESTS> help> - Enable this option to test priority array manager on boot> - (or module load).> + Enable this option to test priority array manager on boot.> + For more information on KUnit and unit tests in general please refer> + to the KUnit documentation in Documentation/dev-tools/kunit/.> > If unsure, say N.> > diff --git a/lib/Makefile b/lib/Makefile> index 5efd1b435a37..deb8946735e8 100644> --- a/lib/Makefile> +++ b/lib/Makefile> @@ -88,7 +88,7 @@ obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o> obj-$(CONFIG_TEST_STRSCPY) += test_strscpy.o> obj-$(CONFIG_TEST_UUID) += test_uuid.o> obj-$(CONFIG_TEST_XARRAY) += test_xarray.o> -obj-$(CONFIG_TEST_PARMAN) += test_parman.o> +obj-$(CONFIG_PARMAN_KUNIT_TEST) += test_parman.o> obj-$(CONFIG_TEST_KMOD) += test_kmod.o> obj-$(CONFIG_TEST_DEBUG_VIRTUAL) += test_debug_virtual.o> obj-$(CONFIG_TEST_MEMCAT_P) += test_memcat_p.o> diff --git a/lib/test_parman.c b/lib/test_parman.c> index 35e32243693c..512f874bc71c 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,41 @@ 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_MSG(test, err, 0, "test_parman_resize failed");> +> + test_parman->parman = parman_create(&test_parman_lsort_ops, test_parman);> + if (IS_ERR_OR_NULL(test_parman->parman)) {> + test_parman_resize(test_parman, 0);> + 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);> }> > static bool test_parman_run_check_budgets(struct test_parman *test_parman)> @@ -265,8 +268,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 +285,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_MSG(test, err, 0, "parman_item_add failed");> +> test_parman->prio_array[item->parman_item.index] = item;> test_parman->used_items++;> } else {> @@ -294,22 +298,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)",> + 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 +319,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");> > 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)",> + 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, i,> + "Item has different index in compare to where it actually is (%lu != %d)",> + 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)",> + used_items, test_parman->used_items);> > - return 0;> + KUNIT_ASSERT_LT_MSG(test, last_unused_items, TEST_PARMAN_RESIZE_STEP_COUNT,> + "Number of unused item at the end of array is bigger than resize step (%u >= %lu)",> + 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>