[PATCH 0/4] selftests, sysctl, lib: Fix prime_numbers and sysctl test to run

[PATCH 0/4] selftests, sysctl, lib: Fix prime_numbers and sysctl test to run

[Masami Hiramatsu]

Hi,

Recently, I found some tests were always skipped.
Here is a series of patches to fix those issues.

The prime_numbers test is skipped in some cases because
prime_numbers.ko is not always compiled.
Since the CONFIG_PRIME_NUMBERS is not independently
configurable item (it has no title and help), it is enabled
only if other configs (DRM_DEBUG_SELFTEST etc.) select it.

To fix this issue, I added a title and help for
CONFIG_PRIME_NUMBERS.

The sysctl test is skipped because
 - selftests/sysctl/config requires CONFIG_TEST_SYSCTL=y. But
   since lib/test_sysctl.c doesn't use module_init(), the
   test_syscall is not listed under /sys/module/ and the 
   test script gives up.
 - Even if we make CONFIG_TEST_SYSCTL=m, the test script checks
   /sys/modules/test_sysctl before loading module and gives up.
 - Ayway, since the test module introduces useless sysctl
   interface to the kernel, it would better be a module.

This series includes fixes for above 3 points.
 - Fix lib/test_sysctl.c to use module_init()
 - Fix tools/testing/selftests/sysctl/sysctl.sh to try to load
   test module if it is not loaded (nor embedded).
 - Fix tools/testing/selftests/sysctl/config to require
   CONFIG_TEST_SYSCTL=m, not y.

Thank you,

---

Masami Hiramatsu (4):
      lib: Make prime number generator independently selectable
      lib: Make test_sysctl initialized as module
      selftests/sysctl: Fix to load test_sysctl module
      selftests/sysctl: Make sysctl test driver as a module


 lib/math/Kconfig                         |    7 ++++++-
 lib/test_sysctl.c                        |    2 +-
 tools/testing/selftests/sysctl/config    |    2 +-
 tools/testing/selftests/sysctl/sysctl.sh |   13 ++-----------
 4 files changed, 10 insertions(+), 14 deletions(-)

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

[PATCH 1/4] lib: Make prime number generator independently selectable

[Masami Hiramatsu]

Make prime number generator independently selectable from
kconfig. This allows us to enable CONFIG_PRIME_NUMBERS=m
and run the tools/testing/selftests/lib/prime_numbers.sh
without other DRM selftest modules.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 lib/math/Kconfig |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/math/Kconfig b/lib/math/Kconfig
index 15bd50d92308..f19bc9734fa7 100644
--- a/lib/math/Kconfig
+++ b/lib/math/Kconfig
@@ -6,7 +6,12 @@ config CORDIC
 	  calculations are in fixed point. Module will be called cordic.
 
 config PRIME_NUMBERS
-	tristate
+	tristate "Simple prime number generator for testing"
+	help
+	  This option provides a simple prime number generator for test
+	  modules.
+
+	  If unsure, say N.
 
 config RATIONAL
 	bool

Re: [PATCH 1/4] lib: Make prime number generator independently selectable

[Kees Cook]

On Thu, May 28, 2020 at 11:52:06PM +0900, Masami Hiramatsu wrote:
> Make prime number generator independently selectable from
> kconfig. This allows us to enable CONFIG_PRIME_NUMBERS=m
> and run the tools/testing/selftests/lib/prime_numbers.sh
> without other DRM selftest modules.

Nice catch! I see that tools/testing/selftests/lib/config already has
CONFIG_PRIME_NUMBERS=m (based on this commit log I was expecting to see
it added in the diff, but I see it's not needed).

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH 1/4] lib: Make prime number generator independently selectable

[Masami Hiramatsu]

On Thu, 28 May 2020 22:56:59 -0700
Kees Cook <keescook@chromium.org> wrote:

