Re: [PATCH v2 6/7] mm, slab: add static key for should_failslab()
From: Vlastimil Babka <hidden>
Date: 2024-06-25 14:24:50
Also in:
bpf, linux-mm, lkml
On 6/20/24 12:49 AM, Vlastimil Babka wrote:
quoted hunk ↗ jump to hunk
--- a/mm/slub.c +++ b/mm/slub.c@@ -3874,13 +3874,37 @@ static __always_inline void maybe_wipe_obj_freeptr(struct kmem_cache *s, 0, sizeof(void *)); } -noinline int should_failslab(struct kmem_cache *s, gfp_t gfpflags) +#if defined(CONFIG_FUNCTION_ERROR_INJECTION) || defined(CONFIG_FAILSLAB) +DEFINE_STATIC_KEY_FALSE(should_failslab_active); + +#ifdef CONFIG_FUNCTION_ERROR_INJECTION +noinline +#else +static inline +#endif +int should_failslab(struct kmem_cache *s, gfp_t gfpflags)
Note that it has been found that (regardless of this series) gcc may clone this to a should_failslab.constprop.0 in case the function is empty because __should_failslab is compiled out (CONFIG_FAILSLAB=n). The "noinline" doesn't help - the original function stays but only the clone is actually being called, thus overriding the original function achieves nothing, see: https://github.com/bpftrace/bpftrace/issues/3258 So we could use __noclone to prevent that, and I was thinking by adding something this to error-injection.h: #ifdef CONFIG_FUNCTION_ERROR_INJECTION #define __error_injectable(alternative) noinline __noclone #else #define __error_injectable(alternative) alternative #endif and the usage here would be: __error_injectable(static inline) int should_failslab(...) Does that look acceptable, or is it too confusing that "static inline" is specified there as the storage class to use when error injection is actually disabled?
quoted hunk ↗ jump to hunk
{ if (__should_failslab(s, gfpflags)) return -ENOMEM; return 0; } -ALLOW_ERROR_INJECTION(should_failslab, ERRNO); +ALLOW_ERROR_INJECTION_KEY(should_failslab, ERRNO, &should_failslab_active); + +static __always_inline int should_failslab_wrapped(struct kmem_cache *s, + gfp_t gfp) +{ + if (static_branch_unlikely(&should_failslab_active)) + return should_failslab(s, gfp); + else + return 0; +} +#else +static __always_inline int should_failslab_wrapped(struct kmem_cache *s, + gfp_t gfp) +{ + return false; +} +#endif static __fastpath_inline struct kmem_cache *slab_pre_alloc_hook(struct kmem_cache *s, gfp_t flags)@@ -3889,7 +3913,7 @@ struct kmem_cache *slab_pre_alloc_hook(struct kmem_cache *s, gfp_t flags) might_alloc(flags); - if (unlikely(should_failslab(s, flags))) + if (should_failslab_wrapped(s, flags)) return NULL; return s;