All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] lib/Kconfig.debug: Make TEST_LOCKUP depend on module
@ 2020-07-24  1:50 Tiezhu Yang
  2020-07-24  1:50 ` [PATCH 2/2] lib/test_lockup.c: Fix return value of test_lockup_init() Tiezhu Yang
  2020-07-24  2:11 ` [PATCH 1/2] lib/Kconfig.debug: Make TEST_LOCKUP depend on module Guenter Roeck
  0 siblings, 2 replies; 4+ messages in thread
From: Tiezhu Yang @ 2020-07-24  1:50 UTC (permalink / raw)
  To: Andrew Morton, Konstantin Khlebnikov, Guenter Roeck, Kees Cook
  Cc: linux-kernel, Xuefeng Li

Since test_lockup is a test module to generate lockups, it is better to
limit TEST_LOCKUP to module (=m) or disabled (=n) because we can not use
the module parameters when CONFIG_TEST_LOCKUP=y.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 lib/Kconfig.debug | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 9ad9210..18b7c67 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1035,6 +1035,7 @@ config WQ_WATCHDOG
 
 config TEST_LOCKUP
 	tristate "Test module to generate lockups"
+	depends on m
 	help
 	  This builds the "test_lockup" module that helps to make sure
 	  that watchdogs and lockup detectors are working properly.
-- 
2.1.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] lib/test_lockup.c: Fix return value of test_lockup_init()
  2020-07-24  1:50 [PATCH 1/2] lib/Kconfig.debug: Make TEST_LOCKUP depend on module Tiezhu Yang
@ 2020-07-24  1:50 ` Tiezhu Yang
  2020-07-24  2:12   ` Guenter Roeck
  2020-07-24  2:11 ` [PATCH 1/2] lib/Kconfig.debug: Make TEST_LOCKUP depend on module Guenter Roeck
  1 sibling, 1 reply; 4+ messages in thread
From: Tiezhu Yang @ 2020-07-24  1:50 UTC (permalink / raw)
  To: Andrew Morton, Konstantin Khlebnikov, Guenter Roeck, Kees Cook
  Cc: linux-kernel, Xuefeng Li

Since filp_open() returns an error pointer, we should use IS_ERR() to
check the return value and then return PTR_ERR() if failed to get the
actual return value instead of always -EINVAL.

E.g. without this patch:

[root@localhost loongson]# ls no_such_file
ls: cannot access no_such_file: No such file or directory
[root@localhost loongson]# modprobe test_lockup file_path=no_such_file lock_sb_umount time_secs=60 state=S
modprobe: ERROR: could not insert 'test_lockup': Invalid argument
[root@localhost loongson]# dmesg | tail -1
[  126.100596] test_lockup: cannot find file_path

With this patch:

[root@localhost loongson]# ls no_such_file
ls: cannot access no_such_file: No such file or directory
[root@localhost loongson]# modprobe test_lockup file_path=no_such_file lock_sb_umount time_secs=60 state=S
modprobe: ERROR: could not insert 'test_lockup': Unknown symbol in module, or unknown parameter (see dmesg)
[root@localhost loongson]# dmesg | tail -1
[   95.134362] test_lockup: failed to open no_such_file: -2

Fixes: aecd42df6d39 ("lib/test_lockup.c: add parameters for locking generic vfs locks")
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 lib/test_lockup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/test_lockup.c b/lib/test_lockup.c
index bd7c7ff..e720276 100644
--- a/lib/test_lockup.c
+++ b/lib/test_lockup.c
@@ -512,8 +512,8 @@ static int __init test_lockup_init(void)
 	if (test_file_path[0]) {
 		test_file = filp_open(test_file_path, O_RDONLY, 0);
 		if (IS_ERR(test_file)) {
-			pr_err("cannot find file_path\n");
-			return -EINVAL;
+			pr_err("failed to open %s: %ld\n", test_file_path, PTR_ERR(test_file));
+			return PTR_ERR(test_file);
 		}
 		test_inode = file_inode(test_file);
 	} else if (test_lock_inode ||
-- 
2.1.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] lib/Kconfig.debug: Make TEST_LOCKUP depend on module
  2020-07-24  1:50 [PATCH 1/2] lib/Kconfig.debug: Make TEST_LOCKUP depend on module Tiezhu Yang
  2020-07-24  1:50 ` [PATCH 2/2] lib/test_lockup.c: Fix return value of test_lockup_init() Tiezhu Yang
