* [PATCH v3 0/1] Add KUnit tests for llist
@ 2024-09-17 0:51 Artur Alves
2024-09-17 0:51 ` [PATCH v3 1/1] lib/llist_kunit.c: add " Artur Alves
2024-09-19 16:01 ` [PATCH v3 0/1] Add " Shuah Khan
0 siblings, 2 replies; 11+ messages in thread
From: Artur Alves @ 2024-09-17 0:51 UTC (permalink / raw)
To: Andrew Morton, linux-kernel, Brendan Higgins, David Gow, Rae Moar,
linux-kselftest, kunit-dev
Cc: n, andrealmeid, vinicius, diego.daniel.professional
Hi all,
This is part of a hackathon organized by LKCAMP[1], focused on writing
tests using KUnit. We reached out a while ago asking for advice on what
would be a useful contribution[2] and ended up choosing data structures
that did not yet have tests.
This patch adds tests for the llist data structure, defined in
include/linux/llist.h, and is inspired by the KUnit tests for the doubly
linked list in lib/list-test.c[3].
It is important to note that this patch depends on the patch referenced
in [4], as it utilizes the newly created lib/tests/ subdirectory.
[1] https://lkcamp.dev/about/
[2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
[3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
[4] https://lore.kernel.org/all/20240720181025.work.002-kees@kernel.org/
---
Changes in v3:
- Resolved checkpatch warnings:
- Renamed tests for macros starting with 'for_each'
- Removed link from commit message
- Replaced hardcoded constants with ENTRIES_SIZE
- Updated initialization of llist_node array
- Fixed typos
- Update Kconfig.debug message for llist_kunit
Changes in v2:
- Add MODULE_DESCRIPTION()
- Move the tests from lib/llist_kunit.c to lib/tests/llist_kunit.c
- Change the license from "GPL v2" to "GPL"
Artur Alves (1):
lib/llist_kunit.c: add KUnit tests for llist
lib/Kconfig.debug | 11 ++
lib/tests/Makefile | 1 +
lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 370 insertions(+)
create mode 100644 lib/tests/llist_kunit.c
--
2.46.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/1] lib/llist_kunit.c: add KUnit tests for llist
2024-09-17 0:51 [PATCH v3 0/1] Add KUnit tests for llist Artur Alves
@ 2024-09-17 0:51 ` Artur Alves
2024-10-02 20:27 ` Rae Moar
2024-10-03 6:56 ` David Gow
2024-09-19 16:01 ` [PATCH v3 0/1] Add " Shuah Khan
1 sibling, 2 replies; 11+ messages in thread
From: Artur Alves @ 2024-09-17 0:51 UTC (permalink / raw)
To: Andrew Morton, linux-kernel, Brendan Higgins, David Gow, Rae Moar,
linux-kselftest, kunit-dev
Cc: n, andrealmeid, vinicius, diego.daniel.professional
Add KUnit tests for the llist data structure. They test the vast
majority of methods and macros defined in include/linux/llist.h.
These are inspired by the existing tests for the 'list' doubly
linked in lib/list-test.c. Each test case (llist_test_x) tests
the behaviour of the llist function/macro 'x'.
Signed-off-by: Artur Alves <arturacb@gmail.com>
---
lib/Kconfig.debug | 11 ++
lib/tests/Makefile | 1 +
lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 370 insertions(+)
create mode 100644 lib/tests/llist_kunit.c
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index b5696659f027..f6bd98f8ce2b 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2813,6 +2813,17 @@ config USERCOPY_KUNIT_TEST
on the copy_to/from_user infrastructure, making sure basic
user/kernel boundary testing is working.
+config LLIST_KUNIT_TEST
+ tristate "KUnit tests for lib/llist" if !KUNIT_ALL_TESTS
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ This option builds the llist (lock-less list) KUnit test suite.
+ It tests the API and basic functionality of the macros and
+ functions defined in <linux/llist.h>.
+
+ If unsure, say N.
+
config TEST_UDELAY
tristate "udelay test driver"
help
diff --git a/lib/tests/Makefile b/lib/tests/Makefile
index c6a14cc8663e..8d7c40a73110 100644
--- a/lib/tests/Makefile
+++ b/lib/tests/Makefile
@@ -34,4 +34,5 @@ CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable)
obj-$(CONFIG_STACKINIT_KUNIT_TEST) += stackinit_kunit.o
obj-$(CONFIG_STRING_KUNIT_TEST) += string_kunit.o
obj-$(CONFIG_STRING_HELPERS_KUNIT_TEST) += string_helpers_kunit.o
+obj-$(CONFIG_LLIST_KUNIT_TEST) += llist_kunit.o
diff --git a/lib/tests/llist_kunit.c b/lib/tests/llist_kunit.c
new file mode 100644
index 000000000000..817bb2948697
--- /dev/null
+++ b/lib/tests/llist_kunit.c
@@ -0,0 +1,358 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for the Kernel lock-less linked-list structure.
+ *
+ * Author: Artur Alves <arturacb@gmail.com>
+ */
+
+#include <kunit/test.h>
+#include <linux/llist.h>
+
+#define ENTRIES_SIZE 3
+
+struct llist_test_struct {
+ int data;
+ struct llist_node node;
+};
+
+static void llist_test_init_llist(struct kunit *test)
+{
+ /* test if the llist is correctly initialized */
+ struct llist_head llist1 = LLIST_HEAD_INIT(llist1);
+ LLIST_HEAD(llist2);
+ struct llist_head llist3, *llist4, *llist5;
+
+ KUNIT_EXPECT_TRUE(test, llist_empty(&llist1));
+
+ KUNIT_EXPECT_TRUE(test, llist_empty(&llist2));
+
+ init_llist_head(&llist3);
+ KUNIT_EXPECT_TRUE(test, llist_empty(&llist3));
+
+ llist4 = kzalloc(sizeof(*llist4), GFP_KERNEL | __GFP_NOFAIL);
+ init_llist_head(llist4);
+ KUNIT_EXPECT_TRUE(test, llist_empty(llist4));
+ kfree(llist4);
+
+ llist5 = kmalloc(sizeof(*llist5), GFP_KERNEL | __GFP_NOFAIL);
+ memset(llist5, 0xFF, sizeof(*llist5));
+ init_llist_head(llist5);
+ KUNIT_EXPECT_TRUE(test, llist_empty(llist5));
+ kfree(llist5);
+}
+
+static void llist_test_init_llist_node(struct kunit *test)
+{
+ struct llist_node a;
+
+ init_llist_node(&a);
+
+ KUNIT_EXPECT_PTR_EQ(test, a.next, &a);
+}
+
+static void llist_test_llist_entry(struct kunit *test)
+{
+ struct llist_test_struct test_struct, *aux;
+ struct llist_node *llist = &test_struct.node;
+
+ aux = llist_entry(llist, struct llist_test_struct, node);
+ KUNIT_EXPECT_PTR_EQ(test, &test_struct, aux);
+}
+
+static void llist_test_add(struct kunit *test)
+{
+ struct llist_node a, b;
+ LLIST_HEAD(llist);
+
+ init_llist_node(&a);
+ init_llist_node(&b);
+
+ /* The first assertion must be true, given that llist is empty */
+ KUNIT_EXPECT_TRUE(test, llist_add(&a, &llist));
+ KUNIT_EXPECT_FALSE(test, llist_add(&b, &llist));
+
+ /* Should be [List] -> b -> a */
+ KUNIT_EXPECT_PTR_EQ(test, llist.first, &b);
+ KUNIT_EXPECT_PTR_EQ(test, b.next, &a);
+}
+
+static void llist_test_add_batch(struct kunit *test)
+{
+ struct llist_node a, b, c;
+ LLIST_HEAD(llist);
+ LLIST_HEAD(llist2);
+
+ init_llist_node(&a);
+ init_llist_node(&b);
+ init_llist_node(&c);
+
+ llist_add(&a, &llist2);
+ llist_add(&b, &llist2);
+ llist_add(&c, &llist2);
+
+ /* This assertion must be true, given that llist is empty */
+ KUNIT_EXPECT_TRUE(test, llist_add_batch(&c, &a, &llist));
+
+ /* should be [List] -> c -> b -> a */
+ KUNIT_EXPECT_PTR_EQ(test, llist.first, &c);
+ KUNIT_EXPECT_PTR_EQ(test, c.next, &b);
+ KUNIT_EXPECT_PTR_EQ(test, b.next, &a);
+}
+
+static void llist_test_llist_next(struct kunit *test)
+{
+ struct llist_node a, b;
+ LLIST_HEAD(llist);
+
+ init_llist_node(&a);
+ init_llist_node(&b);
+
+ llist_add(&a, &llist);
+ llist_add(&b, &llist);
+
+ /* should be [List] -> b -> a */
+ KUNIT_EXPECT_PTR_EQ(test, llist_next(&b), &a);
+ KUNIT_EXPECT_NULL(test, llist_next(&a));
+}
+
+static void llist_test_empty_llist(struct kunit *test)
+{
+ struct llist_head llist = LLIST_HEAD_INIT(llist);
+ struct llist_node a;
+
+ KUNIT_EXPECT_TRUE(test, llist_empty(&llist));
+
+ llist_add(&a, &llist);
+
+ KUNIT_EXPECT_FALSE(test, llist_empty(&llist));
+}
+
+static void llist_test_llist_on_list(struct kunit *test)
+{
+ struct llist_node a, b;
+ LLIST_HEAD(llist);
+
+ init_llist_node(&a);
+ init_llist_node(&b);
+
+ llist_add(&a, &llist);
+
+ /* should be [List] -> a */
+ KUNIT_EXPECT_TRUE(test, llist_on_list(&a));
+ KUNIT_EXPECT_FALSE(test, llist_on_list(&b));
+}
+
+static void llist_test_del_first(struct kunit *test)
+{
+ struct llist_node a, b, *c;
+ LLIST_HEAD(llist);
+
+ llist_add(&a, &llist);
+ llist_add(&b, &llist);
+
+ /* before: [List] -> b -> a */
+ c = llist_del_first(&llist);
+
+ /* should be [List] -> a */
+ KUNIT_EXPECT_PTR_EQ(test, llist.first, &a);
+
+ /* del must return a pointer to llist_node b
+ * the returned pointer must be marked on list
+ */
+ KUNIT_EXPECT_PTR_EQ(test, c, &b);
+ KUNIT_EXPECT_TRUE(test, llist_on_list(c));
+}
+
+static void llist_test_del_first_init(struct kunit *test)
+{
+ struct llist_node a, *b;
+ LLIST_HEAD(llist);
+
+ llist_add(&a, &llist);
+
+ b = llist_del_first_init(&llist);
+
+ /* should be [List] */
+ KUNIT_EXPECT_TRUE(test, llist_empty(&llist));
+
+ /* the returned pointer must be marked out of the list */
+ KUNIT_EXPECT_FALSE(test, llist_on_list(b));
+}
+
+static void llist_test_del_first_this(struct kunit *test)
+{
+ struct llist_node a, b;
+ LLIST_HEAD(llist);
+
+ llist_add(&a, &llist);
+ llist_add(&b, &llist);
+
+ llist_del_first_this(&llist, &a);
+
+ /* before: [List] -> b -> a */
+
+ // should remove only if is the first node in the llist
+ KUNIT_EXPECT_FALSE(test, llist_del_first_this(&llist, &a));
+
+ KUNIT_EXPECT_TRUE(test, llist_del_first_this(&llist, &b));
+
+ /* should be [List] -> a */
+ KUNIT_EXPECT_PTR_EQ(test, llist.first, &a);
+}
+
+static void llist_test_del_all(struct kunit *test)
+{
+ struct llist_node a, b;
+ LLIST_HEAD(llist);
+ LLIST_HEAD(empty_llist);
+
+ llist_add(&a, &llist);
+ llist_add(&b, &llist);
+
+ /* deleting from a empty llist should return NULL */
+ KUNIT_EXPECT_NULL(test, llist_del_all(&empty_llist));
+
+ llist_del_all(&llist);
+
+ KUNIT_EXPECT_TRUE(test, llist_empty(&llist));
+}
+
+static void llist_test_for_each(struct kunit *test)
+{
+ struct llist_node entries[ENTRIES_SIZE] = { 0 };
+ struct llist_node *pos, *deleted_nodes;
+ LLIST_HEAD(llist);
+ int i = 0, j = 0;
+
+ for (int i = ENTRIES_SIZE - 1; i >= 0; i--)
+ llist_add(&entries[i], &llist);
+
+ /* before [List] -> entries[0] -> ... -> entries[ENTRIES_SIZE - 1] */
+ llist_for_each(pos, llist.first) {
+ KUNIT_EXPECT_PTR_EQ(test, pos, &entries[i++]);
+ }
+
+ KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, i);
+
+ /* traversing deleted nodes */
+ deleted_nodes = llist_del_all(&llist);
+
+ llist_for_each(pos, deleted_nodes) {
+ KUNIT_EXPECT_PTR_EQ(test, pos, &entries[j++]);
+ }
+
+ KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, j);
+}
+
+static void llist_test_safe_for_each(struct kunit *test)
+{
+ struct llist_node entries[ENTRIES_SIZE];
+ struct llist_node *pos, *n;
+ LLIST_HEAD(llist);
+ int i = 0;
+
+ for (int i = ENTRIES_SIZE - 1; i >= 0; i--)
+ llist_add(&entries[i], &llist);
+
+ llist_for_each_safe(pos, n, llist.first) {
+ KUNIT_EXPECT_PTR_EQ(test, pos, &entries[i++]);
+ llist_del_first(&llist);
+ }
+
+ KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, i);
+ KUNIT_EXPECT_TRUE(test, llist_empty(&llist));
+}
+
+static void llist_test_entry_for_each(struct kunit *test)
+{
+ struct llist_test_struct entries[ENTRIES_SIZE], *pos;
+ LLIST_HEAD(llist);
+ int i = 0;
+
+ for (int i = ENTRIES_SIZE - 1; i >= 0; --i) {
+ entries[i].data = i;
+ llist_add(&entries[i].node, &llist);
+ }
+
+ i = 0;
+
+ llist_for_each_entry(pos, llist.first, node) {
+ KUNIT_EXPECT_EQ(test, pos->data, i);
+ i++;
+ }
+
+ KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, i);
+}
+
+static void llist_test_entry_safe_for_each(struct kunit *test)
+{
+ struct llist_test_struct entries[ENTRIES_SIZE], *pos, *n;
+ LLIST_HEAD(llist);
+ int i = 0;
+
+ for (int i = ENTRIES_SIZE - 1; i >= 0; --i) {
+ entries[i].data = i;
+ llist_add(&entries[i].node, &llist);
+ }
+
+ i = 0;
+
+ llist_for_each_entry_safe(pos, n, llist.first, node) {
+ KUNIT_EXPECT_EQ(test, pos->data, i++);
+ llist_del_first(&llist);
+ }
+
+ KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, i);
+ KUNIT_EXPECT_TRUE(test, llist_empty(&llist));
+}
+
+static void llist_test_reverse_order(struct kunit *test)
+{
+ struct llist_node entries[ENTRIES_SIZE], *pos, *reversed_llist;
+ LLIST_HEAD(llist);
+ int i = 0;
+
+ for (int i = 0; i < ENTRIES_SIZE; i++)
+ llist_add(&entries[i], &llist);
+
+ /* before [List] -> entries[2] -> entries[1] -> entries[0] */
+ reversed_llist = llist_reverse_order(llist_del_first(&llist));
+
+ /* should be [List] -> entries[0] -> entries[1] -> entries[2] */
+ llist_for_each(pos, reversed_llist) {
+ KUNIT_EXPECT_PTR_EQ(test, pos, &entries[i++]);
+ }
+
+ KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, i);
+}
+
+static struct kunit_case llist_test_cases[] = {
+ KUNIT_CASE(llist_test_init_llist),
+ KUNIT_CASE(llist_test_init_llist_node),
+ KUNIT_CASE(llist_test_llist_entry),
+ KUNIT_CASE(llist_test_add),
+ KUNIT_CASE(llist_test_add_batch),
+ KUNIT_CASE(llist_test_llist_next),
+ KUNIT_CASE(llist_test_empty_llist),
+ KUNIT_CASE(llist_test_llist_on_list),
+ KUNIT_CASE(llist_test_del_first),
+ KUNIT_CASE(llist_test_del_first_init),
+ KUNIT_CASE(llist_test_del_first_this),
+ KUNIT_CASE(llist_test_del_all),
+ KUNIT_CASE(llist_test_for_each),
+ KUNIT_CASE(llist_test_safe_for_each),
+ KUNIT_CASE(llist_test_entry_for_each),
+ KUNIT_CASE(llist_test_entry_safe_for_each),
+ KUNIT_CASE(llist_test_reverse_order),
+ {}
+};
+
+static struct kunit_suite llist_test_suite = {
+ .name = "llist",
+ .test_cases = llist_test_cases,
+};
+
+kunit_test_suite(llist_test_suite);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("KUnit tests for the llist data structure.");
--
2.46.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/1] Add KUnit tests for llist
2024-09-17 0:51 [PATCH v3 0/1] Add KUnit tests for llist Artur Alves
2024-09-17 0:51 ` [PATCH v3 1/1] lib/llist_kunit.c: add " Artur Alves
@ 2024-09-19 16:01 ` Shuah Khan
2024-09-19 22:27 ` Artur Alves Cavalcante de Barros
2024-09-20 7:10 ` David Gow
1 sibling, 2 replies; 11+ messages in thread
From: Shuah Khan @ 2024-09-19 16:01 UTC (permalink / raw)
To: Artur Alves, Andrew Morton, linux-kernel, Brendan Higgins,
David Gow, Rae Moar, linux-kselftest, kunit-dev
Cc: n, andrealmeid, vinicius, diego.daniel.professional, Shuah Khan
On 9/16/24 18:51, Artur Alves wrote:
> Hi all,
>
> This is part of a hackathon organized by LKCAMP[1], focused on writing
> tests using KUnit. We reached out a while ago asking for advice on what
> would be a useful contribution[2] and ended up choosing data structures
> that did not yet have tests.
>
> This patch adds tests for the llist data structure, defined in
> include/linux/llist.h, and is inspired by the KUnit tests for the doubly
> linked list in lib/list-test.c[3].
>
> It is important to note that this patch depends on the patch referenced
> in [4], as it utilizes the newly created lib/tests/ subdirectory.
>
> [1] https://lkcamp.dev/about/
> [2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
> [3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
> [4] https://lore.kernel.org/all/20240720181025.work.002-kees@kernel.org/
>
> ---
> Changes in v3:
> - Resolved checkpatch warnings:
> - Renamed tests for macros starting with 'for_each'
Shouldn't this a separate patch to make it easy to review?
> - Removed link from commit message
> - Replaced hardcoded constants with ENTRIES_SIZE
Shouldn't this a separate patch to make it easy to review?
> - Updated initialization of llist_node array
> - Fixed typos
> - Update Kconfig.debug message for llist_kunit
Are these changes to existing code or warnings on your added code?
>
> Changes in v2:
> - Add MODULE_DESCRIPTION()
> - Move the tests from lib/llist_kunit.c to lib/tests/llist_kunit.c
> - Change the license from "GPL v2" to "GPL"
>
> Artur Alves (1):
> lib/llist_kunit.c: add KUnit tests for llist
>
> lib/Kconfig.debug | 11 ++
> lib/tests/Makefile | 1 +
> lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 370 insertions(+)
> create mode 100644 lib/tests/llist_kunit.c
>
You are combining lot of changes in one single patch. Each change as a separate
patch will help reviewers.
Adding new test should be a separate patch.
- renaming as a separate patch
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/1] Add KUnit tests for llist
2024-09-19 16:01 ` [PATCH v3 0/1] Add " Shuah Khan
@ 2024-09-19 22:27 ` Artur Alves Cavalcante de Barros
2024-09-20 7:10 ` David Gow
1 sibling, 0 replies; 11+ messages in thread
From: Artur Alves Cavalcante de Barros @ 2024-09-19 22:27 UTC (permalink / raw)
To: Shuah Khan, Andrew Morton, linux-kernel, Brendan Higgins,
David Gow, Rae Moar, linux-kselftest, kunit-dev
Cc: n, andrealmeid, vinicius, diego.daniel.professional
On 9/19/24 1:01 PM, Shuah Khan wrote:
> On 9/16/24 18:51, Artur Alves wrote:
>> Hi all,
>>
>> This is part of a hackathon organized by LKCAMP[1], focused on writing
>> tests using KUnit. We reached out a while ago asking for advice on what
>> would be a useful contribution[2] and ended up choosing data structures
>> that did not yet have tests.
>>
>> This patch adds tests for the llist data structure, defined in
>> include/linux/llist.h, and is inspired by the KUnit tests for the doubly
>> linked list in lib/list-test.c[3].
>>
>> It is important to note that this patch depends on the patch referenced
>> in [4], as it utilizes the newly created lib/tests/ subdirectory.
>>
>> [1] https://lkcamp.dev/about/
>> [2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
>> [3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
>> [4] https://lore.kernel.org/all/20240720181025.work.002-kees@kernel.org/
>>
>> ---
>> Changes in v3:
>> - Resolved checkpatch warnings:
>> - Renamed tests for macros starting with 'for_each'
>
> Shouldn't this a separate patch to make it easy to review?
>
>> - Removed link from commit message
>> - Replaced hardcoded constants with ENTRIES_SIZE
>
> Shouldn't this a separate patch to make it easy to review?
>
>> - Updated initialization of llist_node array
>> - Fixed typos
>> - Update Kconfig.debug message for llist_kunit
>
> Are these changes to existing code or warnings on your added code?
>>
>> Changes in v2:
>> - Add MODULE_DESCRIPTION()
>> - Move the tests from lib/llist_kunit.c to lib/tests/llist_kunit.c
>> - Change the license from "GPL v2" to "GPL"
>>
>> Artur Alves (1):
>> lib/llist_kunit.c: add KUnit tests for llist
>>
>> lib/Kconfig.debug | 11 ++
>> lib/tests/Makefile | 1 +
>> lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 370 insertions(+)
>> create mode 100644 lib/tests/llist_kunit.c
>>
>
> You are combining lot of changes in one single patch. Each change as a
> separate
> patch will help reviewers.
>
> Adding new test should be a separate patch.
>
> - renaming as a separate patch
>
> thanks,
> -- Shuah
Hi, thanks for the reply!
I'm not sure if I understood your concerns ...
In this patch, I'm adding the entire test suite for the lock-less list
data structure, which is the primary reason for its larger size. The
changes in V2 and V3 were made in response to code review suggestions
from previous iterations.
However, as a big patch I see how this cause an annoyance to review. I'm
open to any suggestions on how I can reduce its size or make the review
process more manageable.
Best regards,
- Artur
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/1] Add KUnit tests for llist
2024-09-19 16:01 ` [PATCH v3 0/1] Add " Shuah Khan
2024-09-19 22:27 ` Artur Alves Cavalcante de Barros
@ 2024-09-20 7:10 ` David Gow
2024-09-20 15:10 ` Shuah Khan
2024-09-21 2:49 ` Artur Alves Cavalcante de Barros
1 sibling, 2 replies; 11+ messages in thread
From: David Gow @ 2024-09-20 7:10 UTC (permalink / raw)
To: Shuah Khan
Cc: Artur Alves, Andrew Morton, linux-kernel, Brendan Higgins,
Rae Moar, linux-kselftest, kunit-dev, n, andrealmeid, vinicius,
diego.daniel.professional
[-- Attachment #1: Type: text/plain, Size: 3101 bytes --]
On Fri, 20 Sept 2024 at 00:01, Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 9/16/24 18:51, Artur Alves wrote:
> > Hi all,
> >
> > This is part of a hackathon organized by LKCAMP[1], focused on writing
> > tests using KUnit. We reached out a while ago asking for advice on what
> > would be a useful contribution[2] and ended up choosing data structures
> > that did not yet have tests.
> >
> > This patch adds tests for the llist data structure, defined in
> > include/linux/llist.h, and is inspired by the KUnit tests for the doubly
> > linked list in lib/list-test.c[3].
> >
> > It is important to note that this patch depends on the patch referenced
> > in [4], as it utilizes the newly created lib/tests/ subdirectory.
> >
> > [1] https://lkcamp.dev/about/
> > [2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
> > [3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
> > [4] https://lore.kernel.org/all/20240720181025.work.002-kees@kernel.org/
> >
> > ---
> > Changes in v3:
> > - Resolved checkpatch warnings:
> > - Renamed tests for macros starting with 'for_each'
>
> Shouldn't this a separate patch to make it easy to review?
>
I think that, if this were renaming these in an already existing test
(like the confusingly similar list test), then yes. But since it's
only a change from v2, I think we're okay.
> > - Removed link from commit message
> > - Replaced hardcoded constants with ENTRIES_SIZE
>
> Shouldn't this a separate patch to make it easy to review?
Again, if we want to change this in other tests (list, hlist) we
should split it into a separate patch, but I think it's okay for llist
to go in with these already cleaned up.
>
> > - Updated initialization of llist_node array
> > - Fixed typos
> > - Update Kconfig.debug message for llist_kunit
>
> Are these changes to existing code or warnings on your added code?
I think these are all changes to the added code since v2. Artur, is that right?
> >
> > Changes in v2:
> > - Add MODULE_DESCRIPTION()
> > - Move the tests from lib/llist_kunit.c to lib/tests/llist_kunit.c
> > - Change the license from "GPL v2" to "GPL"
> >
> > Artur Alves (1):
> > lib/llist_kunit.c: add KUnit tests for llist
> >
> > lib/Kconfig.debug | 11 ++
> > lib/tests/Makefile | 1 +
> > lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 370 insertions(+)
> > create mode 100644 lib/tests/llist_kunit.c
> >
>
> You are combining lot of changes in one single patch. Each change as a separate
> patch will help reviewers.
>
> Adding new test should be a separate patch.
>
> - renaming as a separate patch
>
I think given that these are just changes between patch versions, not
renaming/modifying already committed code, that this is okay to go in
as one patch?
The actual patch is only doing one thing: adding a test suite for the
llist structure. I don't see the point in committing a version of it
only to immediately rename things and clean bits up separately in this
case.
Cheers,
-- David
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5294 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/1] Add KUnit tests for llist
2024-09-20 7:10 ` David Gow
@ 2024-09-20 15:10 ` Shuah Khan
2024-09-21 3:07 ` Artur Alves Cavalcante de Barros
2024-09-21 2:49 ` Artur Alves Cavalcante de Barros
1 sibling, 1 reply; 11+ messages in thread
From: Shuah Khan @ 2024-09-20 15:10 UTC (permalink / raw)
To: David Gow
Cc: Artur Alves, Andrew Morton, linux-kernel, Brendan Higgins,
Rae Moar, linux-kselftest, kunit-dev, n, andrealmeid, vinicius,
diego.daniel.professional, Shuah Khan
On 9/20/24 01:10, David Gow wrote:
> On Fri, 20 Sept 2024 at 00:01, Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>> On 9/16/24 18:51, Artur Alves wrote:
>>> Hi all,
>>>
>>> This is part of a hackathon organized by LKCAMP[1], focused on writing
>>> tests using KUnit. We reached out a while ago asking for advice on what
>>> would be a useful contribution[2] and ended up choosing data structures
>>> that did not yet have tests.
>>>
>>> This patch adds tests for the llist data structure, defined in
>>> include/linux/llist.h, and is inspired by the KUnit tests for the doubly
>>> linked list in lib/list-test.c[3].
>>>
>>> It is important to note that this patch depends on the patch referenced
>>> in [4], as it utilizes the newly created lib/tests/ subdirectory.
>>>
>>> [1] https://lkcamp.dev/about/
>>> [2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
>>> [3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
>>> [4] https://lore.kernel.org/all/20240720181025.work.002-kees@kernel.org/
>>>
>>> ---
>>> Changes in v3:
>>> - Resolved checkpatch warnings:
>>> - Renamed tests for macros starting with 'for_each'
>>
>> Shouldn't this a separate patch to make it easy to review?
>>
>
> I think that, if this were renaming these in an already existing test
> (like the confusingly similar list test), then yes. But since it's
> only a change from v2, I think we're okay.
>
>>> - Removed link from commit message
>>> - Replaced hardcoded constants with ENTRIES_SIZE
>>
>> Shouldn't this a separate patch to make it easy to review?
>
> Again, if we want to change this in other tests (list, hlist) we
> should split it into a separate patch, but I think it's okay for llist
> to go in with these already cleaned up.
>
>>
>>> - Updated initialization of llist_node array
>>> - Fixed typos
>>> - Update Kconfig.debug message for llist_kunit
>>
>> Are these changes to existing code or warnings on your added code?
>
> I think these are all changes to the added code since v2. Artur, is that right?
>
>>>
>>> Changes in v2:
>>> - Add MODULE_DESCRIPTION()
>>> - Move the tests from lib/llist_kunit.c to lib/tests/llist_kunit.c
>>> - Change the license from "GPL v2" to "GPL"
>>>
>>> Artur Alves (1):
>>> lib/llist_kunit.c: add KUnit tests for llist
>>>
>>> lib/Kconfig.debug | 11 ++
>>> lib/tests/Makefile | 1 +
>>> lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 370 insertions(+)
>>> create mode 100644 lib/tests/llist_kunit.c
>>>
>>
>> You are combining lot of changes in one single patch. Each change as a separate
>> patch will help reviewers.
>>
>> Adding new test should be a separate patch.
>>
>> - renaming as a separate patch
>>
>
> I think given that these are just changes between patch versions, not
> renaming/modifying already committed code, that this is okay to go in
> as one patch?
>
> The actual patch is only doing one thing: adding a test suite for the
> llist structure. I don't see the point in committing a version of it
> only to immediately rename things and clean bits up separately in this
> case.
I do think it will help to separate the renaming and adding a new test.
It makes it easier to follow.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/1] Add KUnit tests for llist
2024-09-20 7:10 ` David Gow
2024-09-20 15:10 ` Shuah Khan
@ 2024-09-21 2:49 ` Artur Alves Cavalcante de Barros
2024-09-23 15:48 ` Shuah Khan
1 sibling, 1 reply; 11+ messages in thread
From: Artur Alves Cavalcante de Barros @ 2024-09-21 2:49 UTC (permalink / raw)
To: David Gow, Shuah Khan
Cc: Andrew Morton, linux-kernel, Brendan Higgins, Rae Moar,
linux-kselftest, kunit-dev, n, andrealmeid, vinicius,
diego.daniel.professional
On 9/20/24 4:10 AM, David Gow wrote:
> On Fri, 20 Sept 2024 at 00:01, Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>> On 9/16/24 18:51, Artur Alves wrote:
>>> Hi all,
>>>
>>> This is part of a hackathon organized by LKCAMP[1], focused on writing
>>> tests using KUnit. We reached out a while ago asking for advice on what
>>> would be a useful contribution[2] and ended up choosing data structures
>>> that did not yet have tests.
>>>
>>> This patch adds tests for the llist data structure, defined in
>>> include/linux/llist.h, and is inspired by the KUnit tests for the doubly
>>> linked list in lib/list-test.c[3].
>>>
>>> It is important to note that this patch depends on the patch referenced
>>> in [4], as it utilizes the newly created lib/tests/ subdirectory.
>>>
>>> [1] https://lkcamp.dev/about/
>>> [2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
>>> [3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
>>> [4] https://lore.kernel.org/all/20240720181025.work.002-kees@kernel.org/
>>>
>>> ---
>>> Changes in v3:
>>> - Resolved checkpatch warnings:
>>> - Renamed tests for macros starting with 'for_each'
>>
>> Shouldn't this a separate patch to make it easy to review?
>>
>
> I think that, if this were renaming these in an already existing test
> (like the confusingly similar list test), then yes. But since it's
> only a change from v2, I think we're okay.
>
Yes, the renaming refers to some test cases from the test suite that I'm
adding, with the purpose of resolving some checkpatch warnings, as
suggested by Rae Moar's review[1].
>>> - Removed link from commit message
>>> - Replaced hardcoded constants with ENTRIES_SIZE
>>
>> Shouldn't this a separate patch to make it easy to review?
>
> Again, if we want to change this in other tests (list, hlist) we
> should split it into a separate patch, but I think it's okay for llist
> to go in with these already cleaned up.
>
>>
>>> - Updated initialization of llist_node array
>>> - Fixed typos
>>> - Update Kconfig.debug message for llist_kunit
>>
>> Are these changes to existing code or warnings on your added code?
>
> I think these are all changes to the added code since v2. Artur, is that right?
>
This is the case! All changes are in the added code, so it doesn't
introduce any checkpatch warnings that were present in v2.
>>>
>>> Changes in v2:
>>> - Add MODULE_DESCRIPTION()
>>> - Move the tests from lib/llist_kunit.c to lib/tests/llist_kunit.c
>>> - Change the license from "GPL v2" to "GPL"
>>>
>>> Artur Alves (1):
>>> lib/llist_kunit.c: add KUnit tests for llist
>>>
>>> lib/Kconfig.debug | 11 ++
>>> lib/tests/Makefile | 1 +
>>> lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 370 insertions(+)
>>> create mode 100644 lib/tests/llist_kunit.c
>>>
>>
>> You are combining lot of changes in one single patch. Each change as a separate
>> patch will help reviewers.
>>
>> Adding new test should be a separate patch.
>>
>> - renaming as a separate patch
>>
>
> I think given that these are just changes between patch versions, not
> renaming/modifying already committed code, that this is okay to go in
> as one patch?
>
> The actual patch is only doing one thing: adding a test suite for the
> llist structure. I don't see the point in committing a version of it
> only to immediately rename things and clean bits up separately in this
> case.
>
>
> Cheers,
> -- David
Thanks for replying!
I'd like to reaffirm that the patch is, in fact, doing one thing: adding
tests for the llist data structure. All the changes in V2 and V3 refer
to the code that I'm adding. I'm not modifying any existing list tests,
only adding new ones.
[1]
https://lore.kernel.org/all/20240903214027.77533-1-arturacb@gmail.com/T/#mc29a53b120d2f8589f8bd882ab972d15c8a3d202
Best regards,
- Artur
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/1] Add KUnit tests for llist
2024-09-20 15:10 ` Shuah Khan
@ 2024-09-21 3:07 ` Artur Alves Cavalcante de Barros
0 siblings, 0 replies; 11+ messages in thread
From: Artur Alves Cavalcante de Barros @ 2024-09-21 3:07 UTC (permalink / raw)
To: Shuah Khan, David Gow
Cc: Andrew Morton, linux-kernel, Brendan Higgins, Rae Moar,
linux-kselftest, kunit-dev, n, andrealmeid, vinicius,
diego.daniel.professional
On 9/20/24 12:10 PM, Shuah Khan wrote:
> On 9/20/24 01:10, David Gow wrote:
>> On Fri, 20 Sept 2024 at 00:01, Shuah Khan <skhan@linuxfoundation.org>
>> wrote:
>>>
>>> On 9/16/24 18:51, Artur Alves wrote:
>>>> Hi all,
>>>>
>>>> This is part of a hackathon organized by LKCAMP[1], focused on writing
>>>> tests using KUnit. We reached out a while ago asking for advice on what
>>>> would be a useful contribution[2] and ended up choosing data structures
>>>> that did not yet have tests.
>>>>
>>>> This patch adds tests for the llist data structure, defined in
>>>> include/linux/llist.h, and is inspired by the KUnit tests for the
>>>> doubly
>>>> linked list in lib/list-test.c[3].
>>>>
>>>> It is important to note that this patch depends on the patch referenced
>>>> in [4], as it utilizes the newly created lib/tests/ subdirectory.
>>>>
>>>> [1] https://lkcamp.dev/about/
>>>> [2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
>>>> [3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
>>>> [4] https://lore.kernel.org/all/20240720181025.work.002-
>>>> kees@kernel.org/
>>>>
>>>> ---
>>>> Changes in v3:
>>>> - Resolved checkpatch warnings:
>>>> - Renamed tests for macros starting with 'for_each'
>>>
>>> Shouldn't this a separate patch to make it easy to review?
>>>
>>
>> I think that, if this were renaming these in an already existing test
>> (like the confusingly similar list test), then yes. But since it's
>> only a change from v2, I think we're okay.
>>
>>>> - Removed link from commit message
>>>> - Replaced hardcoded constants with ENTRIES_SIZE
>>>
>>> Shouldn't this a separate patch to make it easy to review?
>>
>> Again, if we want to change this in other tests (list, hlist) we
>> should split it into a separate patch, but I think it's okay for llist
>> to go in with these already cleaned up.
>>
>>>
>>>> - Updated initialization of llist_node array
>>>> - Fixed typos
>>>> - Update Kconfig.debug message for llist_kunit
>>>
>>> Are these changes to existing code or warnings on your added code?
>>
>> I think these are all changes to the added code since v2. Artur, is
>> that right?
>>
>>>>
>>>> Changes in v2:
>>>> - Add MODULE_DESCRIPTION()
>>>> - Move the tests from lib/llist_kunit.c to lib/tests/
>>>> llist_kunit.c
>>>> - Change the license from "GPL v2" to "GPL"
>>>>
>>>> Artur Alves (1):
>>>> lib/llist_kunit.c: add KUnit tests for llist
>>>>
>>>> lib/Kconfig.debug | 11 ++
>>>> lib/tests/Makefile | 1 +
>>>> lib/tests/llist_kunit.c | 358 +++++++++++++++++++++++++++++++++++
>>>> +++++
>>>> 3 files changed, 370 insertions(+)
>>>> create mode 100644 lib/tests/llist_kunit.c
>>>>
>>>
>>> You are combining lot of changes in one single patch. Each change as
>>> a separate
>>> patch will help reviewers.
>>>
>>> Adding new test should be a separate patch.
>>>
>>> - renaming as a separate patch
>>>
>>
>> I think given that these are just changes between patch versions, not
>> renaming/modifying already committed code, that this is okay to go in
>> as one patch?
>>
>> The actual patch is only doing one thing: adding a test suite for the
>> llist structure. I don't see the point in committing a version of it
>> only to immediately rename things and clean bits up separately in this
>> case.
>
> I do think it will help to separate the renaming and adding a new test.
> It makes it easier to follow.
>
> thanks,
> -- Shuah
>
Hi, Shuah!
The renaming is in the test suite that I'm adding, as suggested by Rae
Moar[1]. I'm not modifying any existing code; all my changes are in the
new code that I'm adding.
I'm sorry, but it isn't clear to me. Could you please provide an example
of what you're suggesting?
Would you prefer that I undo the renaming, submit the patch with the
checkpatch warnings, and then follow up with a new patch to rename the
test cases and resolve the warnings?
[1]
https://lore.kernel.org/all/20240903214027.77533-1-arturacb@gmail.com/T/#mc29a53b120d2f8589f8bd882ab972d15c8a3d202
Thanks for your time,
- Artur
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/1] Add KUnit tests for llist
2024-09-21 2:49 ` Artur Alves Cavalcante de Barros
@ 2024-09-23 15:48 ` Shuah Khan
0 siblings, 0 replies; 11+ messages in thread
From: Shuah Khan @ 2024-09-23 15:48 UTC (permalink / raw)
To: Artur Alves Cavalcante de Barros, David Gow
Cc: Andrew Morton, linux-kernel, Brendan Higgins, Rae Moar,
linux-kselftest, kunit-dev, n, andrealmeid, vinicius,
diego.daniel.professional, Shuah Khan
On 9/20/24 20:49, Artur Alves Cavalcante de Barros wrote:
> On 9/20/24 4:10 AM, David Gow wrote:
>> On Fri, 20 Sept 2024 at 00:01, Shuah Khan <skhan@linuxfoundation.org> wrote:
>>>
>>> On 9/16/24 18:51, Artur Alves wrote:
>>>> Hi all,
>>>>
>>>> This is part of a hackathon organized by LKCAMP[1], focused on writing
>>>> tests using KUnit. We reached out a while ago asking for advice on what
>>>> would be a useful contribution[2] and ended up choosing data structures
>>>> that did not yet have tests.
>>>>
>>>> This patch adds tests for the llist data structure, defined in
>>>> include/linux/llist.h, and is inspired by the KUnit tests for the doubly
>>>> linked list in lib/list-test.c[3].
>>>>
>>>> It is important to note that this patch depends on the patch referenced
>>>> in [4], as it utilizes the newly created lib/tests/ subdirectory.
>>>>
>>>> [1] https://lkcamp.dev/about/
>>>> [2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
>>>> [3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
>>>> [4] https://lore.kernel.org/all/20240720181025.work.002-kees@kernel.org/
>>>>
>>>> ---
>>>> Changes in v3:
>>>> - Resolved checkpatch warnings:
>>>> - Renamed tests for macros starting with 'for_each'
>>>
>>> Shouldn't this a separate patch to make it easy to review?
>>>
>>
>> I think that, if this were renaming these in an already existing test
>> (like the confusingly similar list test), then yes. But since it's
>> only a change from v2, I think we're okay.
>>
>
> Yes, the renaming refers to some test cases from the test suite that I'm adding, with the purpose of resolving some checkpatch warnings, as suggested by Rae Moar's review[1].
>
>>>> - Removed link from commit message
>>>> - Replaced hardcoded constants with ENTRIES_SIZE
>>>
>>> Shouldn't this a separate patch to make it easy to review?
>>
>> Again, if we want to change this in other tests (list, hlist) we
>> should split it into a separate patch, but I think it's okay for llist
>> to go in with these already cleaned up.
>>
>>>
>>>> - Updated initialization of llist_node array
>>>> - Fixed typos
>>>> - Update Kconfig.debug message for llist_kunit
>>>
>>> Are these changes to existing code or warnings on your added code?
>>
>> I think these are all changes to the added code since v2. Artur, is that right?
>>
>
> This is the case! All changes are in the added code, so it doesn't introduce any checkpatch warnings that were present in v2.
>
>>>>
>>>> Changes in v2:
>>>> - Add MODULE_DESCRIPTION()
>>>> - Move the tests from lib/llist_kunit.c to lib/tests/llist_kunit.c
>>>> - Change the license from "GPL v2" to "GPL"
>>>>
>>>> Artur Alves (1):
>>>> lib/llist_kunit.c: add KUnit tests for llist
>>>>
>>>> lib/Kconfig.debug | 11 ++
>>>> lib/tests/Makefile | 1 +
>>>> lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 370 insertions(+)
>>>> create mode 100644 lib/tests/llist_kunit.c
>>>>
>>>
>>> You are combining lot of changes in one single patch. Each change as a separate
>>> patch will help reviewers.
>>>
>>> Adding new test should be a separate patch.
>>>
>>> - renaming as a separate patch
>>>
>>
>> I think given that these are just changes between patch versions, not
>> renaming/modifying already committed code, that this is okay to go in
>> as one patch?
>>
>> The actual patch is only doing one thing: adding a test suite for the
>> llist structure. I don't see the point in committing a version of it
>> only to immediately rename things and clean bits up separately in this
>> case.
>>
>>
>> Cheers,
>> -- David
>
> Thanks for replying!
>
> I'd like to reaffirm that the patch is, in fact, doing one thing: adding tests for the llist data structure. All the changes in V2 and V3 refer to the code that I'm adding. I'm not modifying any existing list tests, only adding new ones.
>
> [1] https://lore.kernel.org/all/20240903214027.77533-1-arturacb@gmail.com/T/#mc29a53b120d2f8589f8bd882ab972d15c8a3d202
>
> Best regards,
> - Artur
Sounds good to me. It was a bit confusing because the v2 and v3 change
new and code which wasn't clear to me.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/1] lib/llist_kunit.c: add KUnit tests for llist
2024-09-17 0:51 ` [PATCH v3 1/1] lib/llist_kunit.c: add " Artur Alves
@ 2024-10-02 20:27 ` Rae Moar
2024-10-03 6:56 ` David Gow
1 sibling, 0 replies; 11+ messages in thread
From: Rae Moar @ 2024-10-02 20:27 UTC (permalink / raw)
To: Artur Alves
Cc: Andrew Morton, linux-kernel, Brendan Higgins, David Gow,
linux-kselftest, kunit-dev, n, andrealmeid, vinicius,
diego.daniel.professional
On Mon, Sep 16, 2024 at 8:51 PM Artur Alves <arturacb@gmail.com> wrote:
>
> Add KUnit tests for the llist data structure. They test the vast
> majority of methods and macros defined in include/linux/llist.h.
>
> These are inspired by the existing tests for the 'list' doubly
> linked in lib/list-test.c. Each test case (llist_test_x) tests
> the behaviour of the llist function/macro 'x'.
>
> Signed-off-by: Artur Alves <arturacb@gmail.com>
Hello!
Sorry for the delay in getting back to you. We have been busy with
LPC. This looks good to me!
I just have one comment left: the patch to add the lib/tests directory
hasn't landed in the kselftest/kunit branch so this patch doesn't
apply cleanly. Since we are planning on taking this patch through that
branch, could you switch the files to be lib/Makefile and
lib/llist_kunit.c for now. And we can move them in the great migration
later.
But otherwise,
Reviewed-by: Rae Moar <rmoar@google.com>
Thanks!
Rae
> ---
> lib/Kconfig.debug | 11 ++
> lib/tests/Makefile | 1 +
> lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 370 insertions(+)
> create mode 100644 lib/tests/llist_kunit.c
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index b5696659f027..f6bd98f8ce2b 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2813,6 +2813,17 @@ config USERCOPY_KUNIT_TEST
> on the copy_to/from_user infrastructure, making sure basic
> user/kernel boundary testing is working.
>
> +config LLIST_KUNIT_TEST
> + tristate "KUnit tests for lib/llist" if !KUNIT_ALL_TESTS
> + depends on KUNIT
> + default KUNIT_ALL_TESTS
> + help
> + This option builds the llist (lock-less list) KUnit test suite.
> + It tests the API and basic functionality of the macros and
> + functions defined in <linux/llist.h>.
> +
> + If unsure, say N.
> +
> config TEST_UDELAY
> tristate "udelay test driver"
> help
> diff --git a/lib/tests/Makefile b/lib/tests/Makefile
> index c6a14cc8663e..8d7c40a73110 100644
> --- a/lib/tests/Makefile
> +++ b/lib/tests/Makefile
> @@ -34,4 +34,5 @@ CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable)
> obj-$(CONFIG_STACKINIT_KUNIT_TEST) += stackinit_kunit.o
> obj-$(CONFIG_STRING_KUNIT_TEST) += string_kunit.o
> obj-$(CONFIG_STRING_HELPERS_KUNIT_TEST) += string_helpers_kunit.o
> +obj-$(CONFIG_LLIST_KUNIT_TEST) += llist_kunit.o
>
> diff --git a/lib/tests/llist_kunit.c b/lib/tests/llist_kunit.c
> new file mode 100644
> index 000000000000..817bb2948697
> --- /dev/null
> +++ b/lib/tests/llist_kunit.c
> @@ -0,0 +1,358 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for the Kernel lock-less linked-list structure.
> + *
> + * Author: Artur Alves <arturacb@gmail.com>
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/llist.h>
> +
> +#define ENTRIES_SIZE 3
> +
> +struct llist_test_struct {
> + int data;
> + struct llist_node node;
> +};
> +
> +static void llist_test_init_llist(struct kunit *test)
> +{
> + /* test if the llist is correctly initialized */
> + struct llist_head llist1 = LLIST_HEAD_INIT(llist1);
> + LLIST_HEAD(llist2);
> + struct llist_head llist3, *llist4, *llist5;
> +
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist1));
> +
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist2));
> +
> + init_llist_head(&llist3);
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist3));
> +
> + llist4 = kzalloc(sizeof(*llist4), GFP_KERNEL | __GFP_NOFAIL);
> + init_llist_head(llist4);
> + KUNIT_EXPECT_TRUE(test, llist_empty(llist4));
> + kfree(llist4);
> +
> + llist5 = kmalloc(sizeof(*llist5), GFP_KERNEL | __GFP_NOFAIL);
> + memset(llist5, 0xFF, sizeof(*llist5));
> + init_llist_head(llist5);
> + KUNIT_EXPECT_TRUE(test, llist_empty(llist5));
> + kfree(llist5);
> +}
> +
> +static void llist_test_init_llist_node(struct kunit *test)
> +{
> + struct llist_node a;
> +
> + init_llist_node(&a);
> +
> + KUNIT_EXPECT_PTR_EQ(test, a.next, &a);
> +}
> +
> +static void llist_test_llist_entry(struct kunit *test)
> +{
> + struct llist_test_struct test_struct, *aux;
> + struct llist_node *llist = &test_struct.node;
> +
> + aux = llist_entry(llist, struct llist_test_struct, node);
> + KUNIT_EXPECT_PTR_EQ(test, &test_struct, aux);
> +}
> +
> +static void llist_test_add(struct kunit *test)
> +{
> + struct llist_node a, b;
> + LLIST_HEAD(llist);
> +
> + init_llist_node(&a);
> + init_llist_node(&b);
> +
> + /* The first assertion must be true, given that llist is empty */
> + KUNIT_EXPECT_TRUE(test, llist_add(&a, &llist));
> + KUNIT_EXPECT_FALSE(test, llist_add(&b, &llist));
> +
> + /* Should be [List] -> b -> a */
> + KUNIT_EXPECT_PTR_EQ(test, llist.first, &b);
> + KUNIT_EXPECT_PTR_EQ(test, b.next, &a);
> +}
> +
> +static void llist_test_add_batch(struct kunit *test)
> +{
> + struct llist_node a, b, c;
> + LLIST_HEAD(llist);
> + LLIST_HEAD(llist2);
> +
> + init_llist_node(&a);
> + init_llist_node(&b);
> + init_llist_node(&c);
> +
> + llist_add(&a, &llist2);
> + llist_add(&b, &llist2);
> + llist_add(&c, &llist2);
> +
> + /* This assertion must be true, given that llist is empty */
> + KUNIT_EXPECT_TRUE(test, llist_add_batch(&c, &a, &llist));
> +
> + /* should be [List] -> c -> b -> a */
> + KUNIT_EXPECT_PTR_EQ(test, llist.first, &c);
> + KUNIT_EXPECT_PTR_EQ(test, c.next, &b);
> + KUNIT_EXPECT_PTR_EQ(test, b.next, &a);
> +}
> +
> +static void llist_test_llist_next(struct kunit *test)
> +{
> + struct llist_node a, b;
> + LLIST_HEAD(llist);
> +
> + init_llist_node(&a);
> + init_llist_node(&b);
> +
> + llist_add(&a, &llist);
> + llist_add(&b, &llist);
> +
> + /* should be [List] -> b -> a */
> + KUNIT_EXPECT_PTR_EQ(test, llist_next(&b), &a);
> + KUNIT_EXPECT_NULL(test, llist_next(&a));
> +}
> +
> +static void llist_test_empty_llist(struct kunit *test)
> +{
> + struct llist_head llist = LLIST_HEAD_INIT(llist);
> + struct llist_node a;
> +
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist));
> +
> + llist_add(&a, &llist);
> +
> + KUNIT_EXPECT_FALSE(test, llist_empty(&llist));
> +}
> +
> +static void llist_test_llist_on_list(struct kunit *test)
> +{
> + struct llist_node a, b;
> + LLIST_HEAD(llist);
> +
> + init_llist_node(&a);
> + init_llist_node(&b);
> +
> + llist_add(&a, &llist);
> +
> + /* should be [List] -> a */
> + KUNIT_EXPECT_TRUE(test, llist_on_list(&a));
> + KUNIT_EXPECT_FALSE(test, llist_on_list(&b));
> +}
> +
> +static void llist_test_del_first(struct kunit *test)
> +{
> + struct llist_node a, b, *c;
> + LLIST_HEAD(llist);
> +
> + llist_add(&a, &llist);
> + llist_add(&b, &llist);
> +
> + /* before: [List] -> b -> a */
> + c = llist_del_first(&llist);
> +
> + /* should be [List] -> a */
> + KUNIT_EXPECT_PTR_EQ(test, llist.first, &a);
> +
> + /* del must return a pointer to llist_node b
> + * the returned pointer must be marked on list
> + */
> + KUNIT_EXPECT_PTR_EQ(test, c, &b);
> + KUNIT_EXPECT_TRUE(test, llist_on_list(c));
> +}
> +
> +static void llist_test_del_first_init(struct kunit *test)
> +{
> + struct llist_node a, *b;
> + LLIST_HEAD(llist);
> +
> + llist_add(&a, &llist);
> +
> + b = llist_del_first_init(&llist);
> +
> + /* should be [List] */
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist));
> +
> + /* the returned pointer must be marked out of the list */
> + KUNIT_EXPECT_FALSE(test, llist_on_list(b));
> +}
> +
> +static void llist_test_del_first_this(struct kunit *test)
> +{
> + struct llist_node a, b;
> + LLIST_HEAD(llist);
> +
> + llist_add(&a, &llist);
> + llist_add(&b, &llist);
> +
> + llist_del_first_this(&llist, &a);
> +
> + /* before: [List] -> b -> a */
> +
> + // should remove only if is the first node in the llist
> + KUNIT_EXPECT_FALSE(test, llist_del_first_this(&llist, &a));
> +
> + KUNIT_EXPECT_TRUE(test, llist_del_first_this(&llist, &b));
> +
> + /* should be [List] -> a */
> + KUNIT_EXPECT_PTR_EQ(test, llist.first, &a);
> +}
> +
> +static void llist_test_del_all(struct kunit *test)
> +{
> + struct llist_node a, b;
> + LLIST_HEAD(llist);
> + LLIST_HEAD(empty_llist);
> +
> + llist_add(&a, &llist);
> + llist_add(&b, &llist);
> +
> + /* deleting from a empty llist should return NULL */
> + KUNIT_EXPECT_NULL(test, llist_del_all(&empty_llist));
> +
> + llist_del_all(&llist);
> +
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist));
> +}
> +
> +static void llist_test_for_each(struct kunit *test)
> +{
> + struct llist_node entries[ENTRIES_SIZE] = { 0 };
> + struct llist_node *pos, *deleted_nodes;
> + LLIST_HEAD(llist);
> + int i = 0, j = 0;
> +
> + for (int i = ENTRIES_SIZE - 1; i >= 0; i--)
> + llist_add(&entries[i], &llist);
> +
> + /* before [List] -> entries[0] -> ... -> entries[ENTRIES_SIZE - 1] */
> + llist_for_each(pos, llist.first) {
> + KUNIT_EXPECT_PTR_EQ(test, pos, &entries[i++]);
> + }
> +
> + KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, i);
> +
> + /* traversing deleted nodes */
> + deleted_nodes = llist_del_all(&llist);
> +
> + llist_for_each(pos, deleted_nodes) {
> + KUNIT_EXPECT_PTR_EQ(test, pos, &entries[j++]);
> + }
> +
> + KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, j);
> +}
> +
> +static void llist_test_safe_for_each(struct kunit *test)
> +{
> + struct llist_node entries[ENTRIES_SIZE];
> + struct llist_node *pos, *n;
> + LLIST_HEAD(llist);
> + int i = 0;
> +
> + for (int i = ENTRIES_SIZE - 1; i >= 0; i--)
> + llist_add(&entries[i], &llist);
> +
> + llist_for_each_safe(pos, n, llist.first) {
> + KUNIT_EXPECT_PTR_EQ(test, pos, &entries[i++]);
> + llist_del_first(&llist);
> + }
> +
> + KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, i);
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist));
> +}
> +
> +static void llist_test_entry_for_each(struct kunit *test)
> +{
> + struct llist_test_struct entries[ENTRIES_SIZE], *pos;
> + LLIST_HEAD(llist);
> + int i = 0;
> +
> + for (int i = ENTRIES_SIZE - 1; i >= 0; --i) {
> + entries[i].data = i;
> + llist_add(&entries[i].node, &llist);
> + }
> +
> + i = 0;
> +
> + llist_for_each_entry(pos, llist.first, node) {
> + KUNIT_EXPECT_EQ(test, pos->data, i);
> + i++;
> + }
> +
> + KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, i);
> +}
> +
> +static void llist_test_entry_safe_for_each(struct kunit *test)
> +{
> + struct llist_test_struct entries[ENTRIES_SIZE], *pos, *n;
> + LLIST_HEAD(llist);
> + int i = 0;
> +
> + for (int i = ENTRIES_SIZE - 1; i >= 0; --i) {
> + entries[i].data = i;
> + llist_add(&entries[i].node, &llist);
> + }
> +
> + i = 0;
> +
> + llist_for_each_entry_safe(pos, n, llist.first, node) {
> + KUNIT_EXPECT_EQ(test, pos->data, i++);
> + llist_del_first(&llist);
> + }
> +
> + KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, i);
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist));
> +}
> +
> +static void llist_test_reverse_order(struct kunit *test)
> +{
> + struct llist_node entries[ENTRIES_SIZE], *pos, *reversed_llist;
> + LLIST_HEAD(llist);
> + int i = 0;
> +
> + for (int i = 0; i < ENTRIES_SIZE; i++)
> + llist_add(&entries[i], &llist);
> +
> + /* before [List] -> entries[2] -> entries[1] -> entries[0] */
> + reversed_llist = llist_reverse_order(llist_del_first(&llist));
> +
> + /* should be [List] -> entries[0] -> entries[1] -> entries[2] */
> + llist_for_each(pos, reversed_llist) {
> + KUNIT_EXPECT_PTR_EQ(test, pos, &entries[i++]);
> + }
> +
> + KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, i);
> +}
> +
> +static struct kunit_case llist_test_cases[] = {
> + KUNIT_CASE(llist_test_init_llist),
> + KUNIT_CASE(llist_test_init_llist_node),
> + KUNIT_CASE(llist_test_llist_entry),
> + KUNIT_CASE(llist_test_add),
> + KUNIT_CASE(llist_test_add_batch),
> + KUNIT_CASE(llist_test_llist_next),
> + KUNIT_CASE(llist_test_empty_llist),
> + KUNIT_CASE(llist_test_llist_on_list),
> + KUNIT_CASE(llist_test_del_first),
> + KUNIT_CASE(llist_test_del_first_init),
> + KUNIT_CASE(llist_test_del_first_this),
> + KUNIT_CASE(llist_test_del_all),
> + KUNIT_CASE(llist_test_for_each),
> + KUNIT_CASE(llist_test_safe_for_each),
> + KUNIT_CASE(llist_test_entry_for_each),
> + KUNIT_CASE(llist_test_entry_safe_for_each),
> + KUNIT_CASE(llist_test_reverse_order),
> + {}
> +};
> +
> +static struct kunit_suite llist_test_suite = {
> + .name = "llist",
> + .test_cases = llist_test_cases,
> +};
> +
> +kunit_test_suite(llist_test_suite);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("KUnit tests for the llist data structure.");
> --
> 2.46.0
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20240917005116.304090-2-arturacb%40gmail.com.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/1] lib/llist_kunit.c: add KUnit tests for llist
2024-09-17 0:51 ` [PATCH v3 1/1] lib/llist_kunit.c: add " Artur Alves
2024-10-02 20:27 ` Rae Moar
@ 2024-10-03 6:56 ` David Gow
1 sibling, 0 replies; 11+ messages in thread
From: David Gow @ 2024-10-03 6:56 UTC (permalink / raw)
To: Artur Alves
Cc: Andrew Morton, linux-kernel, Brendan Higgins, Rae Moar,
linux-kselftest, kunit-dev, n, andrealmeid, vinicius,
diego.daniel.professional
[-- Attachment #1: Type: text/plain, Size: 13530 bytes --]
On Tue, 17 Sept 2024 at 08:51, Artur Alves <arturacb@gmail.com> wrote:
>
> Add KUnit tests for the llist data structure. They test the vast
> majority of methods and macros defined in include/linux/llist.h.
>
> These are inspired by the existing tests for the 'list' doubly
> linked in lib/list-test.c. Each test case (llist_test_x) tests
> the behaviour of the llist function/macro 'x'.
>
> Signed-off-by: Artur Alves <arturacb@gmail.com>
> ---
Always nice to see more list tests!
Acked-by: David Gow <davidgow@google.com>
Cheers,
-- David
> lib/Kconfig.debug | 11 ++
> lib/tests/Makefile | 1 +
> lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 370 insertions(+)
> create mode 100644 lib/tests/llist_kunit.c
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index b5696659f027..f6bd98f8ce2b 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2813,6 +2813,17 @@ config USERCOPY_KUNIT_TEST
> on the copy_to/from_user infrastructure, making sure basic
> user/kernel boundary testing is working.
>
> +config LLIST_KUNIT_TEST
> + tristate "KUnit tests for lib/llist" if !KUNIT_ALL_TESTS
> + depends on KUNIT
> + default KUNIT_ALL_TESTS
> + help
> + This option builds the llist (lock-less list) KUnit test suite.
> + It tests the API and basic functionality of the macros and
> + functions defined in <linux/llist.h>.
> +
> + If unsure, say N.
> +
> config TEST_UDELAY
> tristate "udelay test driver"
> help
> diff --git a/lib/tests/Makefile b/lib/tests/Makefile
> index c6a14cc8663e..8d7c40a73110 100644
> --- a/lib/tests/Makefile
> +++ b/lib/tests/Makefile
> @@ -34,4 +34,5 @@ CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable)
> obj-$(CONFIG_STACKINIT_KUNIT_TEST) += stackinit_kunit.o
> obj-$(CONFIG_STRING_KUNIT_TEST) += string_kunit.o
> obj-$(CONFIG_STRING_HELPERS_KUNIT_TEST) += string_helpers_kunit.o
> +obj-$(CONFIG_LLIST_KUNIT_TEST) += llist_kunit.o
>
> diff --git a/lib/tests/llist_kunit.c b/lib/tests/llist_kunit.c
> new file mode 100644
> index 000000000000..817bb2948697
> --- /dev/null
> +++ b/lib/tests/llist_kunit.c
> @@ -0,0 +1,358 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for the Kernel lock-less linked-list structure.
> + *
> + * Author: Artur Alves <arturacb@gmail.com>
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/llist.h>
> +
> +#define ENTRIES_SIZE 3
> +
> +struct llist_test_struct {
> + int data;
> + struct llist_node node;
> +};
> +
> +static void llist_test_init_llist(struct kunit *test)
> +{
> + /* test if the llist is correctly initialized */
> + struct llist_head llist1 = LLIST_HEAD_INIT(llist1);
> + LLIST_HEAD(llist2);
> + struct llist_head llist3, *llist4, *llist5;
> +
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist1));
> +
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist2));
> +
> + init_llist_head(&llist3);
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist3));
> +
> + llist4 = kzalloc(sizeof(*llist4), GFP_KERNEL | __GFP_NOFAIL);
> + init_llist_head(llist4);
> + KUNIT_EXPECT_TRUE(test, llist_empty(llist4));
> + kfree(llist4);
> +
> + llist5 = kmalloc(sizeof(*llist5), GFP_KERNEL | __GFP_NOFAIL);
> + memset(llist5, 0xFF, sizeof(*llist5));
> + init_llist_head(llist5);
> + KUNIT_EXPECT_TRUE(test, llist_empty(llist5));
> + kfree(llist5);
> +}
> +
> +static void llist_test_init_llist_node(struct kunit *test)
> +{
> + struct llist_node a;
> +
> + init_llist_node(&a);
> +
> + KUNIT_EXPECT_PTR_EQ(test, a.next, &a);
> +}
> +
> +static void llist_test_llist_entry(struct kunit *test)
> +{
> + struct llist_test_struct test_struct, *aux;
> + struct llist_node *llist = &test_struct.node;
> +
> + aux = llist_entry(llist, struct llist_test_struct, node);
> + KUNIT_EXPECT_PTR_EQ(test, &test_struct, aux);
> +}
> +
> +static void llist_test_add(struct kunit *test)
> +{
> + struct llist_node a, b;
> + LLIST_HEAD(llist);
> +
> + init_llist_node(&a);
> + init_llist_node(&b);
> +
> + /* The first assertion must be true, given that llist is empty */
> + KUNIT_EXPECT_TRUE(test, llist_add(&a, &llist));
> + KUNIT_EXPECT_FALSE(test, llist_add(&b, &llist));
> +
> + /* Should be [List] -> b -> a */
> + KUNIT_EXPECT_PTR_EQ(test, llist.first, &b);
> + KUNIT_EXPECT_PTR_EQ(test, b.next, &a);
> +}
> +
> +static void llist_test_add_batch(struct kunit *test)
> +{
> + struct llist_node a, b, c;
> + LLIST_HEAD(llist);
> + LLIST_HEAD(llist2);
> +
> + init_llist_node(&a);
> + init_llist_node(&b);
> + init_llist_node(&c);
> +
> + llist_add(&a, &llist2);
> + llist_add(&b, &llist2);
> + llist_add(&c, &llist2);
> +
> + /* This assertion must be true, given that llist is empty */
> + KUNIT_EXPECT_TRUE(test, llist_add_batch(&c, &a, &llist));
> +
> + /* should be [List] -> c -> b -> a */
> + KUNIT_EXPECT_PTR_EQ(test, llist.first, &c);
> + KUNIT_EXPECT_PTR_EQ(test, c.next, &b);
> + KUNIT_EXPECT_PTR_EQ(test, b.next, &a);
> +}
> +
> +static void llist_test_llist_next(struct kunit *test)
> +{
> + struct llist_node a, b;
> + LLIST_HEAD(llist);
> +
> + init_llist_node(&a);
> + init_llist_node(&b);
> +
> + llist_add(&a, &llist);
> + llist_add(&b, &llist);
> +
> + /* should be [List] -> b -> a */
> + KUNIT_EXPECT_PTR_EQ(test, llist_next(&b), &a);
> + KUNIT_EXPECT_NULL(test, llist_next(&a));
> +}
> +
> +static void llist_test_empty_llist(struct kunit *test)
> +{
> + struct llist_head llist = LLIST_HEAD_INIT(llist);
> + struct llist_node a;
> +
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist));
> +
> + llist_add(&a, &llist);
> +
> + KUNIT_EXPECT_FALSE(test, llist_empty(&llist));
> +}
> +
> +static void llist_test_llist_on_list(struct kunit *test)
> +{
> + struct llist_node a, b;
> + LLIST_HEAD(llist);
> +
> + init_llist_node(&a);
> + init_llist_node(&b);
> +
> + llist_add(&a, &llist);
> +
> + /* should be [List] -> a */
> + KUNIT_EXPECT_TRUE(test, llist_on_list(&a));
> + KUNIT_EXPECT_FALSE(test, llist_on_list(&b));
> +}
> +
> +static void llist_test_del_first(struct kunit *test)
> +{
> + struct llist_node a, b, *c;
> + LLIST_HEAD(llist);
> +
> + llist_add(&a, &llist);
> + llist_add(&b, &llist);
> +
> + /* before: [List] -> b -> a */
> + c = llist_del_first(&llist);
> +
> + /* should be [List] -> a */
> + KUNIT_EXPECT_PTR_EQ(test, llist.first, &a);
> +
> + /* del must return a pointer to llist_node b
> + * the returned pointer must be marked on list
> + */
> + KUNIT_EXPECT_PTR_EQ(test, c, &b);
> + KUNIT_EXPECT_TRUE(test, llist_on_list(c));
> +}
> +
> +static void llist_test_del_first_init(struct kunit *test)
> +{
> + struct llist_node a, *b;
> + LLIST_HEAD(llist);
> +
> + llist_add(&a, &llist);
> +
> + b = llist_del_first_init(&llist);
> +
> + /* should be [List] */
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist));
> +
> + /* the returned pointer must be marked out of the list */
> + KUNIT_EXPECT_FALSE(test, llist_on_list(b));
> +}
> +
> +static void llist_test_del_first_this(struct kunit *test)
> +{
> + struct llist_node a, b;
> + LLIST_HEAD(llist);
> +
> + llist_add(&a, &llist);
> + llist_add(&b, &llist);
> +
> + llist_del_first_this(&llist, &a);
> +
> + /* before: [List] -> b -> a */
> +
> + // should remove only if is the first node in the llist
> + KUNIT_EXPECT_FALSE(test, llist_del_first_this(&llist, &a));
> +
> + KUNIT_EXPECT_TRUE(test, llist_del_first_this(&llist, &b));
> +
> + /* should be [List] -> a */
> + KUNIT_EXPECT_PTR_EQ(test, llist.first, &a);
> +}
> +
> +static void llist_test_del_all(struct kunit *test)
> +{
> + struct llist_node a, b;
> + LLIST_HEAD(llist);
> + LLIST_HEAD(empty_llist);
> +
> + llist_add(&a, &llist);
> + llist_add(&b, &llist);
> +
> + /* deleting from a empty llist should return NULL */
> + KUNIT_EXPECT_NULL(test, llist_del_all(&empty_llist));
> +
> + llist_del_all(&llist);
> +
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist));
> +}
> +
> +static void llist_test_for_each(struct kunit *test)
> +{
> + struct llist_node entries[ENTRIES_SIZE] = { 0 };
> + struct llist_node *pos, *deleted_nodes;
> + LLIST_HEAD(llist);
> + int i = 0, j = 0;
> +
> + for (int i = ENTRIES_SIZE - 1; i >= 0; i--)
> + llist_add(&entries[i], &llist);
> +
> + /* before [List] -> entries[0] -> ... -> entries[ENTRIES_SIZE - 1] */
> + llist_for_each(pos, llist.first) {
> + KUNIT_EXPECT_PTR_EQ(test, pos, &entries[i++]);
> + }
> +
> + KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, i);
> +
> + /* traversing deleted nodes */
> + deleted_nodes = llist_del_all(&llist);
> +
> + llist_for_each(pos, deleted_nodes) {
> + KUNIT_EXPECT_PTR_EQ(test, pos, &entries[j++]);
> + }
> +
> + KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, j);
> +}
> +
> +static void llist_test_safe_for_each(struct kunit *test)
> +{
> + struct llist_node entries[ENTRIES_SIZE];
> + struct llist_node *pos, *n;
> + LLIST_HEAD(llist);
> + int i = 0;
> +
> + for (int i = ENTRIES_SIZE - 1; i >= 0; i--)
> + llist_add(&entries[i], &llist);
> +
> + llist_for_each_safe(pos, n, llist.first) {
> + KUNIT_EXPECT_PTR_EQ(test, pos, &entries[i++]);
> + llist_del_first(&llist);
> + }
> +
> + KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, i);
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist));
> +}
> +
> +static void llist_test_entry_for_each(struct kunit *test)
> +{
> + struct llist_test_struct entries[ENTRIES_SIZE], *pos;
> + LLIST_HEAD(llist);
> + int i = 0;
> +
> + for (int i = ENTRIES_SIZE - 1; i >= 0; --i) {
> + entries[i].data = i;
> + llist_add(&entries[i].node, &llist);
> + }
> +
> + i = 0;
> +
> + llist_for_each_entry(pos, llist.first, node) {
> + KUNIT_EXPECT_EQ(test, pos->data, i);
> + i++;
> + }
> +
> + KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, i);
> +}
> +
> +static void llist_test_entry_safe_for_each(struct kunit *test)
> +{
> + struct llist_test_struct entries[ENTRIES_SIZE], *pos, *n;
> + LLIST_HEAD(llist);
> + int i = 0;
> +
> + for (int i = ENTRIES_SIZE - 1; i >= 0; --i) {
> + entries[i].data = i;
> + llist_add(&entries[i].node, &llist);
> + }
> +
> + i = 0;
> +
> + llist_for_each_entry_safe(pos, n, llist.first, node) {
> + KUNIT_EXPECT_EQ(test, pos->data, i++);
> + llist_del_first(&llist);
> + }
> +
> + KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, i);
> + KUNIT_EXPECT_TRUE(test, llist_empty(&llist));
> +}
> +
> +static void llist_test_reverse_order(struct kunit *test)
> +{
> + struct llist_node entries[ENTRIES_SIZE], *pos, *reversed_llist;
> + LLIST_HEAD(llist);
> + int i = 0;
> +
> + for (int i = 0; i < ENTRIES_SIZE; i++)
> + llist_add(&entries[i], &llist);
> +
> + /* before [List] -> entries[2] -> entries[1] -> entries[0] */
> + reversed_llist = llist_reverse_order(llist_del_first(&llist));
> +
> + /* should be [List] -> entries[0] -> entries[1] -> entries[2] */
> + llist_for_each(pos, reversed_llist) {
> + KUNIT_EXPECT_PTR_EQ(test, pos, &entries[i++]);
> + }
> +
> + KUNIT_EXPECT_EQ(test, ENTRIES_SIZE, i);
> +}
> +
> +static struct kunit_case llist_test_cases[] = {
> + KUNIT_CASE(llist_test_init_llist),
> + KUNIT_CASE(llist_test_init_llist_node),
> + KUNIT_CASE(llist_test_llist_entry),
> + KUNIT_CASE(llist_test_add),
> + KUNIT_CASE(llist_test_add_batch),
> + KUNIT_CASE(llist_test_llist_next),
> + KUNIT_CASE(llist_test_empty_llist),
> + KUNIT_CASE(llist_test_llist_on_list),
> + KUNIT_CASE(llist_test_del_first),
> + KUNIT_CASE(llist_test_del_first_init),
> + KUNIT_CASE(llist_test_del_first_this),
> + KUNIT_CASE(llist_test_del_all),
> + KUNIT_CASE(llist_test_for_each),
> + KUNIT_CASE(llist_test_safe_for_each),
> + KUNIT_CASE(llist_test_entry_for_each),
> + KUNIT_CASE(llist_test_entry_safe_for_each),
> + KUNIT_CASE(llist_test_reverse_order),
> + {}
> +};
> +
> +static struct kunit_suite llist_test_suite = {
> + .name = "llist",
> + .test_cases = llist_test_cases,
> +};
> +
> +kunit_test_suite(llist_test_suite);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("KUnit tests for the llist data structure.");
> --
> 2.46.0
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20240917005116.304090-2-arturacb%40gmail.com.
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5294 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-10-03 6:56 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-17 0:51 [PATCH v3 0/1] Add KUnit tests for llist Artur Alves
2024-09-17 0:51 ` [PATCH v3 1/1] lib/llist_kunit.c: add " Artur Alves
2024-10-02 20:27 ` Rae Moar
2024-10-03 6:56 ` David Gow
2024-09-19 16:01 ` [PATCH v3 0/1] Add " Shuah Khan
2024-09-19 22:27 ` Artur Alves Cavalcante de Barros
2024-09-20 7:10 ` David Gow
2024-09-20 15:10 ` Shuah Khan
2024-09-21 3:07 ` Artur Alves Cavalcante de Barros
2024-09-21 2:49 ` Artur Alves Cavalcante de Barros
2024-09-23 15:48 ` Shuah Khan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox