Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] lib: stack-allocated percpu_counter in bench triggers debugobjects WARN_ON - [Low] lib: missing kernel-parameters.txt entry for percpu_counter_tree_bench.batch --
commit a1a6611650e4e4f4d5279cc3d46fb8ab12ea614c Author: Mathieu Desnoyers <[email protected]> lib: add a benchmark of hierarchical per-cpu counters This commit adds a KUnit benchmark comparing hierarchical per-CPU counters with struct percpu_counter across update, lifecycle, and churn workloads. It introduces CONFIG_PERCPU_COUNTER_TREE_BENCH and the batch module parameter to configure the hierarchical counter batch size. > diff --git a/lib/tests/percpu_counter_tree_bench.c > b/lib/tests/percpu_counter_tree_bench.c > new file mode 100644 > index 0000000000000..bf260bef196a5 > --- /dev/null > +++ b/lib/tests/percpu_counter_tree_bench.c [ ... ] > +/* Like the RSS counters by default. */ > +static unsigned int batch = 32; > +module_param(batch, uint, 0444); > +MODULE_PARM_DESC(batch, "Batch size of the hierarchical counters (power of > 2, 2 to 128)"); [Severity: Low] Should percpu_counter_tree_bench.batch= also be documented in Documentation/admin-guide/kernel-parameters.txt? Both the commit message and the CONFIG_PERCPU_COUNTER_TREE_BENCH help text in lib/Kconfig advertise passing percpu_counter_tree_bench.batch= on the kernel command line alongside percpu_counter_tree.arity=, which is documented there. [ ... ] > +static int bench_lifecycle_fn(void *data) > +{ > + struct bench_thread *t = data; > + struct percpu_counter_tree c[LIFECYCLE_NR]; > + struct percpu_counter pc[LIFECYCLE_NR]; > + u64 t0; > + int i; > + > + if (bench_wait_go(t->sh)) > + return 0; > + t0 = ktime_get_ns(); > + for (i = 0; t->sh->impl == IMPL_PERCPU_COUNTER && i < LIFECYCLE_ITER; > i++) { > + if (percpu_counter_init_many(pc, 0, GFP_KERNEL, LIFECYCLE_NR)) > + break; > + percpu_counter_add(&pc[0], 1); > + percpu_counter_add(&pc[0], -1); > + percpu_counter_destroy_many(pc, LIFECYCLE_NR); > + cond_resched(); > + } [ ... ] > +static int bench_churn_fn(void *data) > +{ > + struct bench_thread *t = data; > + unsigned int pin_period = t->sh->pin_period; > + struct percpu_counter_tree tmp[LIFECYCLE_NR], *c; > + struct percpu_counter tmp_pc[LIFECYCLE_NR], *pc; > + u64 t0; > + int i; > + > + if (bench_wait_go(t->sh)) > + return 0; > + t0 = ktime_get_ns(); > + for (i = 0; t->sh->impl == IMPL_PERCPU_COUNTER && i < CHURN_ITER; i++) { > + bool pin = pin_period && !(i % pin_period); > + > + pc = pin ? &t->pinned_pc[t->nr_pinned * LIFECYCLE_NR] : tmp_pc; > + if (percpu_counter_init_many(pc, 0, GFP_KERNEL, LIFECYCLE_NR)) > + break; [Severity: Medium] Will passing stack-allocated struct percpu_counter arrays (pc in bench_lifecycle_fn() and tmp_pc in bench_churn_fn()) to percpu_counter_init_many() trigger a debugobjects WARN_ON(1) when CONFIG_DEBUG_OBJECTS_PERCPU_COUNTER is enabled? The call chain is: percpu_counter_init_many() __percpu_counter_init_many() debug_percpu_counter_activate() debug_object_init() __debug_object_init(..., onstack = 0) lookup_object_or_alloc() debug_object_is_on_stack() Because debug_object_init() is called with onstack = 0 while pc and tmp_pc reside on the kthread stack, debug_object_is_on_stack() will warn that a non-onstack object is on the stack. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=10