> On Thu, May 28, 2020 at 11:52:06PM +0900, Masami Hiramatsu wrote:
> > Make prime number generator independently selectable from
> > kconfig. This allows us to enable CONFIG_PRIME_NUMBERS=m
> > and run the tools/testing/selftests/lib/prime_numbers.sh
> > without other DRM selftest modules.
> 
> Nice catch! I see that tools/testing/selftests/lib/config already has
> CONFIG_PRIME_NUMBERS=m (based on this commit log I was expecting to see
> it added in the diff, but I see it's not needed).

Yes, that is the reason why I have found this issue, the "make kselftest-merge"
cannot enable CONFIG_PRIME_NUMBERS=m without this fix. 

> 
> Reviewed-by: Kees Cook <keescook@chromium.org>

Thank you!

> 
> -- 
> Kees Cook


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH 1/4] lib: Make prime number generator independently selectable

[Luis Chamberlain]

On Thu, May 28, 2020 at 11:52:06PM +0900, Masami Hiramatsu wrote:
> Make prime number generator independently selectable from
> kconfig. This allows us to enable CONFIG_PRIME_NUMBERS=m
> and run the tools/testing/selftests/lib/prime_numbers.sh
> without other DRM selftest modules.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>

  Luis

[PATCH 2/4] lib: Make test_sysctl initialized as module

[Masami Hiramatsu]

test_sysctl.c is expected to be used as a module, but since
it does not use module_init(), it never be registered as
a module and not appeared under /sys/module/.
In the result, the selftests/sysctl/sysctl.sh always fails
to find the test module and is skipped.

This makes test_sysctl.c initialized as a module by module_init()
and allow sysctl.sh to find the test module is loaded.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 lib/test_sysctl.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/test_sysctl.c b/lib/test_sysctl.c
index 566dad3f4196..ec4d0f03475d 100644
--- a/lib/test_sysctl.c
+++ b/lib/test_sysctl.c
@@ -149,7 +149,7 @@ static int __init test_sysctl_init(void)
 	}
 	return 0;
 }
-late_initcall(test_sysctl_init);
+module_init(test_sysctl_init);
 
 static void __exit test_sysctl_exit(void)
 {

Re: [PATCH 2/4] lib: Make test_sysctl initialized as module

[Kees Cook]

On Thu, May 28, 2020 at 11:52:16PM +0900, Masami Hiramatsu wrote:
> test_sysctl.c is expected to be used as a module, but since
> it does not use module_init(), it never be registered as
> a module and not appeared under /sys/module/.
> In the result, the selftests/sysctl/sysctl.sh always fails
> to find the test module and is skipped.
> 
> This makes test_sysctl.c initialized as a module by module_init()
> and allow sysctl.sh to find the test module is loaded.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  lib/test_sysctl.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/test_sysctl.c b/lib/test_sysctl.c
> index 566dad3f4196..ec4d0f03475d 100644
> --- a/lib/test_sysctl.c
> +++ b/lib/test_sysctl.c
> @@ -149,7 +149,7 @@ static int __init test_sysctl_init(void)
>  	}
>  	return 0;
>  }
> -late_initcall(test_sysctl_init);
> +module_init(test_sysctl_init);

This is the only part I think we need to double check. As a non-module,
module_init() becomes device_initcall() not late_initcall().

I don't see any notes in the commit log for the original driver that
mention why this needs to be late_initcall(), though, so I *think* it's
safe. Luis?

-- 
Kees Cook

Re: [PATCH 2/4] lib: Make test_sysctl initialized as module

[Luis Chamberlain]

On Thu, May 28, 2020 at 11:52:16PM +0900, Masami Hiramatsu wrote:
> test_sysctl.c is expected to be used as a module, but since
> it does not use module_init(), it never be registered as
> a module and not appeared under /sys/module/.
> In the result, the selftests/sysctl/sysctl.sh always fails
> to find the test module and is skipped.
> 
> This makes test_sysctl.c initialized as a module by module_init()
> and allow sysctl.sh to find the test module is loaded.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>

  Luis

[PATCH 3/4] selftests/sysctl: Fix to load test_sysctl module

[Masami Hiramatsu]

Fix to load test_sysctl.ko module correctly.

sysctl.sh checks whether the test module is embedded (or loaded
already) or not at first, and if not, it returns skip error
instead of trying modprobe. Thus, there is no chance to load the
test_sysctl test module.

Instead, this removes that module embedded check and returns
skip error only if it ensures that there is no embedded test
module *and* no loadable test module.