@ 2020-07-24  2:11 ` Guenter Roeck
  1 sibling, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2020-07-24  2:11 UTC (permalink / raw)
  To: Tiezhu Yang, Andrew Morton, Konstantin Khlebnikov, Kees Cook
  Cc: linux-kernel, Xuefeng Li

On 7/23/20 6:50 PM, Tiezhu Yang wrote:
> Since test_lockup is a test module to generate lockups, it is better to
> limit TEST_LOCKUP to module (=m) or disabled (=n) because we can not use
> the module parameters when CONFIG_TEST_LOCKUP=y.
> 
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>

Makes sense.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  lib/Kconfig.debug | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 9ad9210..18b7c67 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1035,6 +1035,7 @@ config WQ_WATCHDOG
>  
>  config TEST_LOCKUP
>  	tristate "Test module to generate lockups"
> +	depends on m
>  	help
>  	  This builds the "test_lockup" module that helps to make sure
>  	  that watchdogs and lockup detectors are working properly.
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] lib/test_lockup.c: Fix return value of test_lockup_init()
  2020-07-24  1:50 ` [PATCH 2/2] lib/test_lockup.c: Fix return value of test_lockup_init() Tiezhu Yang
@ 2020-07-24  2:12   ` Guenter Roeck
  0 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2020-07-24  2:12 UTC (permalink / raw)
  To: Tiezhu Yang, Andrew Morton, Konstantin Khlebnikov, Kees Cook
  Cc: linux-kernel, Xuefeng Li

On 7/23/20 6:50 PM, Tiezhu Yang wrote:
> Since filp_open() returns an error pointer, we should use IS_ERR() to
> check the return value and then return PTR_ERR() if failed to get the
> actual return value instead of always -EINVAL.
> 
> E.g. without this patch:
> 
> [root@localhost loongson]# ls no_such_file
> ls: cannot access no_such_file: No such file or directory
> [root@localhost loongson]# modprobe test_lockup file_path=no_such_file lock_sb_umount time_secs=60 state=S
> modprobe: ERROR: could not insert 'test_lockup': Invalid argument
> [root@localhost loongson]# dmesg | tail -1
> [  126.100596] test_lockup: cannot find file_path
> 
> With this patch:
> 
> [root@localhost loongson]# ls no_such_file
> ls: cannot access no_such_file: No such file or directory
> [root@localhost loongson]# modprobe test_lockup file_path=no_such_file lock_sb_umount time_secs=60 state=S
> modprobe: ERROR: could not insert 'test_lockup': Unknown symbol in module, or unknown parameter (see dmesg)
> [root@localhost loongson]# dmesg | tail -1
> [   95.134362] test_lockup: failed to open no_such_file: -2
> 
> Fixes: aecd42df6d39 ("lib/test_lockup.c: add parameters for locking generic vfs locks")
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  lib/test_lockup.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/test_lockup.c b/lib/test_lockup.c
> index bd7c7ff..e720276 100644
> --- a/lib/test_lockup.c
> +++ b/lib/test_lockup.c
> @@ -512,8 +512,8 @@ static int __init test_lockup_init(void)
>  	if (test_file_path[0]) {
>  		test_file = filp_open(test_file_path, O_RDONLY, 0);
>  		if (IS_ERR(test_file)) {
> -			pr_err("cannot find file_path\n");
> -			return -EINVAL;
> +			pr_err("failed to open %s: %ld\n", test_file_path, PTR_ERR(test_file));
> +			return PTR_ERR(test_file);
>  		}
>  		test_inode = file_inode(test_file);
>  	} else if (test_lock_inode ||
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-07-24  2:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-24  1:50 [PATCH 1/2] lib/Kconfig.debug: Make TEST_LOCKUP depend on module Tiezhu Yang
2020-07-24  1:50 ` [PATCH 2/2] lib/test_lockup.c: Fix return value of test_lockup_init() Tiezhu Yang
2020-07-24  2:12   ` Guenter Roeck
2020-07-24  2:11 ` [PATCH 1/2] lib/Kconfig.debug: Make TEST_LOCKUP depend on module Guenter Roeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.