On 10/5/21 17:31, Jens Axboe wrote:
> Allocations can be a very hot path, and this out-of-line function
> call is noticeable.
>
> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
It used to be inline b4 (hi, Konstantin!) and then was converted to be like
this intentionally :/
See 4f6923fbb352 ("mm: make should_failslab always available for fault
injection")
And now also kernel/bpf/verifier.c contains:
BTF_ID(func, should_failslab)
I think either your or Andrew's version will break this BTF_ID thing, at the
very least.
But I do strongly agree that putting unconditionally a non-inline call into
slab allocator fastpath sucks. Can we make it so that bpf can only do these
overrides when CONFIG_FAILSLAB is enabled?
I don't know, perhaps putting this BTF_ID() in #ifdef as well, or providing
a dummy that is always available (so that nothing breaks), but doesn't
actually affect slab_pre_alloc_hook() unless CONFIG_FAILSLAB has been enabled?
> ---
>
> diff --git a/include/linux/fault-inject.h b/include/linux/fault-inject.h
> index e525f6957c49..3128d2c8b3b4 100644
> --- a/include/linux/fault-inject.h
> +++ b/include/linux/fault-inject.h
> @@ -64,8 +64,8 @@ static inline struct dentry *fault_create_debugfs_attr(const char *name,
>
> struct kmem_cache;
>
> -int should_failslab(struct kmem_cache *s, gfp_t gfpflags);
> #ifdef CONFIG_FAILSLAB
> +int should_failslab(struct kmem_cache *s, gfp_t gfpflags);
> extern bool __should_failslab(struct kmem_cache *s, gfp_t gfpflags);
> #else
> static inline bool __should_failslab(struct kmem_cache *s, gfp_t gfpflags)
> diff --git a/mm/slab.h b/mm/slab.h
> index 58c01a34e5b8..92fd6fe01877 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -491,8 +491,10 @@ static inline struct kmem_cache *slab_pre_alloc_hook(struct kmem_cache *s,
>
> might_alloc(flags);
>
> +#ifdef CONFIG_FAILSLAB
> if (should_failslab(s, flags))
> return NULL;
> +#endif
>
> if (!memcg_slab_pre_alloc_hook(s, objcgp, size, flags))
> return NULL;
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index ec2bb0beed75..c21bd447f237 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1323,6 +1323,7 @@ EXPORT_TRACEPOINT_SYMBOL(kmem_cache_alloc_node);
> EXPORT_TRACEPOINT_SYMBOL(kfree);
> EXPORT_TRACEPOINT_SYMBOL(kmem_cache_free);
>
> +#ifdef CONFIG_FAILSLAB
> int should_failslab(struct kmem_cache *s, gfp_t gfpflags)
> {
> if (__should_failslab(s, gfpflags))
> @@ -1330,3 +1331,4 @@ int should_failslab(struct kmem_cache *s, gfp_t gfpflags)
> return 0;
> }
> ALLOW_ERROR_INJECTION(should_failslab, ERRNO);
> +#endif
>
