diff mbox series

docs: fault-injection: fix non-working usage of negative values

Message ID 20210603125841.27436-1-wsa+renesas@sang-engineering.com (mailing list archive)
State Mainlined
Commit 005747526d4f3c2ec995891e95cb7625161022f9
Delegated to: Geert Uytterhoeven
Headers show
Series docs: fault-injection: fix non-working usage of negative values | expand

Commit Message

Wolfram Sang June 3, 2021, 12:58 p.m. UTC 
Fault injection uses debugfs in a way that the provided values via sysfs
are interpreted as u64. Providing negative numbers results in an error:

/sys/kernel/debug/fail_function# echo -1 > times
sh: write error: Invalid argument

Update the docs and examples to use "printf %#x <val>" in these cases.
For "retval", reword the paragraph a little and fix a typo.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 .../fault-injection/fault-injection.rst       | 24 +++++++++++--------
 1 file changed, 14 insertions(+), 10 deletions(-)

Comments

Geert Uytterhoeven June 3, 2021, 1:27 p.m. UTC | #1 
Hi Wolfram,

On Thu, Jun 3, 2021 at 2:59 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Fault injection uses debugfs in a way that the provided values via sysfs
> are interpreted as u64. Providing negative numbers results in an error:
>
> /sys/kernel/debug/fail_function# echo -1 > times
> sh: write error: Invalid argument

Interesting...

> Update the docs and examples to use "printf %#x <val>" in these cases.
> For "retval", reword the paragraph a little and fix a typo.

Presumably this is something that used to work, but was broken (not
so) recently.  Perhaps the code should be fixed instead?

The code uses debugfs_create_atomic_t(), and atomic_t is signed, so
I'd expect negative numbers to work.

Gr{oetje,eeting}s,

                        Geert
Wolfram Sang June 3, 2021, 3:35 p.m. UTC | #2 
Yeah, I also assume that it used to work at some time. Didn't bother to
investigate, though.

> The code uses debugfs_create_atomic_t(), and atomic_t is signed, so
> I'd expect negative numbers to work.

In debugfs, it is all cast from unsigned long long to something. Check
simple_attr_write() where the value is obtained with kstrtoull. The code
fix would be to not use simple_attr, I think. I opted for the doc fix.
Jonathan Corbet June 14, 2021, 9:58 p.m. UTC | #3 
Wolfram Sang <wsa+renesas@sang-engineering.com> writes:

> Fault injection uses debugfs in a way that the provided values via sysfs
> are interpreted as u64. Providing negative numbers results in an error:
>
> /sys/kernel/debug/fail_function# echo -1 > times
> sh: write error: Invalid argument
>
> Update the docs and examples to use "printf %#x <val>" in these cases.
> For "retval", reword the paragraph a little and fix a typo.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  .../fault-injection/fault-injection.rst       | 24 +++++++++++--------
>  1 file changed, 14 insertions(+), 10 deletions(-)

Applied, thanks.

jon
diff mbox series

Patch

diff --git a/Documentation/fault-injection/fault-injection.rst b/Documentation/fault-injection/fault-injection.rst
index 31ecfe44e5b4..f47d05ed0d94 100644
--- a/Documentation/fault-injection/fault-injection.rst
+++ b/Documentation/fault-injection/fault-injection.rst
@@ -78,8 +78,10 @@  configuration of fault-injection capabilities.
 
 - /sys/kernel/debug/fail*/times:
 
-	specifies how many times failures may happen at most.
-	A value of -1 means "no limit".
+	specifies how many times failures may happen at most. A value of -1
+	means "no limit". Note, though, that this file only accepts unsigned
+	values. So, if you want to specify -1, you better use 'printf' instead
+	of 'echo', e.g.: $ printf %#x -1 > times
 
 - /sys/kernel/debug/fail*/space:
 
@@ -167,11 +169,13 @@  configuration of fault-injection capabilities.
 	- ERRNO: retval must be -1 to -MAX_ERRNO (-4096).
 	- ERR_NULL: retval must be 0 or -1 to -MAX_ERRNO (-4096).
 
-- /sys/kernel/debug/fail_function/<functiuon-name>/retval:
+- /sys/kernel/debug/fail_function/<function-name>/retval:
 
-	specifies the "error" return value to inject to the given
-	function for given function. This will be created when
-	user specifies new injection entry.
+	specifies the "error" return value to inject to the given function.
+	This will be created when the user specifies a new injection entry.
+	Note that this file only accepts unsigned values. So, if you want to
+	use a negative errno, you better use 'printf' instead of 'echo', e.g.:
+	$ printf %#x -12 > retval
 
 Boot option
 ^^^^^^^^^^^
@@ -255,7 +259,7 @@  Application Examples
     echo Y > /sys/kernel/debug/$FAILTYPE/task-filter
     echo 10 > /sys/kernel/debug/$FAILTYPE/probability
     echo 100 > /sys/kernel/debug/$FAILTYPE/interval
-    echo -1 > /sys/kernel/debug/$FAILTYPE/times
+    printf %#x -1 > /sys/kernel/debug/$FAILTYPE/times
     echo 0 > /sys/kernel/debug/$FAILTYPE/space
     echo 2 > /sys/kernel/debug/$FAILTYPE/verbose
     echo 1 > /sys/kernel/debug/$FAILTYPE/ignore-gfp-wait
@@ -309,7 +313,7 @@  Application Examples
     echo N > /sys/kernel/debug/$FAILTYPE/task-filter
     echo 10 > /sys/kernel/debug/$FAILTYPE/probability
     echo 100 > /sys/kernel/debug/$FAILTYPE/interval
-    echo -1 > /sys/kernel/debug/$FAILTYPE/times
+    printf %#x -1 > /sys/kernel/debug/$FAILTYPE/times
     echo 0 > /sys/kernel/debug/$FAILTYPE/space
     echo 2 > /sys/kernel/debug/$FAILTYPE/verbose
     echo 1 > /sys/kernel/debug/$FAILTYPE/ignore-gfp-wait
@@ -336,11 +340,11 @@  Application Examples
     FAILTYPE=fail_function
     FAILFUNC=open_ctree
     echo $FAILFUNC > /sys/kernel/debug/$FAILTYPE/inject
-    echo -12 > /sys/kernel/debug/$FAILTYPE/$FAILFUNC/retval
+    printf %#x -12 > /sys/kernel/debug/$FAILTYPE/$FAILFUNC/retval
     echo N > /sys/kernel/debug/$FAILTYPE/task-filter
     echo 100 > /sys/kernel/debug/$FAILTYPE/probability
     echo 0 > /sys/kernel/debug/$FAILTYPE/interval
-    echo -1 > /sys/kernel/debug/$FAILTYPE/times
+    printf %#x -1 > /sys/kernel/debug/$FAILTYPE/times
     echo 0 > /sys/kernel/debug/$FAILTYPE/space
     echo 1 > /sys/kernel/debug/$FAILTYPE/verbose