[PATCH 0/3] lib: add module support to sort tests

[PATCH 0/3] lib: add module support to sort tests

[Geert Uytterhoeven]

	Hi Andrew,

This patch series allows to compile the array-based and linked list sort
test code either to loadable modules, or builtin into the kernel.

It's very valuable to have modular tests, so you can run them just by
insmodding the test modules, instead of needing a separate kernel that
runs them at boot.

Thanks for applying!

Geert Uytterhoeven (3):
  Revert "lib/test_sort.c: make it explicitly non-modular"
  lib: add module support to array-based sort tests
  lib: add module support to linked list sorting tests

 lib/Kconfig.debug    |  14 ++---
 lib/Makefile         |   1 +
 lib/list_sort.c      | 149 --------------------------------------------------
 lib/test_list_sort.c | 150 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/test_sort.c      |  11 ++--
 5 files changed, 164 insertions(+), 161 deletions(-)
 create mode 100644 lib/test_list_sort.c

-- 
2.7.4

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH 1/3] Revert "lib/test_sort.c: make it explicitly non-modular"

[Geert Uytterhoeven]

This reverts commit 8893f519330bb073a49c5b4676fce4be6f1be15d.

It's very valuable to have modular tests, so you can run them just by
insmodding the test modules, instead of needing a separate kernel that
runs them at boot.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 lib/test_sort.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/lib/test_sort.c b/lib/test_sort.c
index 4db3911db50ace76..d389c1cc2f6cf795 100644
--- a/lib/test_sort.c
+++ b/lib/test_sort.c
@@ -1,11 +1,8 @@
 #include <linux/sort.h>
 #include <linux/slab.h>
-#include <linux/init.h>
+#include <linux/module.h>
 
-/*
- * A simple boot-time regression test
- * License: GPL
- */
+/* a simple boot-time regression test */
 
 #define TEST_LEN 1000
 
@@ -41,4 +38,6 @@ static int __init test_sort_init(void)
 	kfree(a);
 	return err;
 }
-subsys_initcall(test_sort_init);
+
+module_init(test_sort_init);
+MODULE_LICENSE("GPL");
-- 
2.7.4

[PATCH 2/3] lib: add module support to array-based sort tests

[Geert Uytterhoeven]

Allow to compile the array-based sort test code either to a loadable
module, or builtin into the kernel.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 lib/Kconfig.debug | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 55735c9bdb75a23e..90b2348777495b72 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1740,10 +1740,11 @@ config TEST_LIST_SORT
 	  If unsure, say N.
 
 config TEST_SORT
-	bool "Array-based sort test"
-	depends on DEBUG_KERNEL
+	tristate "Array-based sort test"
+	depends on DEBUG_KERNEL || m
 	help
-	  This option enables the self-test function of 'sort()' at boot.
+	  This option enables the self-test function of 'sort()' at boot,
+	  or at module load time.
 
 	  If unsure, say N.
 
-- 
2.7.4

[PATCH 3/3] lib: add module support to linked list sorting tests

[Geert Uytterhoeven]

Extract the linked list sorting test code into its own source file, to
allow to compile it either to a loadable module, or builtin into the
kernel.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 lib/Kconfig.debug    |   7 +--
 lib/Makefile         |   1 +
 lib/list_sort.c      | 149 --------------------------------------------------
 lib/test_list_sort.c | 150 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 155 insertions(+), 152 deletions(-)
 create mode 100644 lib/test_list_sort.c

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 90b2348777495b72..32363ae022e3282a 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1731,11 +1731,12 @@ config LKDTM
 	Documentation/fault-injection/provoke-crashes.txt
 
 config TEST_LIST_SORT
-	bool "Linked list sorting test"
-	depends on DEBUG_KERNEL
+	tristate "Linked list sorting test"
+	depends on DEBUG_KERNEL || m
 	help
 	  Enable this to turn on 'list_sort()' function test. This test is
-	  executed only once during system boot, so affects only boot time.
+	  executed only once during system boot (so affects only boot time),
+	  or at module load time.
 
 	  If unsure, say N.
 
