Re: [PATCH bpf-next v4 5/5] error-injection: Support fault injection framework

[Akinobu Mita]

2018-01-12 1:15 GMT+09:00 Masami Hiramatsu <mhiramat@xxxxxxxxxx>:
> On Thu, 11 Jan 2018 23:44:57 +0900
> Akinobu Mita <akinobu.mita@xxxxxxxxx> wrote:
>
>> 2018-01-11 9:51 GMT+09:00 Masami Hiramatsu <mhiramat@xxxxxxxxxx>:
>> > Support in-kernel fault-injection framework via debugfs.
>> > This allows you to inject a conditional error to specified
>> > function using debugfs interfaces.
>> >
>> > Here is the result of test script described in
>> > Documentation/fault-injection/fault-injection.txt
>> >
>> >   ===========
>> >   # ./test_fail_function.sh
>> >   1+0 records in
>> >   1+0 records out
>> >   1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.0227404 s, 46.1 MB/s
>> >   btrfs-progs v4.4
>> >   See http://btrfs.wiki.kernel.org for more information.
>> >
>> >   Label:              (null)
>> >   UUID:               bfa96010-12e9-4360-aed0-42eec7af5798
>> >   Node size:          16384
>> >   Sector size:        4096
>> >   Filesystem size:    1001.00MiB
>> >   Block group profiles:
>> >     Data:             single            8.00MiB
>> >     Metadata:         DUP              58.00MiB
>> >     System:           DUP              12.00MiB
>> >   SSD detected:       no
>> >   Incompat features:  extref, skinny-metadata
>> >   Number of devices:  1
>> >   Devices:
>> >      ID        SIZE  PATH
>> >       1  1001.00MiB  /dev/loop2
>> >
>> >   mount: mount /dev/loop2 on /opt/tmpmnt failed: Cannot allocate memory
>> >   SUCCESS!
>> >   ===========
>> >
>> >
>> > Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
>> > Reviewed-by: Josef Bacik <jbacik@xxxxxx>
>> > ---
>> >   Changes in v3:
>> >    - Check and adjust error value for each target function
>> >    - Clear kporbe flag for reuse
>> >    - Add more documents and example
>> > ---
>> >  Documentation/fault-injection/fault-injection.txt |   62 ++++++
>> >  kernel/Makefile                                   |    1
>> >  kernel/fail_function.c                            |  217 +++++++++++++++++++++
>> >  lib/Kconfig.debug                                 |   10 +
>> >  4 files changed, 290 insertions(+)
>> >  create mode 100644 kernel/fail_function.c
>> >
>> > diff --git a/Documentation/fault-injection/fault-injection.txt b/Documentation/fault-injection/fault-injection.txt
>> > index 918972babcd8..4aecbceef9d2 100644
>> > --- a/Documentation/fault-injection/fault-injection.txt
>> > +++ b/Documentation/fault-injection/fault-injection.txt
>> > @@ -30,6 +30,12 @@ o fail_mmc_request
>> >    injects MMC data errors on devices permitted by setting
>> >    debugfs entries under /sys/kernel/debug/mmc0/fail_mmc_request
>> >
>> > +o fail_function
>> > +
>> > +  injects error return on specific functions, which are marked by
>> > +  ALLOW_ERROR_INJECTION() macro, by setting debugfs entries
>> > +  under /sys/kernel/debug/fail_function. No boot option supported.
>> > +
>> >  Configure fault-injection capabilities behavior
>> >  -----------------------------------------------
>> >
>> > @@ -123,6 +129,24 @@ configuration of fault-injection capabilities.
>> >         default is 'N', setting it to 'Y' will disable failure injections
>> >         when dealing with private (address space) futexes.
>> >
>> > +- /sys/kernel/debug/fail_function/inject:
>> > +
>> > +       specifies the target function of error injection by name.
>> > +
>> > +- /sys/kernel/debug/fail_function/retval:
>> > +
>> > +       specifies the "error" return value to inject to the given
>> > +       function.
>> > +
>>
>> Is it possible to inject errors into multiple functions at the same time?
>
> Yes, it is.
>
>> If so, it will be more useful to support it in the fault injection, too.
>> Because some kind of bugs are caused by the combination of errors.
>> (e.g. another error in an error path)
>>
>> I suggest the following interface.
>>
>> - /sys/kernel/debug/fail_function/inject:
>>
>>   specifies the target function of error injection by name.
>>   /sys/kernel/debug/fail_function/<func>/ directory will be created.
>>
>> - /sys/kernel/debug/fail_function/uninject:
>>
>>   specifies the target function of error injection by name that is
>>   currently being injected.  /sys/kernel/debug/fail_function/<func>/
>>   directory will be removed.
>>
>> - /sys/kernel/debug/fail_function/<func>/retval:
>>
>>   specifies the "error" return value to inject to the given function.
>
> OK, it is easy to make it. But also we might need to consider using bpf
> if we do such complex error injection.
>
> BTW, would we need "uninject" file? or just make inject file accept
> "!function" syntax to remove function as ftrace does?

It also sounds good.  Either way is fine with me.