From: Dmitry Vyukov <dvyukov@google.com>
To: Marco Elver <elver@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Frederic Weisbecker <frederic@kernel.org>,
Ingo Molnar <mingo@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
Michael Ellerman <mpe@ellerman.id.au>,
linuxppc-dev@lists.ozlabs.org, linux-perf-users@vger.kernel.org,
x86@kernel.org, linux-sh@vger.kernel.org,
kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 01/14] perf/hw_breakpoint: Add KUnit test for constraints accounting
Date: Mon, 4 Jul 2022 17:10:09 +0200 [thread overview]
Message-ID: <CACT4Y+aA7QkAsufv6EMQ1O8mZaVd-eNOqRrx2a7qvPR4Tt=izA@mail.gmail.com> (raw)
In-Reply-To: <20220704150514.48816-2-elver@google.com>
On Mon, 4 Jul 2022 at 17:06, Marco Elver <elver@google.com> wrote:
>
> Add KUnit test for hw_breakpoint constraints accounting, with various
> interesting mixes of breakpoint targets (some care was taken to catch
> interesting corner cases via bug-injection).
>
> The test cannot be built as a module because it requires access to
> hw_breakpoint_slots(), which is not inlinable or exported on all
> architectures.
>
> Signed-off-by: Marco Elver <elver@google.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
> ---
> v3:
> * Don't use raw_smp_processor_id().
>
> v2:
> * New patch.
> ---
> kernel/events/Makefile | 1 +
> kernel/events/hw_breakpoint_test.c | 323 +++++++++++++++++++++++++++++
> lib/Kconfig.debug | 10 +
> 3 files changed, 334 insertions(+)
> create mode 100644 kernel/events/hw_breakpoint_test.c
>
> diff --git a/kernel/events/Makefile b/kernel/events/Makefile
> index 8591c180b52b..91a62f566743 100644
> --- a/kernel/events/Makefile
> +++ b/kernel/events/Makefile
> @@ -2,4 +2,5 @@
> obj-y := core.o ring_buffer.o callchain.o
>
> obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
> +obj-$(CONFIG_HW_BREAKPOINT_KUNIT_TEST) += hw_breakpoint_test.o
> obj-$(CONFIG_UPROBES) += uprobes.o
> diff --git a/kernel/events/hw_breakpoint_test.c b/kernel/events/hw_breakpoint_test.c
> new file mode 100644
> index 000000000000..433c5c45e2a5
> --- /dev/null
> +++ b/kernel/events/hw_breakpoint_test.c
> @@ -0,0 +1,323 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for hw_breakpoint constraints accounting logic.
> + *
> + * Copyright (C) 2022, Google LLC.
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/cpumask.h>
> +#include <linux/hw_breakpoint.h>
> +#include <linux/kthread.h>
> +#include <linux/perf_event.h>
> +#include <asm/hw_breakpoint.h>
> +
> +#define TEST_REQUIRES_BP_SLOTS(test, slots) \
> + do { \
> + if ((slots) > get_test_bp_slots()) { \
> + kunit_skip((test), "Requires breakpoint slots: %d > %d", slots, \
> + get_test_bp_slots()); \
> + } \
> + } while (0)
> +
> +#define TEST_EXPECT_NOSPC(expr) KUNIT_EXPECT_EQ(test, -ENOSPC, PTR_ERR(expr))
> +
> +#define MAX_TEST_BREAKPOINTS 512
> +
> +static char break_vars[MAX_TEST_BREAKPOINTS];
> +static struct perf_event *test_bps[MAX_TEST_BREAKPOINTS];
> +static struct task_struct *__other_task;
> +
> +static struct perf_event *register_test_bp(int cpu, struct task_struct *tsk, int idx)
> +{
> + struct perf_event_attr attr = {};
> +
> + if (WARN_ON(idx < 0 || idx >= MAX_TEST_BREAKPOINTS))
> + return NULL;
> +
> + hw_breakpoint_init(&attr);
> + attr.bp_addr = (unsigned long)&break_vars[idx];
> + attr.bp_len = HW_BREAKPOINT_LEN_1;
> + attr.bp_type = HW_BREAKPOINT_RW;
> + return perf_event_create_kernel_counter(&attr, cpu, tsk, NULL, NULL);
> +}
> +
> +static void unregister_test_bp(struct perf_event **bp)
> +{
> + if (WARN_ON(IS_ERR(*bp)))
> + return;
> + if (WARN_ON(!*bp))
> + return;
> + unregister_hw_breakpoint(*bp);
> + *bp = NULL;
> +}
> +
> +static int get_test_bp_slots(void)
> +{
> + static int slots;
> +
> + if (!slots)
> + slots = hw_breakpoint_slots(TYPE_DATA);
> +
> + return slots;
> +}
> +
> +static void fill_one_bp_slot(struct kunit *test, int *id, int cpu, struct task_struct *tsk)
> +{
> + struct perf_event *bp = register_test_bp(cpu, tsk, *id);
> +
> + KUNIT_ASSERT_NOT_NULL(test, bp);
> + KUNIT_ASSERT_FALSE(test, IS_ERR(bp));
> + KUNIT_ASSERT_NULL(test, test_bps[*id]);
> + test_bps[(*id)++] = bp;
> +}
> +
> +/*
> + * Fills up the given @cpu/@tsk with breakpoints, only leaving @skip slots free.
> + *
> + * Returns true if this can be called again, continuing at @id.
> + */
> +static bool fill_bp_slots(struct kunit *test, int *id, int cpu, struct task_struct *tsk, int skip)
> +{
> + for (int i = 0; i < get_test_bp_slots() - skip; ++i)
> + fill_one_bp_slot(test, id, cpu, tsk);
> +
> + return *id + get_test_bp_slots() <= MAX_TEST_BREAKPOINTS;
> +}
> +
> +static int dummy_kthread(void *arg)
> +{
> + return 0;
> +}
> +
> +static struct task_struct *get_other_task(struct kunit *test)
> +{
> + struct task_struct *tsk;
> +
> + if (__other_task)
> + return __other_task;
> +
> + tsk = kthread_create(dummy_kthread, NULL, "hw_breakpoint_dummy_task");
> + KUNIT_ASSERT_FALSE(test, IS_ERR(tsk));
> + __other_task = tsk;
> + return __other_task;
> +}
> +
> +static int get_test_cpu(int num)
> +{
> + int cpu;
> +
> + WARN_ON(num < 0);
> +
> + for_each_online_cpu(cpu) {
> + if (num-- <= 0)
> + break;
> + }
> +
> + return cpu;
> +}
> +
> +/* ===== Test cases ===== */
> +
> +static void test_one_cpu(struct kunit *test)
> +{
> + int idx = 0;
> +
> + fill_bp_slots(test, &idx, get_test_cpu(0), NULL, 0);
> + TEST_EXPECT_NOSPC(register_test_bp(-1, current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), NULL, idx));
> +}
> +
> +static void test_many_cpus(struct kunit *test)
> +{
> + int idx = 0;
> + int cpu;
> +
> + /* Test that CPUs are independent. */
> + for_each_online_cpu(cpu) {
> + bool do_continue = fill_bp_slots(test, &idx, cpu, NULL, 0);
> +
> + TEST_EXPECT_NOSPC(register_test_bp(cpu, NULL, idx));
> + if (!do_continue)
> + break;
> + }
> +}
> +
> +static void test_one_task_on_all_cpus(struct kunit *test)
> +{
> + int idx = 0;
> +
> + fill_bp_slots(test, &idx, -1, current, 0);
> + TEST_EXPECT_NOSPC(register_test_bp(-1, current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), NULL, idx));
> + /* Remove one and adding back CPU-target should work. */
> + unregister_test_bp(&test_bps[0]);
> + fill_one_bp_slot(test, &idx, get_test_cpu(0), NULL);
> +}
> +
> +static void test_two_tasks_on_all_cpus(struct kunit *test)
> +{
> + int idx = 0;
> +
> + /* Test that tasks are independent. */
> + fill_bp_slots(test, &idx, -1, current, 0);
> + fill_bp_slots(test, &idx, -1, get_other_task(test), 0);
> +
> + TEST_EXPECT_NOSPC(register_test_bp(-1, current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(-1, get_other_task(test), idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), get_other_task(test), idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), NULL, idx));
> + /* Remove one from first task and adding back CPU-target should not work. */
> + unregister_test_bp(&test_bps[0]);
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), NULL, idx));
> +}
> +
> +static void test_one_task_on_one_cpu(struct kunit *test)
> +{
> + int idx = 0;
> +
> + fill_bp_slots(test, &idx, get_test_cpu(0), current, 0);
> + TEST_EXPECT_NOSPC(register_test_bp(-1, current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), NULL, idx));
> + /*
> + * Remove one and adding back CPU-target should work; this case is
> + * special vs. above because the task's constraints are CPU-dependent.
> + */
> + unregister_test_bp(&test_bps[0]);
> + fill_one_bp_slot(test, &idx, get_test_cpu(0), NULL);
> +}
> +
> +static void test_one_task_mixed(struct kunit *test)
> +{
> + int idx = 0;
> +
> + TEST_REQUIRES_BP_SLOTS(test, 3);
> +
> + fill_one_bp_slot(test, &idx, get_test_cpu(0), current);
> + fill_bp_slots(test, &idx, -1, current, 1);
> + TEST_EXPECT_NOSPC(register_test_bp(-1, current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), NULL, idx));
> +
> + /* Transition from CPU-dependent pinned count to CPU-independent. */
> + unregister_test_bp(&test_bps[0]);
> + unregister_test_bp(&test_bps[1]);
> + fill_one_bp_slot(test, &idx, get_test_cpu(0), NULL);
> + fill_one_bp_slot(test, &idx, get_test_cpu(0), NULL);
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), NULL, idx));
> +}
> +
> +static void test_two_tasks_on_one_cpu(struct kunit *test)
> +{
> + int idx = 0;
> +
> + fill_bp_slots(test, &idx, get_test_cpu(0), current, 0);
> + fill_bp_slots(test, &idx, get_test_cpu(0), get_other_task(test), 0);
> +
> + TEST_EXPECT_NOSPC(register_test_bp(-1, current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(-1, get_other_task(test), idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), get_other_task(test), idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), NULL, idx));
> + /* Can still create breakpoints on some other CPU. */
> + fill_bp_slots(test, &idx, get_test_cpu(1), NULL, 0);
> +}
> +
> +static void test_two_tasks_on_one_all_cpus(struct kunit *test)
> +{
> + int idx = 0;
> +
> + fill_bp_slots(test, &idx, get_test_cpu(0), current, 0);
> + fill_bp_slots(test, &idx, -1, get_other_task(test), 0);
> +
> + TEST_EXPECT_NOSPC(register_test_bp(-1, current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(-1, get_other_task(test), idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), get_other_task(test), idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), NULL, idx));
> + /* Cannot create breakpoints on some other CPU either. */
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(1), NULL, idx));
> +}
> +
> +static void test_task_on_all_and_one_cpu(struct kunit *test)
> +{
> + int tsk_on_cpu_idx, cpu_idx;
> + int idx = 0;
> +
> + TEST_REQUIRES_BP_SLOTS(test, 3);
> +
> + fill_bp_slots(test, &idx, -1, current, 2);
> + /* Transitioning from only all CPU breakpoints to mixed. */
> + tsk_on_cpu_idx = idx;
> + fill_one_bp_slot(test, &idx, get_test_cpu(0), current);
> + fill_one_bp_slot(test, &idx, -1, current);
> +
> + TEST_EXPECT_NOSPC(register_test_bp(-1, current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), NULL, idx));
> +
> + /* We should still be able to use up another CPU's slots. */
> + cpu_idx = idx;
> + fill_one_bp_slot(test, &idx, get_test_cpu(1), NULL);
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(1), NULL, idx));
> +
> + /* Transitioning back to task target on all CPUs. */
> + unregister_test_bp(&test_bps[tsk_on_cpu_idx]);
> + /* Still have a CPU target breakpoint in get_test_cpu(1). */
> + TEST_EXPECT_NOSPC(register_test_bp(-1, current, idx));
> + /* Remove it and try again. */
> + unregister_test_bp(&test_bps[cpu_idx]);
> + fill_one_bp_slot(test, &idx, -1, current);
> +
> + TEST_EXPECT_NOSPC(register_test_bp(-1, current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), current, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(0), NULL, idx));
> + TEST_EXPECT_NOSPC(register_test_bp(get_test_cpu(1), NULL, idx));
> +}
> +
> +static struct kunit_case hw_breakpoint_test_cases[] = {
> + KUNIT_CASE(test_one_cpu),
> + KUNIT_CASE(test_many_cpus),
> + KUNIT_CASE(test_one_task_on_all_cpus),
> + KUNIT_CASE(test_two_tasks_on_all_cpus),
> + KUNIT_CASE(test_one_task_on_one_cpu),
> + KUNIT_CASE(test_one_task_mixed),
> + KUNIT_CASE(test_two_tasks_on_one_cpu),
> + KUNIT_CASE(test_two_tasks_on_one_all_cpus),
> + KUNIT_CASE(test_task_on_all_and_one_cpu),
> + {},
> +};
> +
> +static int test_init(struct kunit *test)
> +{
> + /* Most test cases want 2 distinct CPUs. */
> + return num_online_cpus() < 2 ? -EINVAL : 0;
> +}
> +
> +static void test_exit(struct kunit *test)
> +{
> + for (int i = 0; i < MAX_TEST_BREAKPOINTS; ++i) {
> + if (test_bps[i])
> + unregister_test_bp(&test_bps[i]);
> + }
> +
> + if (__other_task) {
> + kthread_stop(__other_task);
> + __other_task = NULL;
> + }
> +}
> +
> +static struct kunit_suite hw_breakpoint_test_suite = {
> + .name = "hw_breakpoint",
> + .test_cases = hw_breakpoint_test_cases,
> + .init = test_init,
> + .exit = test_exit,
> +};
> +
> +kunit_test_suites(&hw_breakpoint_test_suite);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Marco Elver <elver@google.com>");
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 2e24db4bff19..4c87a6edf046 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2513,6 +2513,16 @@ config STACKINIT_KUNIT_TEST
> CONFIG_GCC_PLUGIN_STRUCTLEAK, CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF,
> or CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL.
>
> +config HW_BREAKPOINT_KUNIT_TEST
> + bool "Test hw_breakpoint constraints accounting" if !KUNIT_ALL_TESTS
> + depends on HAVE_HW_BREAKPOINT
> + depends on KUNIT=y
> + default KUNIT_ALL_TESTS
> + help
> + Tests for hw_breakpoint constraints accounting.
> +
> + If unsure, say N.
> +
> config TEST_UDELAY
> tristate "udelay test driver"
> help
> --
> 2.37.0.rc0.161.g10f37bed90-goog
>
next prev parent reply other threads:[~2022-07-04 15:11 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-04 15:05 [PATCH v3 00/14] perf/hw_breakpoint: Optimize for thousands of tasks Marco Elver
2022-07-04 15:05 ` [PATCH v3 01/14] perf/hw_breakpoint: Add KUnit test for constraints accounting Marco Elver
2022-07-04 15:10 ` Dmitry Vyukov [this message]
2022-07-20 15:22 ` Ian Rogers
2022-07-21 16:22 ` Mark Rutland
2022-07-22 9:10 ` Will Deacon
2022-07-22 9:20 ` Dmitry Vyukov
2022-07-22 10:10 ` Will Deacon
2022-07-22 10:31 ` Dmitry Vyukov
2022-07-22 11:03 ` Will Deacon
2022-07-22 13:41 ` Dmitry Vyukov
2022-07-25 11:00 ` Marco Elver
2022-07-04 15:05 ` [PATCH v3 02/14] perf/hw_breakpoint: Provide hw_breakpoint_is_used() and use in test Marco Elver
2022-07-04 15:09 ` Dmitry Vyukov
2022-07-20 15:22 ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 03/14] perf/hw_breakpoint: Clean up headers Marco Elver
2022-07-20 15:23 ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 04/14] perf/hw_breakpoint: Optimize list of per-task breakpoints Marco Elver
2022-07-20 15:29 ` Ian Rogers
2022-07-20 15:39 ` Marco Elver
2022-07-04 15:05 ` [PATCH v3 05/14] perf/hw_breakpoint: Mark data __ro_after_init Marco Elver
2022-07-20 15:30 ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 06/14] perf/hw_breakpoint: Optimize constant number of breakpoint slots Marco Elver
2022-07-20 15:31 ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 07/14] perf/hw_breakpoint: Make hw_breakpoint_weight() inlinable Marco Elver
2022-07-20 15:32 ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 08/14] perf/hw_breakpoint: Remove useless code related to flexible breakpoints Marco Elver
2022-07-20 15:34 ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 09/14] powerpc/hw_breakpoint: Avoid relying on caller synchronization Marco Elver
2022-07-20 15:35 ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 10/14] locking/percpu-rwsem: Add percpu_is_write_locked() and percpu_is_read_locked() Marco Elver
2022-07-20 15:36 ` Ian Rogers
2022-08-17 12:47 ` Peter Zijlstra
2022-08-29 6:00 ` Marco Elver
2022-07-04 15:05 ` [PATCH v3 11/14] perf/hw_breakpoint: Reduce contention with large number of tasks Marco Elver
2022-07-20 15:38 ` Ian Rogers
2022-08-17 13:03 ` Peter Zijlstra
2022-08-17 13:14 ` Marco Elver
2022-08-29 8:38 ` Peter Zijlstra
2022-08-29 9:38 ` Marco Elver
2022-07-04 15:05 ` [PATCH v3 12/14] perf/hw_breakpoint: Introduce bp_slots_histogram Marco Elver
2022-07-20 15:40 ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 13/14] perf/hw_breakpoint: Optimize max_bp_pinned_slots() for CPU-independent task targets Marco Elver
2022-07-20 15:42 ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 14/14] perf/hw_breakpoint: Optimize toggle_bp_slot() " Marco Elver
2022-07-20 15:44 ` Ian Rogers
2022-07-12 13:39 ` [PATCH v3 00/14] perf/hw_breakpoint: Optimize for thousands of tasks Marco Elver
2022-07-20 15:47 ` Ian Rogers
2022-08-16 14:12 ` Marco Elver
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CACT4Y+aA7QkAsufv6EMQ1O8mZaVd-eNOqRrx2a7qvPR4Tt=izA@mail.gmail.com' \
--to=dvyukov@google.com \
--cc=acme@kernel.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=elver@google.com \
--cc=frederic@kernel.org \
--cc=jolsa@redhat.com \
--cc=kasan-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mark.rutland@arm.com \
--cc=mingo@kernel.org \
--cc=mpe@ellerman.id.au \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).