Michal Hocko
2018-Apr-24 17:38 UTC
[PATCH v3] kvmalloc: always use vmalloc if CONFIG_DEBUG_SG
On Tue 24-04-18 13:28:49, Mikulas Patocka wrote:> > > On Tue, 24 Apr 2018, Michal Hocko wrote: > > > On Tue 24-04-18 13:00:11, Mikulas Patocka wrote: > > > > > > > > > On Tue, 24 Apr 2018, Michal Hocko wrote: > > > > > > > On Tue 24-04-18 11:50:30, Mikulas Patocka wrote: > > > > > > > > > > > > > > > On Tue, 24 Apr 2018, Michal Hocko wrote: > > > > > > > > > > > On Mon 23-04-18 20:06:16, Mikulas Patocka wrote: > > > > > > [...] > > > > > > > @@ -404,6 +405,12 @@ void *kvmalloc_node(size_t size, gfp_t f > > > > > > > */ > > > > > > > WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL); > > > > > > > > > > > > > > +#ifdef CONFIG_DEBUG_SG > > > > > > > + /* Catch bugs when the caller uses DMA API on the result of kvmalloc. */ > > > > > > > + if (!(prandom_u32_max(2) & 1)) > > > > > > > + goto do_vmalloc; > > > > > > > +#endif > > > > > > > > > > > > I really do not think there is anything DEBUG_SG specific here. Why you > > > > > > simply do not follow should_failslab path or even reuse the function? > > > > > > > > > > CONFIG_DEBUG_SG is enabled by default in RHEL and Fedora debug kernel (if > > > > > you don't like CONFIG_DEBUG_SG, pick any other option that is enabled > > > > > there). > > > > > > > > Are you telling me that you are shaping a debugging functionality basing > > > > on what RHEL has enabled? And you call me evil. This is just rediculous. > > > > > > > > > Fail-injection framework is if off by default and it must be explicitly > > > > > enabled and configured by the user - and most users won't enable it. > > > > > > > > It can be enabled easily. And if you care enough for your debugging > > > > kernel then just make it enabled unconditionally. > > > > > > So, should we add a new option CONFIG_KVMALLOC_FALLBACK_DEFAULT? I'm not > > > quite sure if 3 lines of debugging code need an extra option, but if you > > > don't want to reuse any existing debug option, it may be possible. Adding > > > it to the RHEL debug kernel would be trivial. > > > > Wouldn't it be equally trivial to simply enable the fault injection? You > > would get additional failure paths testing as a bonus. > > The RHEL and Fedora debugging kernels are compiled with fault injection. > But the fault-injection framework will do nothing unless it is enabled by > a kernel parameter or debugfs write. > > Most users don't know about the fault injection kernel parameters or > debugfs files and won't enabled it. We need a CONFIG_ option to enable it > by default in the debugging kernels (and we could add a kernel parameter > to override the default, fine-tune the fallback probability etc.)If it is a real issue to install the debugging kernel with the required kernel parameter then I a config option for the default on makes sense to me. -- Michal Hocko SUSE Labs
This patch reorders Kconfig entries, so that menuconfig displays proper indentation. Signed-off-by: Mikulas Patocka <mpatocka at redhat.com> --- lib/Kconfig.debug | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) Index: linux-2.6/lib/Kconfig.debug ==================================================================--- linux-2.6.orig/lib/Kconfig.debug 2018-04-16 21:08:36.000000000 +0200 +++ linux-2.6/lib/Kconfig.debug 2018-04-25 15:56:16.000000000 +0200 @@ -1503,6 +1503,10 @@ config NETDEV_NOTIFIER_ERROR_INJECT If unsure, say N. +config FUNCTION_ERROR_INJECTION + def_bool y + depends on HAVE_FUNCTION_ERROR_INJECTION && KPROBES + config FAULT_INJECTION bool "Fault-injection framework" depends on DEBUG_KERNEL @@ -1510,10 +1514,6 @@ config FAULT_INJECTION Provide fault-injection framework. For more details, see Documentation/fault-injection/. -config FUNCTION_ERROR_INJECTION - def_bool y - depends on HAVE_FUNCTION_ERROR_INJECTION && KPROBES - config FAILSLAB bool "Fault-injection capability for kmalloc" depends on FAULT_INJECTION @@ -1544,16 +1544,6 @@ config FAIL_IO_TIMEOUT Only works with drivers that use the generic timeout handling, for others it wont do anything. -config FAIL_MMC_REQUEST - bool "Fault-injection capability for MMC IO" - depends on FAULT_INJECTION_DEBUG_FS && MMC - help - Provide fault-injection capability for MMC IO. - This will make the mmc core return data errors. This is - useful to test the error handling in the mmc block device - and to test how the mmc host driver handles retries from - the block device. - config FAIL_FUTEX bool "Fault-injection capability for futexes" select DEBUG_FS @@ -1561,6 +1551,12 @@ config FAIL_FUTEX help Provide fault-injection capability for futexes. +config FAULT_INJECTION_DEBUG_FS + bool "Debugfs entries for fault-injection capabilities" + depends on FAULT_INJECTION && SYSFS && DEBUG_FS + help + Enable configuration of fault-injection capabilities via debugfs. + config FAIL_FUNCTION bool "Fault-injection capability for functions" depends on FAULT_INJECTION_DEBUG_FS && FUNCTION_ERROR_INJECTION @@ -1571,11 +1567,15 @@ config FAIL_FUNCTION an error value and have to handle it. This is useful to test the error handling in various subsystems. -config FAULT_INJECTION_DEBUG_FS - bool "Debugfs entries for fault-injection capabilities" - depends on FAULT_INJECTION && SYSFS && DEBUG_FS +config FAIL_MMC_REQUEST + bool "Fault-injection capability for MMC IO" + depends on FAULT_INJECTION_DEBUG_FS && MMC help - Enable configuration of fault-injection capabilities via debugfs. + Provide fault-injection capability for MMC IO. + This will make the mmc core return data errors. This is + useful to test the error handling in the mmc block device + and to test how the mmc host driver handles retries from + the block device. config FAULT_INJECTION_STACKTRACE_FILTER bool "stacktrace filter for fault-injection capabilities"
Mikulas Patocka
2018-Apr-25 20:02 UTC
[PATCH v4] fault-injection: introduce kvmalloc fallback options
On Tue, 24 Apr 2018, Michal Hocko wrote:> > > Wouldn't it be equally trivial to simply enable the fault injection? You > > > would get additional failure paths testing as a bonus. > > > > The RHEL and Fedora debugging kernels are compiled with fault injection. > > But the fault-injection framework will do nothing unless it is enabled by > > a kernel parameter or debugfs write. > > > > Most users don't know about the fault injection kernel parameters or > > debugfs files and won't enabled it. We need a CONFIG_ option to enable it > > by default in the debugging kernels (and we could add a kernel parameter > > to override the default, fine-tune the fallback probability etc.) > > If it is a real issue to install the debugging kernel with the required > kernel parameter then I a config option for the default on makes sense > to me.Yes - the debug kernels use the same default kernel parameters as non-debug kernels and it is expected that all debug features are enabled by default. Here I'm sending the patch using the fault-injection framework and the new option CONFIG_FAIL_KVMALLOC_FALLBACK_PROBABILITY. Mikulas From: Mikulas Patocka <mpatocka at redhat.com> Subject: [PATCH v4] fault-injection: introduce kvmalloc fallback options This patch introduces a fault-injection option "kvmalloc_fallback". This option makes kvmalloc randomly fall back to vmalloc. Unfortunatelly, some kernel code has bugs - it uses kvmalloc and then uses DMA-API on the returned memory or frees it with kfree. Such bugs were found in the virtio-net driver, dm-integrity or RHEL7 powerpc-specific code. This options helps to test for these bugs. The patch introduces a config option FAIL_KVMALLOC_FALLBACK_PROBABILITY. It can be enabled in distribution debug kernels, so that kvmalloc abuse can be tested by the users. The default can be overriden with "kvmalloc_fallback" parameter or in /sys/kernel/debug/kvmalloc_fallback/. Signed-off-by: Mikulas Patocka <mpatocka at redhat.com> --- Documentation/fault-injection/fault-injection.txt | 7 +++++ include/linux/fault-inject.h | 9 +++--- kernel/futex.c | 2 - lib/Kconfig.debug | 15 +++++++++++ mm/failslab.c | 2 - mm/page_alloc.c | 2 - mm/util.c | 30 ++++++++++++++++++++++ 7 files changed, 60 insertions(+), 7 deletions(-) Index: linux-2.6/Documentation/fault-injection/fault-injection.txt ==================================================================--- linux-2.6.orig/Documentation/fault-injection/fault-injection.txt 2018-04-16 21:08:34.000000000 +0200 +++ linux-2.6/Documentation/fault-injection/fault-injection.txt 2018-04-25 21:36:36.000000000 +0200 @@ -15,6 +15,12 @@ o fail_page_alloc injects page allocation failures. (alloc_pages(), get_free_pages(), ...) +o kvmalloc_faillback + + makes the function kvmalloc randonly fall back to vmalloc. This could be used + to detects bugs such as using DMA-API on the result of kvmalloc or freeing + the result of kvmalloc with free. + o fail_futex injects futex deadlock and uaddr fault errors. @@ -167,6 +173,7 @@ use the boot option: failslab fail_page_alloc+ kvmalloc_faillback fail_make_request fail_futex mmc_core.fail_request=<interval>,<probability>,<space>,<times> Index: linux-2.6/include/linux/fault-inject.h ==================================================================--- linux-2.6.orig/include/linux/fault-inject.h 2018-04-16 21:08:36.000000000 +0200 +++ linux-2.6/include/linux/fault-inject.h 2018-04-25 21:38:22.000000000 +0200 @@ -31,17 +31,18 @@ struct fault_attr { struct dentry *dname; }; -#define FAULT_ATTR_INITIALIZER { \ +#define FAULT_ATTR_INITIALIZER(p) { \ + .probability = (p), \ .interval = 1, \ - .times = ATOMIC_INIT(1), \ + .times = ATOMIC_INIT((p) ? -1 : 1), \ + .verbose = (p) ? 0 : 2, \ .require_end = ULONG_MAX, \ .stacktrace_depth = 32, \ .ratelimit_state = RATELIMIT_STATE_INIT_DISABLED, \ - .verbose = 2, \ .dname = NULL, \ } -#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER +#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER(0) int setup_fault_attr(struct fault_attr *attr, char *str); bool should_fail(struct fault_attr *attr, ssize_t size); Index: linux-2.6/lib/Kconfig.debug ==================================================================--- linux-2.6.orig/lib/Kconfig.debug 2018-04-25 15:56:16.000000000 +0200 +++ linux-2.6/lib/Kconfig.debug 2018-04-25 21:39:45.000000000 +0200 @@ -1527,6 +1527,21 @@ config FAIL_PAGE_ALLOC help Provide fault-injection capability for alloc_pages(). +config FAIL_KVMALLOC_FALLBACK_PROBABILITY + int "Default kvmalloc fallback probability" + depends on FAULT_INJECTION + range 0 100 + default "0" + help + This option will make kvmalloc randomly fall back to vmalloc. + Normally, kvmalloc falls back to vmalloc only rarely, if memory + is fragmented. + + This option helps to detect hard-to-reproduce driver bugs, for + example using DMA API on the result of kvmalloc. + + The default may be overriden with the kvmalloc_faillback parameter. + config FAIL_MAKE_REQUEST bool "Fault-injection capability for disk IO" depends on FAULT_INJECTION && BLOCK Index: linux-2.6/mm/util.c ==================================================================--- linux-2.6.orig/mm/util.c 2018-04-25 15:48:39.000000000 +0200 +++ linux-2.6/mm/util.c 2018-04-25 21:43:31.000000000 +0200 @@ -14,6 +14,7 @@ #include <linux/hugetlb.h> #include <linux/vmalloc.h> #include <linux/userfaultfd_k.h> +#include <linux/fault-inject.h> #include <asm/sections.h> #include <linux/uaccess.h> @@ -377,6 +378,29 @@ unsigned long vm_mmap(struct file *file, } EXPORT_SYMBOL(vm_mmap); +#ifdef CONFIG_FAULT_INJECTION + +static struct fault_attr kvmalloc_fallback + FAULT_ATTR_INITIALIZER(CONFIG_FAIL_KVMALLOC_FALLBACK_PROBABILITY); + +static int __init setup_kvmalloc_fallback(char *str) +{ + return setup_fault_attr(&kvmalloc_fallback, str); +} + +__setup("kvmalloc_fallback=", setup_kvmalloc_fallback); + +#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS +static int __init kvmalloc_fallback_debugfs_init(void) +{ + fault_create_debugfs_attr("kvmalloc_fallback", NULL, &kvmalloc_fallback); + return 0; +} +late_initcall(kvmalloc_fallback_debugfs_init); +#endif + +#endif + /** * kvmalloc_node - attempt to allocate physically contiguous memory, but upon * failure, fall back to non-contiguous (vmalloc) allocation. @@ -404,6 +428,11 @@ void *kvmalloc_node(size_t size, gfp_t f */ WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL); +#ifdef CONFIG_FAULT_INJECTION + if (should_fail(&kvmalloc_fallback, size)) + goto do_vmalloc; +#endif + /* * We want to attempt a large physically contiguous block first because * it is less likely to fragment multiple larger blocks and therefore @@ -427,6 +456,7 @@ void *kvmalloc_node(size_t size, gfp_t f if (ret || size <= PAGE_SIZE) return ret; +do_vmalloc: __maybe_unused return __vmalloc_node_flags_caller(size, node, flags, __builtin_return_address(0)); } Index: linux-2.6/kernel/futex.c ==================================================================--- linux-2.6.orig/kernel/futex.c 2018-02-14 20:24:42.000000000 +0100 +++ linux-2.6/kernel/futex.c 2018-04-25 21:11:33.000000000 +0200 @@ -288,7 +288,7 @@ static struct { bool ignore_private; } fail_futex = { - .attr = FAULT_ATTR_INITIALIZER, + .attr = FAULT_ATTR_INITIALIZER(0), .ignore_private = false, }; Index: linux-2.6/mm/failslab.c ==================================================================--- linux-2.6.orig/mm/failslab.c 2018-04-16 21:08:36.000000000 +0200 +++ linux-2.6/mm/failslab.c 2018-04-25 21:11:40.000000000 +0200 @@ -9,7 +9,7 @@ static struct { bool ignore_gfp_reclaim; bool cache_filter; } failslab = { - .attr = FAULT_ATTR_INITIALIZER, + .attr = FAULT_ATTR_INITIALIZER(0), .ignore_gfp_reclaim = true, .cache_filter = false, }; Index: linux-2.6/mm/page_alloc.c ==================================================================--- linux-2.6.orig/mm/page_alloc.c 2018-04-16 21:08:36.000000000 +0200 +++ linux-2.6/mm/page_alloc.c 2018-04-25 21:11:47.000000000 +0200 @@ -3055,7 +3055,7 @@ static struct { bool ignore_gfp_reclaim; u32 min_order; } fail_page_alloc = { - .attr = FAULT_ATTR_INITIALIZER, + .attr = FAULT_ATTR_INITIALIZER(0), .ignore_gfp_reclaim = true, .ignore_gfp_highmem = true, .min_order = 1,
Randy Dunlap
2018-Apr-25 20:20 UTC
[PATCH v4] fault-injection: introduce kvmalloc fallback options
On 04/25/2018 01:02 PM, Mikulas Patocka wrote:> > > From: Mikulas Patocka <mpatocka at redhat.com> > Subject: [PATCH v4] fault-injection: introduce kvmalloc fallback options > > This patch introduces a fault-injection option "kvmalloc_fallback". This > option makes kvmalloc randomly fall back to vmalloc. > > Unfortunatelly, some kernel code has bugs - it uses kvmalloc and thenUnfortunately,> uses DMA-API on the returned memory or frees it with kfree. Such bugs were > found in the virtio-net driver, dm-integrity or RHEL7 powerpc-specific > code. This options helps to test for these bugs. > > The patch introduces a config option FAIL_KVMALLOC_FALLBACK_PROBABILITY. > It can be enabled in distribution debug kernels, so that kvmalloc abuse > can be tested by the users. The default can be overriden withoverridden> "kvmalloc_fallback" parameter or in /sys/kernel/debug/kvmalloc_fallback/. > > Signed-off-by: Mikulas Patocka <mpatocka at redhat.com> > > --- > Documentation/fault-injection/fault-injection.txt | 7 +++++ > include/linux/fault-inject.h | 9 +++--- > kernel/futex.c | 2 - > lib/Kconfig.debug | 15 +++++++++++ > mm/failslab.c | 2 - > mm/page_alloc.c | 2 - > mm/util.c | 30 ++++++++++++++++++++++ > 7 files changed, 60 insertions(+), 7 deletions(-) > > Index: linux-2.6/Documentation/fault-injection/fault-injection.txt > ==================================================================> --- linux-2.6.orig/Documentation/fault-injection/fault-injection.txt 2018-04-16 21:08:34.000000000 +0200 > +++ linux-2.6/Documentation/fault-injection/fault-injection.txt 2018-04-25 21:36:36.000000000 +0200 > @@ -15,6 +15,12 @@ o fail_page_alloc > > injects page allocation failures. (alloc_pages(), get_free_pages(), ...) > > +o kvmalloc_faillbackkvmalloc_fallback> + > + makes the function kvmalloc randonly fall back to vmalloc. This could be usedrandomly> + to detects bugs such as using DMA-API on the result of kvmalloc or freeing > + the result of kvmalloc with free. > + > o fail_futex > > injects futex deadlock and uaddr fault errors. > @@ -167,6 +173,7 @@ use the boot option: > > failslab> fail_page_alloc> + kvmalloc_faillbackkvmalloc_fallback> fail_make_request> fail_futex> mmc_core.fail_request=<interval>,<probability>,<space>,<times>> Index: linux-2.6/lib/Kconfig.debug > ==================================================================> --- linux-2.6.orig/lib/Kconfig.debug 2018-04-25 15:56:16.000000000 +0200 > +++ linux-2.6/lib/Kconfig.debug 2018-04-25 21:39:45.000000000 +0200 > @@ -1527,6 +1527,21 @@ config FAIL_PAGE_ALLOC > help > Provide fault-injection capability for alloc_pages(). > > +config FAIL_KVMALLOC_FALLBACK_PROBABILITY > + int "Default kvmalloc fallback probability" > + depends on FAULT_INJECTION > + range 0 100 > + default "0" > + help > + This option will make kvmalloc randomly fall back to vmalloc. > + Normally, kvmalloc falls back to vmalloc only rarely, if memory > + is fragmented. > + > + This option helps to detect hard-to-reproduce driver bugs, for > + example using DMA API on the result of kvmalloc. > + > + The default may be overriden with the kvmalloc_faillback parameter.overridden kvmalloc_fallback> + > config FAIL_MAKE_REQUEST > bool "Fault-injection capability for disk IO" > depends on FAULT_INJECTION && BLOCK-- ~Randy
On 04/25/2018 01:02 PM, Mikulas Patocka wrote:> This patch reorders Kconfig entries, so that menuconfig displays proper > indentation. > > Signed-off-by: Mikulas Patocka <mpatocka at redhat.com>Acked-by: Randy Dunlap <rdunlap at infradead.org> Tested-by: Randy Dunlap <rdunlap at infradead.org> Thanks.> --- > lib/Kconfig.debug | 36 ++++++++++++++++++------------------ > 1 file changed, 18 insertions(+), 18 deletions(-) > > Index: linux-2.6/lib/Kconfig.debug > ==================================================================> --- linux-2.6.orig/lib/Kconfig.debug 2018-04-16 21:08:36.000000000 +0200 > +++ linux-2.6/lib/Kconfig.debug 2018-04-25 15:56:16.000000000 +0200 > @@ -1503,6 +1503,10 @@ config NETDEV_NOTIFIER_ERROR_INJECT > > If unsure, say N. > > +config FUNCTION_ERROR_INJECTION > + def_bool y > + depends on HAVE_FUNCTION_ERROR_INJECTION && KPROBES > + > config FAULT_INJECTION > bool "Fault-injection framework" > depends on DEBUG_KERNEL > @@ -1510,10 +1514,6 @@ config FAULT_INJECTION > Provide fault-injection framework. > For more details, see Documentation/fault-injection/. > > -config FUNCTION_ERROR_INJECTION > - def_bool y > - depends on HAVE_FUNCTION_ERROR_INJECTION && KPROBES > - > config FAILSLAB > bool "Fault-injection capability for kmalloc" > depends on FAULT_INJECTION > @@ -1544,16 +1544,6 @@ config FAIL_IO_TIMEOUT > Only works with drivers that use the generic timeout handling, > for others it wont do anything. > > -config FAIL_MMC_REQUEST > - bool "Fault-injection capability for MMC IO" > - depends on FAULT_INJECTION_DEBUG_FS && MMC > - help > - Provide fault-injection capability for MMC IO. > - This will make the mmc core return data errors. This is > - useful to test the error handling in the mmc block device > - and to test how the mmc host driver handles retries from > - the block device. > - > config FAIL_FUTEX > bool "Fault-injection capability for futexes" > select DEBUG_FS > @@ -1561,6 +1551,12 @@ config FAIL_FUTEX > help > Provide fault-injection capability for futexes. > > +config FAULT_INJECTION_DEBUG_FS > + bool "Debugfs entries for fault-injection capabilities" > + depends on FAULT_INJECTION && SYSFS && DEBUG_FS > + help > + Enable configuration of fault-injection capabilities via debugfs. > + > config FAIL_FUNCTION > bool "Fault-injection capability for functions" > depends on FAULT_INJECTION_DEBUG_FS && FUNCTION_ERROR_INJECTION > @@ -1571,11 +1567,15 @@ config FAIL_FUNCTION > an error value and have to handle it. This is useful to test the > error handling in various subsystems. > > -config FAULT_INJECTION_DEBUG_FS > - bool "Debugfs entries for fault-injection capabilities" > - depends on FAULT_INJECTION && SYSFS && DEBUG_FS > +config FAIL_MMC_REQUEST > + bool "Fault-injection capability for MMC IO" > + depends on FAULT_INJECTION_DEBUG_FS && MMC > help > - Enable configuration of fault-injection capabilities via debugfs. > + Provide fault-injection capability for MMC IO. > + This will make the mmc core return data errors. This is > + useful to test the error handling in the mmc block device > + and to test how the mmc host driver handles retries from > + the block device. > > config FAULT_INJECTION_STACKTRACE_FILTER > bool "stacktrace filter for fault-injection capabilities" >-- ~Randy
Reasonably Related Threads
- [PATCH v4] fault-injection: introduce kvmalloc fallback options
- [PATCH v3] kvmalloc: always use vmalloc if CONFIG_DEBUG_SG
- [PATCH v5] fault-injection: introduce kvmalloc fallback options
- [PATCH v5] fault-injection: introduce kvmalloc fallback options
- [PATCH v4] fault-injection: introduce kvmalloc fallback options