diff --git a/lib/Makefile b/lib/Makefile
index c9023efbd4ca2b11..6adb879a7feadbc5 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_TEST_FIRMWARE) += test_firmware.o
 obj-$(CONFIG_TEST_HASH) += test_hash.o test_siphash.o
 obj-$(CONFIG_TEST_KASAN) += test_kasan.o
 obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
+obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
 obj-$(CONFIG_TEST_LKM) += test_module.o
 obj-$(CONFIG_TEST_RHASHTABLE) += test_rhashtable.o
 obj-$(CONFIG_TEST_SORT) += test_sort.o
diff --git a/lib/list_sort.c b/lib/list_sort.c
index 3fe401067e20ba81..9e9acc37652f2814 100644
--- a/lib/list_sort.c
+++ b/lib/list_sort.c
@@ -1,6 +1,3 @@
-
-#define pr_fmt(fmt) "list_sort_test: " fmt
-
 #include <linux/kernel.h>
 #include <linux/bug.h>
 #include <linux/compiler.h>
@@ -145,149 +142,3 @@ void list_sort(void *priv, struct list_head *head,
 	merge_and_restore_back_links(priv, cmp, head, part[max_lev], list);
 }
 EXPORT_SYMBOL(list_sort);
-
-#ifdef CONFIG_TEST_LIST_SORT
-
-#include <linux/slab.h>
-#include <linux/random.h>
-
-/*
- * The pattern of set bits in the list length determines which cases
- * are hit in list_sort().
- */
-#define TEST_LIST_LEN (512+128+2) /* not including head */
-
-#define TEST_POISON1 0xDEADBEEF
-#define TEST_POISON2 0xA324354C
-
-struct debug_el {
-	unsigned int poison1;
-	struct list_head list;
-	unsigned int poison2;
-	int value;
-	unsigned serial;
-};
-
-/* Array, containing pointers to all elements in the test list */
-static struct debug_el **elts __initdata;
-
-static int __init check(struct debug_el *ela, struct debug_el *elb)
-{
-	if (ela->serial >= TEST_LIST_LEN) {
-		pr_err("error: incorrect serial %d\n", ela->serial);
-		return -EINVAL;
-	}
-	if (elb->serial >= TEST_LIST_LEN) {
-		pr_err("error: incorrect serial %d\n", elb->serial);
-		return -EINVAL;
-	}
-	if (elts[ela->serial] != ela || elts[elb->serial] != elb) {
-		pr_err("error: phantom element\n");
-		return -EINVAL;
-	}
-	if (ela->poison1 != TEST_POISON1 || ela->poison2 != TEST_POISON2) {
-		pr_err("error: bad poison: %#x/%#x\n",
-			ela->poison1, ela->poison2);
-		return -EINVAL;
-	}
-	if (elb->poison1 != TEST_POISON1 || elb->poison2 != TEST_POISON2) {
-		pr_err("error: bad poison: %#x/%#x\n",
-			elb->poison1, elb->poison2);
-		return -EINVAL;
-	}
-	return 0;
-}
-
-static int __init cmp(void *priv, struct list_head *a, struct list_head *b)
-{
-	struct debug_el *ela, *elb;
-
-	ela = container_of(a, struct debug_el, list);
-	elb = container_of(b, struct debug_el, list);
-
-	check(ela, elb);
-	return ela->value - elb->value;
-}
-
-static int __init list_sort_test(void)
-{
-	int i, count = 1, err = -ENOMEM;
-	struct debug_el *el;
-	struct list_head *cur;
-	LIST_HEAD(head);
-
-	pr_debug("start testing list_sort()\n");
-
-	elts = kcalloc(TEST_LIST_LEN, sizeof(*elts), GFP_KERNEL);
-	if (!elts) {
-		pr_err("error: cannot allocate memory\n");
-		return err;
-	}
-
-	for (i = 0; i < TEST_LIST_LEN; i++) {
-		el = kmalloc(sizeof(*el), GFP_KERNEL);
-		if (!el) {
-			pr_err("error: cannot allocate memory\n");
-			goto exit;
-		}
-		 /* force some equivalencies */
-		el->value = prandom_u32() % (TEST_LIST_LEN / 3);
-		el->serial = i;
-		el->poison1 = TEST_POISON1;
-		el->poison2 = TEST_POISON2;
-		elts[i] = el;
-		list_add_tail(&el->list, &head);
-	}
-
-	list_sort(NULL, &head, cmp);
-
-	err = -EINVAL;
-	for (cur = head.next; cur->next != &head; cur = cur->next) {
-		struct debug_el *el1;
-		int cmp_result;
-
-		if (cur->next->prev != cur) {
-			pr_err("error: list is corrupted\n");
-			goto exit;
-		}
-
-		cmp_result = cmp(NULL, cur, cur->next);
-		if (cmp_result > 0) {
-			pr_err("error: list is not sorted\n");
-			goto exit;
-		}
-
-		el = container_of(cur, struct debug_el, list);
-		el1 = container_of(cur->next, struct debug_el, list);
-		if (cmp_result == 0 && el->serial >= el1->serial) {
-			pr_err("error: order of equivalent elements not "
-				"preserved\n");
-			goto exit;
-		}
-
-		if (check(el, el1)) {
-			pr_err("error: element check failed\n");
-			goto exit;
-		}
-		count++;
-	}
-	if (head.prev != cur) {
-		pr_err("error: list is corrupted\n");
-		goto exit;
-	}
-
-
-	if (count != TEST_LIST_LEN) {
-		pr_err("error: bad list length %d", count);
-		goto exit;
-	}
-
-	err = 0;
-exit:
-	for (i = 0; i < TEST_LIST_LEN; i++)
-		kfree(elts[i]);
-	kfree(elts);
-	return err;
-}
-late_initcall(list_sort_test);
-#endif /* CONFIG_TEST_LIST_SORT */
diff --git a/lib/test_list_sort.c b/lib/test_list_sort.c
new file mode 100644
index 0000000000000000..28e817387b04f1c8
--- /dev/null
+++ b/lib/test_list_sort.c
@@ -0,0 +1,150 @@
+#define pr_fmt(fmt) "list_sort_test: " fmt
+
+#include <linux/kernel.h>
+#include <linux/list_sort.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/printk.h>
+#include <linux/slab.h>
+#include <linux/random.h>
+
+/*
+ * The pattern of set bits in the list length determines which cases
+ * are hit in list_sort().
+ */
+#define TEST_LIST_LEN (512+128+2) /* not including head */
+
+#define TEST_POISON1 0xDEADBEEF
+#define TEST_POISON2 0xA324354C
+
+struct debug_el {
+	unsigned int poison1;
+	struct list_head list;
+	unsigned int poison2;
+	int value;
+	unsigned serial;
+};
+
+/* Array, containing pointers to all elements in the test list */
+static struct debug_el **elts __initdata;
+
+static int __init check(struct debug_el *ela, struct debug_el *elb)
+{
+	if (ela->serial >= TEST_LIST_LEN) {
+		pr_err("error: incorrect serial %d\n", ela->serial);
+		return -EINVAL;
+	}
+	if (elb->serial >= TEST_LIST_LEN) {
+		pr_err("error: incorrect serial %d\n", elb->serial);
+		return -EINVAL;
+	}
+	if (elts[ela->serial] != ela || elts[elb->serial] != elb) {
+		pr_err("error: phantom element\n");
+		return -EINVAL;
+	}
+	if (ela->poison1 != TEST_POISON1 || ela->poison2 != TEST_POISON2) {
+		pr_err("error: bad poison: %#x/%#x\n",
+			ela->poison1, ela->poison2);
+		return -EINVAL;
+	}
+	if (elb->poison1 != TEST_POISON1 || elb->poison2 != TEST_POISON2) {
+		pr_err("error: bad poison: %#x/%#x\n",
+			elb->poison1, elb->poison2);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int __init cmp(void *priv, struct list_head *a, struct list_head *b)
+{
+	struct debug_el *ela, *elb;
+
+	ela = container_of(a, struct debug_el, list);
+	elb = container_of(b, struct debug_el, list);
+
+	check(ela, elb);
+	return ela->value - elb->value;
+}
+
+static int __init list_sort_test(void)
+{
+	int i, count = 1, err = -ENOMEM;
+	struct debug_el *el;
+	struct list_head *cur;
+	LIST_HEAD(head);
+
+	pr_debug("start testing list_sort()\n");
+
+	elts = kcalloc(TEST_LIST_LEN, sizeof(*elts), GFP_KERNEL);
+	if (!elts) {
+		pr_err("error: cannot allocate memory\n");
+		return err;
+	}
+
+	for (i = 0; i < TEST_LIST_LEN; i++) {
+		el = kmalloc(sizeof(*el), GFP_KERNEL);
+		if (!el) {
+			pr_err("error: cannot allocate memory\n");
+			goto exit;
+		}
+		 /* force some equivalencies */
+		el->value = prandom_u32() % (TEST_LIST_LEN / 3);
+		el->serial = i;
+		el->poison1 = TEST_POISON1;
+		el->poison2 = TEST_POISON2;
+		elts[i] = el;
+		list_add_tail(&el->list, &head);
+	}
+
+	list_sort(NULL, &head, cmp);
+
+	err = -EINVAL;
+	for (cur = head.next; cur->next != &head; cur = cur->next) {
+		struct debug_el *el1;
+		int cmp_result;
+
+		if (cur->next->prev != cur) {
+			pr_err("error: list is corrupted\n");
+			goto exit;
+		}
+
+		cmp_result = cmp(NULL, cur, cur->next);
+		if (cmp_result > 0) {
+			pr_err("error: list is not sorted\n");
+			goto exit;
+		}
+
+		el = container_of(cur, struct debug_el, list);
+		el1 = container_of(cur->next, struct debug_el, list);
+		if (cmp_result == 0 && el->serial >= el1->serial) {
+			pr_err("error: order of equivalent elements not "
+				"preserved\n");
+			goto exit;
+		}
+
+		if (check(el, el1)) {
+			pr_err("error: element check failed\n");
+			goto exit;
+		}
+		count++;
+	}
+	if (head.prev != cur) {
+		pr_err("error: list is corrupted\n");
+		goto exit;
+	}
+
+
+	if (count != TEST_LIST_LEN) {
+		pr_err("error: bad list length %d", count);
+		goto exit;
+	}
+
+	err = 0;
+exit:
+	for (i = 0; i < TEST_LIST_LEN; i++)
+		kfree(elts[i]);
+	kfree(elts);
+	return err;
+}
+module_init(list_sort_test);
+MODULE_LICENSE("GPL");
-- 
2.7.4

Re: [PATCH 1/3] Revert "lib/test_sort.c: make it explicitly non-modular"

[Andy Shevchenko]

On Tue, 2017-02-28 at 14:06 +0100, Geert Uytterhoeven wrote:
> This reverts commit 8893f519330bb073a49c5b4676fce4be6f1be15d.
> 
> It's very valuable to have modular tests, so you can run them just by
> insmodding the test modules, instead of needing a separate kernel that
> runs them at boot.
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>  lib/test_sort.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/test_sort.c b/lib/test_sort.c
> index 4db3911db50ace76..d389c1cc2f6cf795 100644
> --- a/lib/test_sort.c
> +++ b/lib/test_sort.c
> @@ -1,11 +1,8 @@
>  #include <linux/sort.h>
>  #include <linux/slab.h>
> -#include <linux/init.h>
> +#include <linux/module.h>
> 

>  
> -/*
> - * A simple boot-time regression test
> - * License: GPL
> - */
> +/* a simple boot-time regression test */

I would leave this piece as is now.

The rest is okay.

>  
>  #define TEST_LEN 1000
>  
> @@ -41,4 +38,6 @@ static int __init test_sort_init(void)
>  	kfree(a);
>  	return err;
>  }
> -subsys_initcall(test_sort_init);
> +
> +module_init(test_sort_init);
> +MODULE_LICENSE("GPL");

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH 3/3] lib: add module support to linked list sorting tests

