| Message ID | 151427438796.32561.4235654585430455286.stgit@devbox |
|---|---|
| Headers | show |
| Series | Separate error injection table from kprobes | expand |
On Tue, Dec 26, 2017 at 04:46:28PM +0900, Masami Hiramatsu wrote: > Hi Josef and Alexei, > > Here are the 2nd version of patches to moving error injection > table from kprobes. In this series I did a small fixes and > add function-based fault injection. > > Here is the previous version: > > https://lkml.org/lkml/2017/12/22/554 > > There are 2 main reasons why I separate it from kprobes. > > - kprobes users can modify execution path not only at > error-injection whitelist functions but also other > functions. I don't like to suggest user that such > limitation is from kprobes itself. > > - This error injection information is also useful for > ftrace (function-hook) and livepatch. It should not > be limited by CONFIG_KPROBES. > > So I introduced CONFIG_FUNCTION_ERROR_INJECTION for this feature. > Also CONFIG_FAIL_FUNCTION is added, which provides function-based > error injection interface via debugfs following fault-injection > framework. See [4/4]. > > Any thoughts? Sorry Masami, I've been on vacation for the last two weeks. This approach is fine by me, if we want to allow other mechanisms other than bpf to use this functionality then hooray. I'll do a proper review when you post v3, just wanted to let you know I wasn't ignoring you. Thanks, Josef
On Thu, 4 Jan 2018 11:07:16 -0500 Josef Bacik <josef@toxicpanda.com> wrote: > On Tue, Dec 26, 2017 at 04:46:28PM +0900, Masami Hiramatsu wrote: > > Hi Josef and Alexei, > > > > Here are the 2nd version of patches to moving error injection > > table from kprobes. In this series I did a small fixes and > > add function-based fault injection. > > > > Here is the previous version: > > > > https://lkml.org/lkml/2017/12/22/554 > > > > There are 2 main reasons why I separate it from kprobes. > > > > - kprobes users can modify execution path not only at > > error-injection whitelist functions but also other > > functions. I don't like to suggest user that such > > limitation is from kprobes itself. > > > > - This error injection information is also useful for > > ftrace (function-hook) and livepatch. It should not > > be limited by CONFIG_KPROBES. > > > > So I introduced CONFIG_FUNCTION_ERROR_INJECTION for this feature. > > Also CONFIG_FAIL_FUNCTION is added, which provides function-based > > error injection interface via debugfs following fault-injection > > framework. See [4/4]. > > > > Any thoughts? > > Sorry Masami, I've been on vacation for the last two weeks. This approach is > fine by me, if we want to allow other mechanisms other than bpf to use this > functionality then hooray. I'll do a proper review when you post v3, just > wanted to let you know I wasn't ignoring you. Thanks, Yeah, thank you for the kindful notice ;) BTW, could you tell me how I can run your test case? When I tried to build the tests (samples/bpf) I got below error and stopped. [mhiramat@devbox bpf]$ LANG=C make make -C ../../ /home/mhiramat/ksrc/linux/samples/bpf/ make[1]: Entering directory '/home/mhiramat/ksrc/linux' CHK include/config/kernel.release CHK include/generated/uapi/linux/version.h CHK include/generated/utsrelease.h CHK include/generated/bounds.h CHK include/generated/timeconst.h CHK include/generated/asm-offsets.h CALL scripts/checksyscalls.sh DESCEND objtool CHK scripts/mod/devicetable-offsets.h HOSTCC /home/mhiramat/ksrc/linux/samples/bpf/test_lru_dist.o /home/mhiramat/ksrc/linux/samples/bpf/test_lru_dist.c:39:8: error: redefinition of 'struct list_head' struct list_head { ^~~~~~~~~ In file included from /home/mhiramat/ksrc/linux/samples/bpf/test_lru_dist.c:9:0: ./tools/include/linux/types.h:69:8: note: originally defined here struct list_head { ^~~~~~~~~ make[2]: *** [scripts/Makefile.host:107: /home/mhiramat/ksrc/linux/samples/bpf/test_lru_dist.o] Error 1 make[1]: *** [Makefile:1675: /home/mhiramat/ksrc/linux/samples/bpf/] Error 2 make[1]: Leaving directory '/home/mhiramat/ksrc/linux' make: *** [Makefile:204: all] Error 2 Thank you,
Hi Josef and Alexei, Here are the 2nd version of patches to moving error injection table from kprobes. In this series I did a small fixes and add function-based fault injection. Here is the previous version: https://lkml.org/lkml/2017/12/22/554 There are 2 main reasons why I separate it from kprobes. - kprobes users can modify execution path not only at error-injection whitelist functions but also other functions. I don't like to suggest user that such limitation is from kprobes itself. - This error injection information is also useful for ftrace (function-hook) and livepatch. It should not be limited by CONFIG_KPROBES. So I introduced CONFIG_FUNCTION_ERROR_INJECTION for this feature. Also CONFIG_FAIL_FUNCTION is added, which provides function-based error injection interface via debugfs following fault-injection framework. See [4/4]. Any thoughts? BTW, I think we should add an error-range description in ALLOW_ERROR_INJECTION() macro. If user sets a success return value and override it by mistake, caller must break data or cause kernel panic. Thank you, --- Masami Hiramatsu (4): tracing/kprobe: bpf: Check error injectable event is on function entry tracing/kprobe: bpf: Compare instruction pointer with original one error-injection: Separate error-injection from kprobe error-injection: Support fault injection framework Documentation/fault-injection/fault-injection.txt | 5 + arch/Kconfig | 2 arch/x86/Kconfig | 2 arch/x86/include/asm/error-injection.h | 12 + arch/x86/kernel/kprobes/ftrace.c | 14 - arch/x86/lib/Makefile | 2 arch/x86/lib/error-inject.c | 19 ++ fs/btrfs/disk-io.c | 2 fs/btrfs/free-space-cache.c | 2 include/asm-generic/error-injection.h | 20 ++ include/asm-generic/vmlinux.lds.h | 14 + include/linux/bpf.h | 12 - include/linux/error-injection.h | 21 ++ include/linux/kprobes.h | 1 include/linux/module.h | 6 - kernel/Makefile | 1 kernel/fail_function.c | 169 ++++++++++++++++++ kernel/kprobes.c | 163 ----------------- kernel/module.c | 8 - kernel/trace/Kconfig | 4 kernel/trace/bpf_trace.c | 9 - kernel/trace/trace_kprobe.c | 32 +-- kernel/trace/trace_probe.h | 12 + lib/Kconfig.debug | 14 + lib/Makefile | 1 lib/error-inject.c | 198 +++++++++++++++++++++ 26 files changed, 506 insertions(+), 239 deletions(-) create mode 100644 arch/x86/include/asm/error-injection.h create mode 100644 arch/x86/lib/error-inject.c create mode 100644 include/asm-generic/error-injection.h create mode 100644 include/linux/error-injection.h create mode 100644 kernel/fail_function.c create mode 100644 lib/error-inject.c -- Masami Hiramatsu (Linaro)