This also avoid referring config file since that is not
installed.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/testing/selftests/sysctl/sysctl.sh |   13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/sysctl/sysctl.sh b/tools/testing/selftests/sysctl/sysctl.sh
index 6a970b127c9b..c3459f9f2429 100755
--- a/tools/testing/selftests/sysctl/sysctl.sh
+++ b/tools/testing/selftests/sysctl/sysctl.sh
@@ -40,16 +40,6 @@ ALL_TESTS="$ALL_TESTS 0004:1:1:uint_0001"
 ALL_TESTS="$ALL_TESTS 0005:3:1:int_0003"
 ALL_TESTS="$ALL_TESTS 0006:50:1:bitmap_0001"
 
-test_modprobe()
-{
-       if [ ! -d $DIR ]; then
-               echo "$0: $DIR not present" >&2
-               echo "You must have the following enabled in your kernel:" >&2
-               cat $TEST_DIR/config >&2
-               exit $ksft_skip
-       fi
-}
-
 function allow_user_defaults()
 {
 	if [ -z $DIR ]; then
@@ -125,10 +115,12 @@ function load_req_mod()
 	if [ ! -d $DIR ]; then
 		if ! modprobe -q -n $TEST_DRIVER; then
 			echo "$0: module $TEST_DRIVER not found [SKIP]"
+			echo "You must set CONFIG_TEST_SYSCTL=m in your kernel" >&2
 			exit $ksft_skip
 		fi
 		modprobe $TEST_DRIVER
 		if [ $? -ne 0 ]; then
+			echo "$0: modprobe $TEST_DRIVER failed."
 			exit
 		fi
 	fi
@@ -929,7 +921,6 @@ test_reqs
 allow_user_defaults
 check_production_sysctl_writes_strict
 load_req_mod
-test_modprobe
 
 trap "test_finish" EXIT
 

Re: [PATCH 3/4] selftests/sysctl: Fix to load test_sysctl module

[Kees Cook]

On Thu, May 28, 2020 at 11:52:26PM +0900, Masami Hiramatsu wrote:
> Fix to load test_sysctl.ko module correctly.
> 
> sysctl.sh checks whether the test module is embedded (or loaded
> already) or not at first, and if not, it returns skip error
> instead of trying modprobe. Thus, there is no chance to load the
> test_sysctl test module.
> 
> Instead, this removes that module embedded check and returns
> skip error only if it ensures that there is no embedded test
> module *and* no loadable test module.
> 
> This also avoid referring config file since that is not
> installed.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH 3/4] selftests/sysctl: Fix to load test_sysctl module

[Luis Chamberlain]

On Thu, May 28, 2020 at 11:52:26PM +0900, Masami Hiramatsu wrote:
> Fix to load test_sysctl.ko module correctly.
> 
> sysctl.sh checks whether the test module is embedded (or loaded
> already) or not at first, and if not, it returns skip error
> instead of trying modprobe. Thus, there is no chance to load the
> test_sysctl test module.
> 
> Instead, this removes that module embedded check and returns
> skip error only if it ensures that there is no embedded test
> module *and* no loadable test module.
> 
> This also avoid referring config file since that is not
> installed.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>

  Luis

[PATCH 4/4] selftests/sysctl: Make sysctl test driver as a module

[Masami Hiramatsu]

Fix config file to require CONFIG_TEST_SYSCTL=m instead of y
because this driver introduces a test sysctl interfaces which
are normally not used, and only used for the selftest.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/testing/selftests/sysctl/config |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/sysctl/config b/tools/testing/selftests/sysctl/config
index 6ca14800d755..fc263efd1fad 100644
--- a/tools/testing/selftests/sysctl/config
+++ b/tools/testing/selftests/sysctl/config
@@ -1 +1 @@
-CONFIG_TEST_SYSCTL=y
+CONFIG_TEST_SYSCTL=m

Re: [PATCH 4/4] selftests/sysctl: Make sysctl test driver as a module

[Kees Cook]

On Thu, May 28, 2020 at 11:52:37PM +0900, Masami Hiramatsu wrote:
> Fix config file to require CONFIG_TEST_SYSCTL=m instead of y
> because this driver introduces a test sysctl interfaces which
> are normally not used, and only used for the selftest.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH 4/4] selftests/sysctl: Make sysctl test driver as a module