[Arnd Bergmann]

On Tue, Feb 28, 2017 at 2:06 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:

> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 90b2348777495b72..32363ae022e3282a 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1731,11 +1731,12 @@ config LKDTM
>         Documentation/fault-injection/provoke-crashes.txt
>
>  config TEST_LIST_SORT
> -       bool "Linked list sorting test"
> -       depends on DEBUG_KERNEL
> +       tristate "Linked list sorting test"
> +       depends on DEBUG_KERNEL || m

I don't see any other test code with a "DEBUG_KERNEL || m" dependency,
but several that are "DEBUG_KERNEL && m". Maybe we should just
force it to be a module like the others? If there are good reasons to
have it built-in, I'd just drop the "|| m" here.

     Arnd

Re: [PATCH 0/3] lib: add module support to sort tests

[Andy Shevchenko]

On Tue, 2017-02-28 at 14:06 +0100, Geert Uytterhoeven wrote:
> 	Hi Andrew,
> 
> This patch series allows to compile the array-based and linked list
> sort
> test code either to loadable modules, or builtin into the kernel.
> 
> It's very valuable to have modular tests, so you can run them just by
> insmodding the test modules, instead of needing a separate kernel that
> runs them at boot.
> 
> Thanks for applying!

FWIW:
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

On the first glance looks okay to me. I didn't carefully checked code
move (I guess -M -C will not help much, though one can play with numbers
there), so I believe Geert didn't change much there.

