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
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
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
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
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
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) {
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?
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
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
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
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
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
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
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
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
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!
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
linux-kselftest-mirror@lists.linaro.org
-
Kees Cook -
Luis Chamberlain -
Masami Hiramatsu -
Shuah Khan