[Luis Chamberlain]

On Thu, May 28, 2020 at 11:52:37PM +0900, Masami Hiramatsu wrote:
> Fix config file to require CONFIG_TEST_SYSCTL=m instead of y
> because this driver introduces a test sysctl interfaces which
> are normally not used, and only used for the selftest.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>

  Luis

Re: [PATCH 0/4] selftests, sysctl, lib: Fix prime_numbers and sysctl test to run

[Shuah Khan]

On 5/28/20 8:51 AM, Masami Hiramatsu wrote:
> Hi,
> 
> Recently, I found some tests were always skipped.
> Here is a series of patches to fix those issues.
> 
> The prime_numbers test is skipped in some cases because
> prime_numbers.ko is not always compiled.
> Since the CONFIG_PRIME_NUMBERS is not independently
> configurable item (it has no title and help), it is enabled
> only if other configs (DRM_DEBUG_SELFTEST etc.) select it.
> 
> To fix this issue, I added a title and help for
> CONFIG_PRIME_NUMBERS.
> 
> The sysctl test is skipped because
>   - selftests/sysctl/config requires CONFIG_TEST_SYSCTL=y. But
>     since lib/test_sysctl.c doesn't use module_init(), the
>     test_syscall is not listed under /sys/module/ and the
>     test script gives up.
>   - Even if we make CONFIG_TEST_SYSCTL=m, the test script checks
>     /sys/modules/test_sysctl before loading module and gives up.
>   - Ayway, since the test module introduces useless sysctl
>     interface to the kernel, it would better be a module.
> 
> This series includes fixes for above 3 points.
>   - Fix lib/test_sysctl.c to use module_init()
>   - Fix tools/testing/selftests/sysctl/sysctl.sh to try to load
>     test module if it is not loaded (nor embedded).
>   - Fix tools/testing/selftests/sysctl/config to require
>     CONFIG_TEST_SYSCTL=m, not y.
> 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (4):
>        lib: Make prime number generator independently selectable
>        lib: Make test_sysctl initialized as module
>        selftests/sysctl: Fix to load test_sysctl module
>        selftests/sysctl: Make sysctl test driver as a module
> 
> 
>   lib/math/Kconfig                         |    7 ++++++-
>   lib/test_sysctl.c                        |    2 +-
>   tools/testing/selftests/sysctl/config    |    2 +-
>   tools/testing/selftests/sysctl/sysctl.sh |   13 ++-----------
>   4 files changed, 10 insertions(+), 14 deletions(-)
> 
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
> 

Thanks Masami. I see Kees reviewing patches. I will wait for Luis to
weigh in on patch 2 before pulling this series in.

thanks,
-- Shuah

Re: [PATCH 0/4] selftests, sysctl, lib: Fix prime_numbers and sysctl test to run

[Masami Hiramatsu]

On Fri, 29 May 2020 08:14:39 -0600
Shuah Khan <skhan@linuxfoundation.org> wrote:

> On 5/28/20 8:51 AM, Masami Hiramatsu wrote:
> > Hi,
> > 
> > Recently, I found some tests were always skipped.
> > Here is a series of patches to fix those issues.
> > 
> > The prime_numbers test is skipped in some cases because
> > prime_numbers.ko is not always compiled.
> > Since the CONFIG_PRIME_NUMBERS is not independently
> > configurable item (it has no title and help), it is enabled
> > only if other configs (DRM_DEBUG_SELFTEST etc.) select it.
> > 
> > To fix this issue, I added a title and help for
> > CONFIG_PRIME_NUMBERS.
> > 
> > The sysctl test is skipped because
> >   - selftests/sysctl/config requires CONFIG_TEST_SYSCTL=y. But
> >     since lib/test_sysctl.c doesn't use module_init(), the
> >     test_syscall is not listed under /sys/module/ and the
> >     test script gives up.
> >   - Even if we make CONFIG_TEST_SYSCTL=m, the test script checks
> >     /sys/modules/test_sysctl before loading module and gives up.
> >   - Ayway, since the test module introduces useless sysctl
> >     interface to the kernel, it would better be a module.
> > 
> > This series includes fixes for above 3 points.
> >   - Fix lib/test_sysctl.c to use module_init()
> >   - Fix tools/testing/selftests/sysctl/sysctl.sh to try to load
> >     test module if it is not loaded (nor embedded).
> >   - Fix tools/testing/selftests/sysctl/config to require
> >     CONFIG_TEST_SYSCTL=m, not y.
> > 
> > Thank you,
> > 
> > ---
> > 
> > Masami Hiramatsu (4):
> >        lib: Make prime number generator independently selectable
> >        lib: Make test_sysctl initialized as module
> >        selftests/sysctl: Fix to load test_sysctl module
> >        selftests/sysctl: Make sysctl test driver as a module
> > 
> > 
> >   lib/math/Kconfig                         |    7 ++++++-
> >   lib/test_sysctl.c                        |    2 +-
> >   tools/testing/selftests/sysctl/config    |    2 +-
> >   tools/testing/selftests/sysctl/sysctl.sh |   13 ++-----------
> >   4 files changed, 10 insertions(+), 14 deletions(-)
> > 
> > --
> > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
> > 
> 
> Thanks Masami. I see Kees reviewing patches. I will wait for Luis to
> weigh in on patch 2 before pulling this series in.