> 
> Geert Uytterhoeven (3):
>   Revert "lib/test_sort.c: make it explicitly non-modular"
>   lib: add module support to array-based sort tests
>   lib: add module support to linked list sorting tests
> 
>  lib/Kconfig.debug    |  14 ++---
>  lib/Makefile         |   1 +
>  lib/list_sort.c      | 149 ------------------------------------------
> --------
>  lib/test_list_sort.c | 150
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/test_sort.c      |  11 ++--
>  5 files changed, 164 insertions(+), 161 deletions(-)
>  create mode 100644 lib/test_list_sort.c
> 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH 3/3] lib: add module support to linked list sorting tests

[Geert Uytterhoeven]

Hi Arnd,

On Tue, Feb 28, 2017 at 3:28 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tue, Feb 28, 2017 at 2:06 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 90b2348777495b72..32363ae022e3282a 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -1731,11 +1731,12 @@ config LKDTM
>>         Documentation/fault-injection/provoke-crashes.txt
>>
>>  config TEST_LIST_SORT
>> -       bool "Linked list sorting test"
>> -       depends on DEBUG_KERNEL
>> +       tristate "Linked list sorting test"
>> +       depends on DEBUG_KERNEL || m
>
> I don't see any other test code with a "DEBUG_KERNEL || m" dependency,
> but several that are "DEBUG_KERNEL && m". Maybe we should just
> force it to be a module like the others? If there are good reasons to
> have it built-in, I'd just drop the "|| m" here.

