Harry wrote:
> Since kmalloc_nolock() always fails in NMI and hardirq contexts on
> PREEMPT_RT, slub_kunit cannot properly test _nolock() APIs.
>
> Register a kprobe pre-handler to invoke kmalloc_nolock() and
> kfree_nolock() in the middle of the slab allocator. However, do not
> register the handler on UP kernels because that use case is not
> well supported [1] in the kernel.
>
> To attach the pre-handler while s->cpu_sheaves->lock or n->list_lock
> is held, add a wrapper function for lockdep_assert_held() that calls
> a no-op function slab_attach_kprobe_locked() on debug builds. The
> function is optimized away when neither CONFIG_PROVE_LOCKING nor
> CONFIG_DEBUG_VM is selected and register_kprobe() fails.
>
> The function calls barrier() to prevent the compiler from optimizing
> away its callsites. Otherwise, the compiler may consider the function
> does not have any side effect and remove callsites.
>
> Compared to using plain kprobe, this has two advantages: 1) it avoids
> hardcoding function names in the test, and 2) it can trigger those APIs
> in the middle of a function, where the lock is expected to be held as
> annotated with lockdep.
>
> While it was proposed [2] to use kunit function redirection to test
> this, it is currently infeasible as some lock helpers don't have
> symbols.
>
> Factor out the nested loop that calls kmalloc and friends to
> test_kmalloc_kfree(), and call them in
> test_kmalloc_kfree_nolock_{perf,kprobe}(), each being an independent
> test case. During the refactoring, drop alloc_fail handling as it
> doesn't provide much benefits.
Nice test addition!
Reviewed-by: Shengming Hu <[email protected]>
Nit below:
> Link: https://lore.kernel.org/linux-mm/[email protected] [1]
> Link: https://lore.kernel.org/linux-mm/[email protected] [2]
> Acked-by: Vlastimil Babka (SUSE) <[email protected]>
> Signed-off-by: Harry Yoo (Oracle) <[email protected]>
> ---
> lib/tests/slub_kunit.c | 167 +++++++++++++++++++++++++++++++++++--------------
> mm/slub.c | 36 ++++++++---
> 2 files changed, 148 insertions(+), 55 deletions(-)
>
> diff --git a/lib/tests/slub_kunit.c b/lib/tests/slub_kunit.c
> index fa6d31dbca16..a391467c1aa6 100644
> --- a/lib/tests/slub_kunit.c
> +++ b/lib/tests/slub_kunit.c
> @@ -8,6 +8,7 @@
> #include <linux/rcupdate.h>
> #include <linux/delay.h>
> #include <linux/perf_event.h>
> +#include <linux/kprobes.h>
> #include "../mm/slab.h"
>
> static struct kunit_resource resource;
> @@ -292,7 +293,7 @@ static void test_krealloc_redzone_zeroing(struct kunit *test)
> kmem_cache_destroy(s);
> }
>
> -#ifdef CONFIG_PERF_EVENTS
> +#if defined(CONFIG_PERF_EVENTS) || (defined(CONFIG_KPROBES) && defined(CONFIG_SMP))
> #define NR_ITERATIONS 1000
> #define NR_OBJECTS 1000
> static void *objects[NR_OBJECTS];
> @@ -302,26 +303,40 @@ struct test_nolock_context {
> int callback_count;
> int alloc_ok;
> int alloc_fail;
> +#ifdef CONFIG_PERF_EVENTS
> struct perf_event *event;
> +#endif
> +#if defined(CONFIG_KPROBES) && defined(CONFIG_SMP)
> + struct kprobe kprobe;
> +#endif
> };
>
> -static struct perf_event_attr hw_attr = {
> - .type = PERF_TYPE_HARDWARE,
> - .config = PERF_COUNT_HW_CPU_CYCLES,
> - .size = sizeof(struct perf_event_attr),
> - .pinned = 1,
> - .disabled = 1,
> - .freq = 1,
> - .sample_freq = 100000,
> -};
> +static void test_kmalloc_kfree(void)
> +{
> + int i, j;
> +
> + for (i = 0; i < NR_ITERATIONS; i++) {
> + for (j = 0; j < NR_OBJECTS; j++) {
> + gfp_t gfp = (i % 2) ? GFP_KERNEL : GFP_KERNEL_ACCOUNT;
>
> -static void overflow_handler_test_kmalloc_kfree_nolock(struct perf_event *event,
> - struct perf_sample_data *data,
> - struct pt_regs *regs)
> + objects[j] = kmalloc_obj(*objects[j], gfp);
> + if (!objects[j]) {
> + j--;
> + while (j >= 0)
> + kfree(objects[j--]);
> + return;
> + }
> + }
> +
> + for (j = 0; j < NR_OBJECTS; j++)
> + kfree(objects[j]);
> + }
> +}
> +
> +static void test_nolock(struct test_nolock_context *ctx)
> {
> void *objp;
> gfp_t gfp;
> - struct test_nolock_context *ctx = event->overflow_handler_context;
>
> /* __GFP_ACCOUNT to test kmalloc_nolock() in alloc_slab_obj_exts() */
> gfp = (ctx->callback_count % 2) ? 0 : __GFP_ACCOUNT;
> @@ -335,47 +350,104 @@ static void overflow_handler_test_kmalloc_kfree_nolock(struct perf_event *event,
> kfree_nolock(objp);
> ctx->callback_count++;
> }
> +#endif
>
> -static void test_kmalloc_kfree_nolock(struct kunit *test)
> +#ifdef CONFIG_PERF_EVENTS
> +static struct perf_event_attr hw_attr = {
> + .type = PERF_TYPE_HARDWARE,
> + .config = PERF_COUNT_HW_CPU_CYCLES,
> + .size = sizeof(struct perf_event_attr),
> + .pinned = 1,
> + .disabled = 1,
> + .freq = 1,
> + .sample_freq = 100000,
> +};
> +
> +static void overflow_handler_test_nolock(struct perf_event *event,
> + struct perf_sample_data *data,
> + struct pt_regs *regs)
> +{
> + struct test_nolock_context *ctx = event->overflow_handler_context;
> +
> + test_nolock(ctx);
> +}
> +
> +static bool enable_perf_events(struct test_nolock_context *ctx)
> {
> - int i, j;
> - struct test_nolock_context ctx = { .test = test };
> struct perf_event *event;
> - bool alloc_fail = false;
>
> event = perf_event_create_kernel_counter(&hw_attr, -1, current,
> - overflow_handler_test_kmalloc_kfree_nolock,
> - &ctx);
> + overflow_handler_test_nolock,
> + ctx);
> +
> if (IS_ERR(event))
> - kunit_skip(test, "Failed to create perf event");
> - ctx.event = event;
> - perf_event_enable(ctx.event);
> - for (i = 0; i < NR_ITERATIONS; i++) {
> - for (j = 0; j < NR_OBJECTS; j++) {
> - gfp_t gfp = (i % 2) ? GFP_KERNEL : GFP_KERNEL_ACCOUNT;
> + return false;
>
> - objects[j] = kmalloc(64, gfp);
> - if (!objects[j]) {
> - j--;
> - while (j >= 0)
> - kfree(objects[j--]);
> - alloc_fail = true;
> - goto cleanup;
> - }
> - }
> - for (j = 0; j < NR_OBJECTS; j++)
> - kfree(objects[j]);
> - }
> + ctx->event = event;
> + perf_event_enable(ctx->event);
> + return true;
> +}
>
> -cleanup:
> - perf_event_disable(ctx.event);
> - perf_event_release_kernel(ctx.event);
> +static void disable_perf_events(struct test_nolock_context *ctx)
> +{
> + kunit_info(ctx->test, "HW perf events: callback_count: %d, alloc_ok: %d, alloc_fail: %d\n",
> + ctx->callback_count, ctx->alloc_ok, ctx->alloc_fail);
> +
> + perf_event_disable(ctx->event);
> + perf_event_release_kernel(ctx->event);
> +}
> +
> +static void test_kmalloc_kfree_nolock_perf(struct kunit *test)
> +{
> + struct test_nolock_context ctx = { .test = test };
>
> - kunit_info(test, "callback_count: %d, alloc_ok: %d, alloc_fail: %d\n",
> - ctx.callback_count, ctx.alloc_ok, ctx.alloc_fail);
> + if (!enable_perf_events(&ctx))
> + kunit_skip(test, "Failed to enable perf event, skipping");
>
> - if (alloc_fail)
> - kunit_skip(test, "Allocation failed");
> + test_kmalloc_kfree();
> +
> + disable_perf_events(&ctx);
> + KUNIT_EXPECT_EQ(test, 0, slab_errors);
> +}
> +#endif
> +
Maybe it would be worth checking that the perf event actually fired:
KUNIT_EXPECT_GT(test, ctx.callback_count, 0);
Otherwise, the test could pass without the callback.
> +#if defined(CONFIG_KPROBES) && defined(CONFIG_SMP)
> +static int slab_kprobe_pre_handler(struct kprobe *p, struct pt_regs *regs)
> +{
> + struct test_nolock_context *ctx;
> +
> + ctx = container_of(p, struct test_nolock_context, kprobe);
> + test_nolock(ctx);
> + return 0;
> +}
> +
> +static bool register_slab_kprobes(struct test_nolock_context *ctx)
> +{
> + ctx->kprobe.symbol_name = "slab_attach_kprobe_locked";
> + ctx->kprobe.pre_handler = slab_kprobe_pre_handler;
> +
> + if (register_kprobe(&ctx->kprobe))
> + return false;
> + return true;
> +}
> +
> +static void unregister_slab_kprobes(struct test_nolock_context *ctx)
> +{
> + kunit_info(ctx->test, "kprobes: callback_count: %d, alloc_ok: %d, alloc_fail: %d\n",
> + ctx->callback_count, ctx->alloc_ok, ctx->alloc_fail);
> + unregister_kprobe(&ctx->kprobe);
> +}
> +
> +static void test_kmalloc_kfree_nolock_kprobe(struct kunit *test)
> +{
> + struct test_nolock_context ctx = { .test = test };
> +
> + if (!register_slab_kprobes(&ctx))
> + kunit_skip(test, "Failed to register kprobe, skipping");
> +
> + test_kmalloc_kfree();
> +
> + unregister_slab_kprobes(&ctx);
> KUNIT_EXPECT_EQ(test, 0, slab_errors);
> }
Same here. Maybe it would be useful to check that the kprobe handler was
invoked at least once:
KUNIT_EXPECT_GT(test, ctx.callback_count, 0);
--
With Best Regards,
Shengming
> #endif
> @@ -405,7 +477,10 @@ static struct kunit_case test_cases[] = {
> KUNIT_CASE(test_leak_destroy),
> KUNIT_CASE(test_krealloc_redzone_zeroing),
> #ifdef CONFIG_PERF_EVENTS
> - KUNIT_CASE_SLOW(test_kmalloc_kfree_nolock),
> + KUNIT_CASE_SLOW(test_kmalloc_kfree_nolock_perf),
> +#endif
> +#if defined(CONFIG_KPROBES) && defined(CONFIG_SMP)
> + KUNIT_CASE_SLOW(test_kmalloc_kfree_nolock_kprobe),
> #endif
> {}
> };
> diff --git a/mm/slub.c b/mm/slub.c
> index da9efb040040..e32a68677537 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -908,6 +908,24 @@ static inline unsigned int obj_exts_offset_in_object(struct kmem_cache *s)
> }
> #endif
>
> +/*
> + * A no-op function used to attach kprobe handlers in slub_kunit tests.
> + * The barrier is needed to prevent the compiler from optimizing out callsites.
> + */
> +#if defined(CONFIG_DEBUG_VM) || defined(CONFIG_PROVE_LOCKING)
> +static noinline void slab_attach_kprobe_locked(void)
> +{
> + barrier();
> +}
> +#else
> +static inline void slab_attach_kprobe_locked(void) { }
> +#endif
> +
> +#define slab_lockdep_assert_held(lock) do { \
> + lockdep_assert_held(lock); \
> + slab_attach_kprobe_locked(); \
> +} while (0)
> +
> #ifdef CONFIG_SLUB_DEBUG
>
> /*
> @@ -1665,7 +1683,7 @@ static void add_full(struct kmem_cache *s,
> if (!(s->flags & SLAB_STORE_USER))
> return;
>
> - lockdep_assert_held(&n->list_lock);
> + slab_lockdep_assert_held(&n->list_lock);
> list_add(&slab->slab_list, &n->full);
> }
>
> @@ -1674,7 +1692,7 @@ static void remove_full(struct kmem_cache *s, struct kmem_cache_node *n, struct
> if (!(s->flags & SLAB_STORE_USER))
> return;
>
> - lockdep_assert_held(&n->list_lock);
> + slab_lockdep_assert_held(&n->list_lock);
> list_del(&slab->slab_list);
> }
>
> @@ -2868,7 +2886,7 @@ static unsigned int __sheaf_flush_main_batch(struct kmem_cache *s)
> void *objects[PCS_BATCH_MAX];
> struct slab_sheaf *sheaf;
>
> - lockdep_assert_held(this_cpu_ptr(&s->cpu_sheaves->lock));
> + slab_lockdep_assert_held(this_cpu_ptr(&s->cpu_sheaves->lock));
>
> pcs = this_cpu_ptr(s->cpu_sheaves);
> sheaf = pcs->main;
> @@ -3547,7 +3565,7 @@ __add_partial(struct kmem_cache_node *n, struct slab *slab, enum add_mode mode)
> static inline void add_partial(struct kmem_cache_node *n,
> struct slab *slab, enum add_mode mode)
> {
> - lockdep_assert_held(&n->list_lock);
> + slab_lockdep_assert_held(&n->list_lock);
> __add_partial(n, slab, mode);
> }
>
> @@ -3561,7 +3579,7 @@ static inline void clear_node_partial_state(struct kmem_cache_node *n,
> static inline void remove_partial(struct kmem_cache_node *n,
> struct slab *slab)
> {
> - lockdep_assert_held(&n->list_lock);
> + slab_lockdep_assert_held(&n->list_lock);
> list_del(&slab->slab_list);
> clear_node_partial_state(n, slab);
> }
> @@ -3577,7 +3595,7 @@ static void *alloc_single_from_partial(struct kmem_cache *s,
> {
> void *object;
>
> - lockdep_assert_held(&n->list_lock);
> + slab_lockdep_assert_held(&n->list_lock);
>
> #ifdef CONFIG_SLUB_DEBUG
> if (s->flags & SLAB_CONSISTENCY_CHECKS) {
> @@ -4642,7 +4660,7 @@ __pcs_replace_empty_main(struct kmem_cache *s, struct slub_percpu_sheaves *pcs,
> struct node_barn *barn;
> bool allow_spin;
>
> - lockdep_assert_held(this_cpu_ptr(&s->cpu_sheaves->lock));
> + slab_lockdep_assert_held(this_cpu_ptr(&s->cpu_sheaves->lock));
>
> /* Bootstrap or debug cache, back off */
> if (unlikely(!cache_has_sheaves(s))) {
> @@ -5765,7 +5783,7 @@ static void __pcs_install_empty_sheaf(struct kmem_cache *s,
> struct slub_percpu_sheaves *pcs, struct slab_sheaf *empty,
> struct node_barn *barn)
> {
> - lockdep_assert_held(this_cpu_ptr(&s->cpu_sheaves->lock));
> + slab_lockdep_assert_held(this_cpu_ptr(&s->cpu_sheaves->lock));
>
> /* This is what we expect to find if nobody interrupted us. */
> if (likely(!pcs->spare)) {
> @@ -5816,7 +5834,7 @@ __pcs_replace_full_main(struct kmem_cache *s, struct slub_percpu_sheaves *pcs,
> bool put_fail;
>
> restart:
> - lockdep_assert_held(this_cpu_ptr(&s->cpu_sheaves->lock));
> + slab_lockdep_assert_held(this_cpu_ptr(&s->cpu_sheaves->lock));
>
> /* Bootstrap or debug cache, back off */
> if (unlikely(!cache_has_sheaves(s))) {
>
> --
> 2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.