OK, Thanks Shuah! 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH 0/4] selftests, sysctl, lib: Fix prime_numbers and sysctl test to run

[Shuah Khan]

On 5/29/20 8:39 AM, Masami Hiramatsu wrote:
> On Fri, 29 May 2020 08:14:39 -0600
> Shuah Khan <skhan@linuxfoundation.org> wrote:
> 
>> On 5/28/20 8:51 AM, Masami Hiramatsu wrote:
>>> Hi,
>>>
>>> Recently, I found some tests were always skipped.
>>> Here is a series of patches to fix those issues.
>>>
>>> The prime_numbers test is skipped in some cases because
>>> prime_numbers.ko is not always compiled.
>>> Since the CONFIG_PRIME_NUMBERS is not independently
>>> configurable item (it has no title and help), it is enabled
>>> only if other configs (DRM_DEBUG_SELFTEST etc.) select it.
>>>
>>> To fix this issue, I added a title and help for
>>> CONFIG_PRIME_NUMBERS.
>>>
>>> The sysctl test is skipped because
>>>    - selftests/sysctl/config requires CONFIG_TEST_SYSCTL=y. But
>>>      since lib/test_sysctl.c doesn't use module_init(), the
>>>      test_syscall is not listed under /sys/module/ and the
>>>      test script gives up.
>>>    - Even if we make CONFIG_TEST_SYSCTL=m, the test script checks
>>>      /sys/modules/test_sysctl before loading module and gives up.
>>>    - Ayway, since the test module introduces useless sysctl
>>>      interface to the kernel, it would better be a module.
>>>
>>> This series includes fixes for above 3 points.
>>>    - Fix lib/test_sysctl.c to use module_init()
>>>    - Fix tools/testing/selftests/sysctl/sysctl.sh to try to load
>>>      test module if it is not loaded (nor embedded).
>>>    - Fix tools/testing/selftests/sysctl/config to require
>>>      CONFIG_TEST_SYSCTL=m, not y.
>>>
>>> Thank you,
>>>
>>> ---
>>>
>>> Masami Hiramatsu (4):
>>>         lib: Make prime number generator independently selectable
>>>         lib: Make test_sysctl initialized as module
>>>         selftests/sysctl: Fix to load test_sysctl module
>>>         selftests/sysctl: Make sysctl test driver as a module
>>>
>>>
>>>    lib/math/Kconfig                         |    7 ++++++-
>>>    lib/test_sysctl.c                        |    2 +-
>>>    tools/testing/selftests/sysctl/config    |    2 +-
>>>    tools/testing/selftests/sysctl/sysctl.sh |   13 ++-----------
>>>    4 files changed, 10 insertions(+), 14 deletions(-)
>>>
>>> --
>>> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
>>>
>>
>> Thanks Masami. I see Kees reviewing patches. I will wait for Luis to
>> weigh in on patch 2 before pulling this series in.
> 
> OK, Thanks Shuah!
> 
> 

Applied to linux-kselftest next for Linux 5.8-rc1.

thanks,
-- Shuah