Some people want to run all tests at boot time, so forcing the test to be
modular breaks that.

I used "|| m" as I like to enable all tests as modules, even when not
using DEBUG_KERNEL.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] Revert "lib/test_sort.c: make it explicitly non-modular"

[Geert Uytterhoeven]

Hi Andy,

On Tue, Feb 28, 2017 at 3:24 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Tue, 2017-02-28 at 14:06 +0100, Geert Uytterhoeven wrote:
>> This reverts commit 8893f519330bb073a49c5b4676fce4be6f1be15d.
>>
>> It's very valuable to have modular tests, so you can run them just by
>> insmodding the test modules, instead of needing a separate kernel that
>> runs them at boot.
>>
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>>  lib/test_sort.c | 11 +++++------
>>  1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/lib/test_sort.c b/lib/test_sort.c
>> index 4db3911db50ace76..d389c1cc2f6cf795 100644
>> --- a/lib/test_sort.c
>> +++ b/lib/test_sort.c
>> @@ -1,11 +1,8 @@
>>  #include <linux/sort.h>
>>  #include <linux/slab.h>
>> -#include <linux/init.h>
>> +#include <linux/module.h>
>>
>
>>
>> -/*
>> - * A simple boot-time regression test
>> - * License: GPL
>> - */
>> +/* a simple boot-time regression test */
>
> I would leave this piece as is now.

As this is a revert, I prefer to revert 100%.

> The rest is okay.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds