a7526fe8b9
Patch series "revert unconditional slab and page allocator fault injection calls". These two patches largely revert commits that added function call overhead into slab and page allocation hotpaths and that cannot be currently disabled even though related CONFIG_ options do exist. A much more involved solution that can keep the callsites always existing but hidden behind a static key if unused, is possible [1] and can be pursued by anyone who believes it's necessary. Meanwhile the fact the should_failslab() error injection is already not functional on kernels built with current gcc without anyone noticing [2], and lukewarm response to [1] suggests the need is not there. I believe it will be more fair to have the state after this series as a baseline for possible further optimisation, instead of the unconditional overhead. For example a possible compromise for anyone who's fine with an empty function call overhead but not the full CONFIG_FAILSLAB / CONFIG_FAIL_PAGE_ALLOC overhead is to reuse patch 1 from [1] but insert a static key check only inside should_failslab() and should_fail_alloc_page() before performing the more expensive checks. [1] https://lore.kernel.org/all/20240620-fault-injection-statickeys-v2-0-e23947d3d84b@suse.cz/#t [2] https://github.com/bpftrace/bpftrace/issues/3258 This patch (of 2): This mostly reverts commit4f6923fbb3("mm: make should_failslab always available for fault injection"). The commit made should_failslab() a noinline function that's always called from the slab allocation hotpath, even if it's empty because CONFIG_SHOULD_FAILSLAB is not enabled, and there is no option to disable that call. This is visible in profiles and the function call overhead can be noticeable especially with cpu mitigations. Meanwhile the bpftrace program example in the commit silently does not work without CONFIG_SHOULD_FAILSLAB anyway with a recent gcc, because the empty function gets a .constprop clone that is actually being called (uselessly) from the slab hotpath, while the error injection is hooked to the original function that's not being called at all [1]. Thus put the whole should_failslab() function back behind CONFIG_SHOULD_FAILSLAB. It's not a complete revert of4f6923fbb3- the int return type that returns -ENOMEM on failure is preserved, as well ALLOW_ERROR_INJECTION annotation. The BTF_ID() record that was meanwhile added is also guarded by CONFIG_SHOULD_FAILSLAB. [1] https://github.com/bpftrace/bpftrace/issues/3258 Link: https://lkml.kernel.org/r/20240711-b4-fault-injection-reverts-v1-0-9e2651945d68@suse.cz Link: https://lkml.kernel.org/r/20240711-b4-fault-injection-reverts-v1-1-9e2651945d68@suse.cz Signed-off-by: Vlastimil Babka <vbabka@suse.cz> Cc: Akinobu Mita <akinobu.mita@gmail.com> Cc: Alexei Starovoitov <ast@kernel.org> Cc: Andrii Nakryiko <andrii@kernel.org> Cc: Christoph Lameter <cl@linux.com> Cc: Daniel Borkmann <daniel@iogearbox.net> Cc: David Rientjes <rientjes@google.com> Cc: Eduard Zingerman <eddyz87@gmail.com> Cc: Hao Luo <haoluo@google.com> Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: John Fastabend <john.fastabend@gmail.com> Cc: KP Singh <kpsingh@kernel.org> Cc: Martin KaFai Lau <martin.lau@linux.dev> Cc: Mateusz Guzik <mjguzik@gmail.com> Cc: Roman Gushchin <roman.gushchin@linux.dev> Cc: Song Liu <song@kernel.org> Cc: Stanislav Fomichev <sdf@fomichev.me> Cc: Yonghong Song <yonghong.song@linux.dev> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
76 lines
1.7 KiB
C
76 lines
1.7 KiB
C
// SPDX-License-Identifier: GPL-2.0
|
|
#include <linux/fault-inject.h>
|
|
#include <linux/error-injection.h>
|
|
#include <linux/slab.h>
|
|
#include <linux/mm.h>
|
|
#include "slab.h"
|
|
|
|
static struct {
|
|
struct fault_attr attr;
|
|
bool ignore_gfp_reclaim;
|
|
bool cache_filter;
|
|
} failslab = {
|
|
.attr = FAULT_ATTR_INITIALIZER,
|
|
.ignore_gfp_reclaim = true,
|
|
.cache_filter = false,
|
|
};
|
|
|
|
int should_failslab(struct kmem_cache *s, gfp_t gfpflags)
|
|
{
|
|
int flags = 0;
|
|
|
|
/* No fault-injection for bootstrap cache */
|
|
if (unlikely(s == kmem_cache))
|
|
return 0;
|
|
|
|
if (gfpflags & __GFP_NOFAIL)
|
|
return 0;
|
|
|
|
if (failslab.ignore_gfp_reclaim &&
|
|
(gfpflags & __GFP_DIRECT_RECLAIM))
|
|
return 0;
|
|
|
|
if (failslab.cache_filter && !(s->flags & SLAB_FAILSLAB))
|
|
return 0;
|
|
|
|
/*
|
|
* In some cases, it expects to specify __GFP_NOWARN
|
|
* to avoid printing any information(not just a warning),
|
|
* thus avoiding deadlocks. See commit 6b9dbedbe349 for
|
|
* details.
|
|
*/
|
|
if (gfpflags & __GFP_NOWARN)
|
|
flags |= FAULT_NOWARN;
|
|
|
|
return should_fail_ex(&failslab.attr, s->object_size, flags) ? -ENOMEM : 0;
|
|
}
|
|
ALLOW_ERROR_INJECTION(should_failslab, ERRNO);
|
|
|
|
static int __init setup_failslab(char *str)
|
|
{
|
|
return setup_fault_attr(&failslab.attr, str);
|
|
}
|
|
__setup("failslab=", setup_failslab);
|
|
|
|
#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
|
|
static int __init failslab_debugfs_init(void)
|
|
{
|
|
struct dentry *dir;
|
|
umode_t mode = S_IFREG | 0600;
|
|
|
|
dir = fault_create_debugfs_attr("failslab", NULL, &failslab.attr);
|
|
if (IS_ERR(dir))
|
|
return PTR_ERR(dir);
|
|
|
|
debugfs_create_bool("ignore-gfp-wait", mode, dir,
|
|
&failslab.ignore_gfp_reclaim);
|
|
debugfs_create_bool("cache-filter", mode, dir,
|
|
&failslab.cache_filter);
|
|
|
|
return 0;
|
|
}
|
|
|
|
late_initcall(failslab_debugfs_init);
|
|
|
|
#endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */
|