Re: [PATCH linux-kselftest/test v4] lib/list-test: add a test for the 'list' doubly linked list

On Fri, Oct 18, 2019 at 02:55:49PM -0700, David Gow wrote:

Add a KUnit test for the kernel doubly linked list implementation in include/linux/list.h

Each test case (list_test_x) is focused on testing the behaviour of the list function/macro 'x'. None of the tests pass invalid lists to these macros, and so should behave identically with DEBUG_LIST enabled and disabled.

Note that, at present, it only tests the list_ types (not the singly-linked hlist_), and does not yet test all of the list_for_each_entry* macros (and some related things like list_prepare_entry).

Signed-off-by: David Gow davidgow@google.com

The changes from v3 are mostly to do with naming:

• The Kconfig entry has been renamed from LIST_TEST to LIST_KUNIT_TEST, which matches the SYSCTL_KUNIT_TEST entry,
• The Kconfig description was updated to better match other KUnit tests, specifying that the test is not intended for use in a production kernel. A now-redundant mention of the test running a boot was removed.
• The MAINTAINERS entry refers to a "KUNIT TEST" rather than a "UNIT TEST"
• The module name has changed from "list-test" to "list-kunit-test".

Earlier versions of the test can be found: v3: https://lore.kernel.org/linux-kselftest/20191016215707.95317-1-davidgow@goog... v2: https://lore.kernel.org/linux-kselftest/20191010185631.26541-1-davidgow@goog... v1: https://lore.kernel.org/linux-kselftest/20191007213633.92565-1-davidgow@goog...

MAINTAINERS | 5 + lib/Kconfig.debug | 18 ++ lib/Makefile | 3 + lib/list-test.c | 740 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 766 insertions(+) create mode 100644 lib/list-test.c

diff --git a/MAINTAINERS b/MAINTAINERS index 7ef985e01457..7ced1b69a3d3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9504,6 +9504,11 @@ F: Documentation/misc-devices/lis3lv02d.rst F: drivers/misc/lis3lv02d/ F: drivers/platform/x86/hp_accel.c +LIST KUNIT TEST +M: David Gow davidgow@google.com +S: Maintained +F: lib/list-test.c

LIVE PATCHING M: Josh Poimboeuf jpoimboe@redhat.com M: Jiri Kosina jikos@kernel.org diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index a3017a5dadcd..7991b78eb1f3 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1961,6 +1961,24 @@ config SYSCTL_KUNIT_TEST If unsure, say N. +config LIST_KUNIT_TEST

• bool "KUnit Test for Kernel Linked-list structures"
• depends on KUNIT
• help

•  This builds the linked list KUnit test suite.
  

•  It tests that the API and basic functionality of the list_head type
  

•  and associated macros.
  

•  KUnit tests run during boot and output the results to the debug log
  

•  in TAP format (http://testanything.org/). Only useful for kernel devs
  

•  running the KUnit test harness, and not intended for inclusion into a
  

•  production build.
  

•  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.
  

config TEST_UDELAY tristate "udelay test driver" help diff --git a/lib/Makefile b/lib/Makefile index bba1fd5485f7..890e581d00c4 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -292,3 +292,6 @@ obj-$(CONFIG_GENERIC_LIB_MULDI3) += muldi3.o obj-$(CONFIG_GENERIC_LIB_CMPDI2) += cmpdi2.o obj-$(CONFIG_GENERIC_LIB_UCMPDI2) += ucmpdi2.o obj-$(CONFIG_OBJAGG) += objagg.o

+# KUnit tests +obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o diff --git a/lib/list-test.c b/lib/list-test.c new file mode 100644 index 000000000000..75ba3ddac959 --- /dev/null +++ b/lib/list-test.c @@ -0,0 +1,740 @@ +// SPDX-License-Identifier: GPL-2.0 +/*

• • KUnit test for the Kernel Linked-list structures.
• • Copyright (C) 2019, Google LLC.
• • Author: David Gow davidgow@google.com
• */

+#include <kunit/test.h>

+#include <linux/list.h>

+struct list_test_struct {

• int data;
• struct list_head list;

+};

+static void list_test_list_init(struct kunit *test) +{

• /* Test the different ways of initialising a list. */
• struct list_head list1 = LIST_HEAD_INIT(list1);
• struct list_head list2;
• LIST_HEAD(list3);
• struct list_head *list4;
• struct list_head *list5;
• INIT_LIST_HEAD(&list2);
• list4 = kzalloc(sizeof(*list4), GFP_KERNEL);
• KUNIT_ASSERT_NOT_ERR_OR_NULL(test, list4);

Why not just use GFP_KERNEL | GFP_NOFAIL and remove the check?

kzalloc() can't return error pointers. If this were an IS_ERR_OR_NULL() check then it would generate a static checker warning, but static checkers don't know about KUNIT_ASSERT_NOT_ERR_OR_NULL() yet so you're safe.

But generally NULL is a special case of success. A common situation is where the user deliberately disables a feature, that means it's not an error but we also don't have a valid pointer to return because it's disabled.

regards, dan carpenter