Message ID | 20231102153455.1252-4-michal.wajdeczko@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | ida: Allow allocations of contiguous IDs | expand |
On Thu, Nov 02, 2023 at 04:34:55PM +0100, Michal Wajdeczko wrote: > New functionality of the IDA (contiguous IDs allocations) requires > some validation coverage. Add KUnit tests for simple scenarios: > - counting single ID at different locations > - counting different sets of IDs > - ID allocation start at requested position > - different contiguous ID allocations are supported > > More advanced tests for subtle corner cases may come later. Why are you using kunit instead of extending the existing test-cases?
On 02.11.2023 18:43, Matthew Wilcox wrote: > On Thu, Nov 02, 2023 at 04:34:55PM +0100, Michal Wajdeczko wrote: >> New functionality of the IDA (contiguous IDs allocations) requires >> some validation coverage. Add KUnit tests for simple scenarios: >> - counting single ID at different locations >> - counting different sets of IDs >> - ID allocation start at requested position >> - different contiguous ID allocations are supported >> >> More advanced tests for subtle corner cases may come later. > > Why are you using kunit instead of extending the existing test-cases? I just assumed (maybe wrong) that kunit is preferred these days as some other components are even converting their existing test code to kunit. But also I might be biased as I was working recently with kunit and just found it helpful in fast test development. Note that to run these new IDA tests, anyone who cares just need a single command line: $ ./tools/testing/kunit/kunit.py run "ida.*" But if you feel that having two places with IDA tests is wrong, we can still convert old tests to kunit (either as follow up or prerequisite) to this patch (well, already did that locally when started working on these improvements)
On Thu, Nov 02, 2023 at 07:58:16PM +0100, Michal Wajdeczko wrote: > On 02.11.2023 18:43, Matthew Wilcox wrote: > > On Thu, Nov 02, 2023 at 04:34:55PM +0100, Michal Wajdeczko wrote: > >> New functionality of the IDA (contiguous IDs allocations) requires > >> some validation coverage. Add KUnit tests for simple scenarios: > >> - counting single ID at different locations > >> - counting different sets of IDs > >> - ID allocation start at requested position > >> - different contiguous ID allocations are supported > >> > >> More advanced tests for subtle corner cases may come later. > > > > Why are you using kunit instead of extending the existing test-cases? > > I just assumed (maybe wrong) that kunit is preferred these days as some > other components are even converting their existing test code to kunit. > > But also I might be biased as I was working recently with kunit and just > found it helpful in fast test development. Note that to run these new > IDA tests, anyone who cares just need a single command line: > > $ ./tools/testing/kunit/kunit.py run "ida.*" > > But if you feel that having two places with IDA tests is wrong, we can > still convert old tests to kunit (either as follow up or prerequisite) > to this patch (well, already did that locally when started working on > these improvements) Why would using kunit be superior to the existing test suite?
On 02.11.2023 20:12, Matthew Wilcox wrote: > On Thu, Nov 02, 2023 at 07:58:16PM +0100, Michal Wajdeczko wrote: >> On 02.11.2023 18:43, Matthew Wilcox wrote: >>> On Thu, Nov 02, 2023 at 04:34:55PM +0100, Michal Wajdeczko wrote: >>>> New functionality of the IDA (contiguous IDs allocations) requires >>>> some validation coverage. Add KUnit tests for simple scenarios: >>>> - counting single ID at different locations >>>> - counting different sets of IDs >>>> - ID allocation start at requested position >>>> - different contiguous ID allocations are supported >>>> >>>> More advanced tests for subtle corner cases may come later. >>> >>> Why are you using kunit instead of extending the existing test-cases? >> >> I just assumed (maybe wrong) that kunit is preferred these days as some >> other components are even converting their existing test code to kunit. >> >> But also I might be biased as I was working recently with kunit and just >> found it helpful in fast test development. Note that to run these new >> IDA tests, anyone who cares just need a single command line: >> >> $ ./tools/testing/kunit/kunit.py run "ida.*" >> >> But if you feel that having two places with IDA tests is wrong, we can >> still convert old tests to kunit (either as follow up or prerequisite) >> to this patch (well, already did that locally when started working on >> these improvements) > > Why would using kunit be superior to the existing test suite? As said above IMO it's just a nice tool, that seems to be already used around. If you look for examples where kunit could win over existing ida test suite, then maybe that kunit allows to run only specific test cases or parametrize test cases or provide ready to use nicer diagnostic messages on missed expectations. It should also be easy (and can be done in unified way) to replace some external functions to trigger desired faults (like altering kzalloc() or xas_store() to force different code paths during our alloc). But since I'm a guest here, knowing that there could be different opinions on competing test suites, we can either drop this patch or convert new test cases with 'group' variants to the old test_ida suite (if that's really desired).
On Thu, Nov 02, 2023 at 09:58:07PM +0100, Michal Wajdeczko wrote: > > Why would using kunit be superior to the existing test suite? > > As said above IMO it's just a nice tool, that seems to be already used > around. If you look for examples where kunit could win over existing > ida test suite, then maybe that kunit allows to run only specific test > cases or parametrize test cases or provide ready to use nicer diagnostic > messages on missed expectations. It should also be easy (and can be > done in unified way) to replace some external functions to trigger > desired faults (like altering kzalloc() or xas_store() to force > different code paths during our alloc). > > But since I'm a guest here, knowing that there could be different > opinions on competing test suites, we can either drop this patch or > convert new test cases with 'group' variants to the old test_ida suite > (if that's really desired). AFAIK, kunit can't be used to extract the in-kernel IDA code and run it in userspace like the current testsuite does (the current testsuite also runs in-kernel, except for the multithreaded tests). So unless it has that functionality, it seems like a regression to convert the existing test-suite to kunit.
On 02.11.2023 22:01, Matthew Wilcox wrote: > On Thu, Nov 02, 2023 at 09:58:07PM +0100, Michal Wajdeczko wrote: >>> Why would using kunit be superior to the existing test suite? >> >> As said above IMO it's just a nice tool, that seems to be already used >> around. If you look for examples where kunit could win over existing >> ida test suite, then maybe that kunit allows to run only specific test >> cases or parametrize test cases or provide ready to use nicer diagnostic >> messages on missed expectations. It should also be easy (and can be >> done in unified way) to replace some external functions to trigger >> desired faults (like altering kzalloc() or xas_store() to force >> different code paths during our alloc). >> >> But since I'm a guest here, knowing that there could be different >> opinions on competing test suites, we can either drop this patch or >> convert new test cases with 'group' variants to the old test_ida suite >> (if that's really desired). > > AFAIK, kunit can't be used to extract the in-kernel IDA code and run it > in userspace like the current testsuite does (the current testsuite also > runs in-kernel, except for the multithreaded tests). So unless it has > that functionality, it seems like a regression to convert the existing > test-suite to kunit. But there is no need extract anything as kunit tests run in kernel space (or under QEMU/UML for convenience) so can access all in-kernel API. [1] https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index fbc89baf7de6..818e788bc359 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2777,6 +2777,18 @@ config SIPHASH_KUNIT_TEST This is intended to help people writing architecture-specific optimized versions. If unsure, say N. +config IDA_KUNIT_TEST + tristate "Kunit tests for IDA functions" if !KUNIT_ALL_TESTS + depends on KUNIT + default KUNIT_ALL_TESTS + help + Enable this option to test the kernel's IDA functions. + + For more information on KUnit and unit tests in general please refer + to the KUnit documentation in Documentation/dev-tools/kunit/. + + If unsure, say N. + config TEST_UDELAY tristate "udelay test driver" help diff --git a/lib/Makefile b/lib/Makefile index 42d307ade225..451dbb373da7 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -396,6 +396,7 @@ obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o obj-$(CONFIG_STRCAT_KUNIT_TEST) += strcat_kunit.o obj-$(CONFIG_STRSCPY_KUNIT_TEST) += strscpy_kunit.o obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o +obj-$(CONFIG_IDA_KUNIT_TEST) += ida_kunit.o obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o diff --git a/lib/ida_kunit.c b/lib/ida_kunit.c new file mode 100644 index 000000000000..01dc82c189f9 --- /dev/null +++ b/lib/ida_kunit.c @@ -0,0 +1,140 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test cases for IDA functions. + */ + +#include <linux/idr.h> +#include <kunit/test.h> + +#define IDA_STRESS_LIMIT (IDA_BITMAP_BITS * XA_CHUNK_SIZE + 1) + +static int ida_test_init(struct kunit *test) +{ + static DEFINE_IDA(ida); + + ida_init(&ida); + test->priv = &ida; + return 0; +} + +static void ida_test_exit(struct kunit *test) +{ + struct ida *ida = test->priv; + + ida_destroy(ida); + KUNIT_EXPECT_TRUE(test, ida_is_empty(ida)); +} + +static const unsigned int ida_params[] = { + 0, + 1, + BITS_PER_XA_VALUE - 1, + BITS_PER_XA_VALUE, + BITS_PER_XA_VALUE + 1, + IDA_BITMAP_BITS - BITS_PER_XA_VALUE - 1, + IDA_BITMAP_BITS - BITS_PER_XA_VALUE, + IDA_BITMAP_BITS - BITS_PER_XA_VALUE + 1, + IDA_BITMAP_BITS - 1, + IDA_BITMAP_BITS, + IDA_BITMAP_BITS + 1, + IDA_BITMAP_BITS + BITS_PER_XA_VALUE - 1, + IDA_BITMAP_BITS + BITS_PER_XA_VALUE, + IDA_BITMAP_BITS + BITS_PER_XA_VALUE + 1, + IDA_BITMAP_BITS + IDA_BITMAP_BITS + BITS_PER_XA_VALUE - 1, + IDA_BITMAP_BITS + IDA_BITMAP_BITS + BITS_PER_XA_VALUE + 1, +}; + +static void uint_get_desc(const unsigned int *p, char *desc) +{ + snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%u", *p); +} + +KUNIT_ARRAY_PARAM(ida, ida_params, uint_get_desc); + +static void test_alloc_one(struct kunit *test) +{ + struct ida *ida = test->priv; + const unsigned int *p = test->param_value; + unsigned int min = *p; + + KUNIT_ASSERT_EQ(test, ida_weight(ida), 0); + KUNIT_ASSERT_EQ(test, ida_alloc_min(ida, min, GFP_KERNEL), min); + KUNIT_ASSERT_EQ(test, ida_weight(ida), 1); + ida_free(ida, min); + KUNIT_ASSERT_EQ(test, ida_weight(ida), 0); +} + +static void test_alloc_many(struct kunit *test) +{ + struct ida *ida = test->priv; + const unsigned int *p = test->param_value; + unsigned int n, num = *p; + + KUNIT_ASSERT_EQ(test, ida_weight(ida), 0); + + for (n = 0; n < num; n++) { + KUNIT_ASSERT_EQ(test, ida_alloc(ida, GFP_KERNEL), n); + KUNIT_ASSERT_EQ(test, ida_weight(ida), n + 1); + } + + kunit_info(test, "weight %lu", ida_weight(ida)); + + for (n = 0; n < num; n++) { + ida_free(ida, n); + KUNIT_ASSERT_EQ(test, ida_weight(ida), num - n - 1); + } + + KUNIT_ASSERT_EQ(test, 0, ida_weight(ida)); +} + +static void test_alloc_min(struct kunit *test) +{ + struct ida *ida = test->priv; + const unsigned int *p = test->param_value; + unsigned int n, min = *p; + + KUNIT_ASSERT_EQ(test, ida_weight(ida), 0); + for (n = min; n < IDA_STRESS_LIMIT; n++) { + KUNIT_ASSERT_EQ(test, ida_alloc_min(ida, min, GFP_KERNEL), n); + KUNIT_ASSERT_EQ(test, ida_weight(ida), n - min + 1); + } +} + +static void test_alloc_group(struct kunit *test) +{ + struct ida *ida = test->priv; + const unsigned int *p = test->param_value; + unsigned int n, group = *p; + unsigned long w; + + for (n = 0; n < IDA_STRESS_LIMIT; n += (1 + group)) { + KUNIT_ASSERT_EQ(test, + ida_alloc_group_range(ida, 0, -1, group, GFP_KERNEL), + n); + KUNIT_ASSERT_EQ(test, ida_weight(ida), n + 1 + group); + } + + KUNIT_ASSERT_NE(test, w = ida_weight(ida), 0); + + for (n = 0; n < IDA_STRESS_LIMIT; n += (1 + group)) { + ida_free_group(ida, n, group); + KUNIT_ASSERT_EQ(test, ida_weight(ida), w - n - 1 - group); + } +} + +static struct kunit_case ida_test_cases[] = { + KUNIT_CASE_PARAM(test_alloc_one, ida_gen_params), + KUNIT_CASE_PARAM(test_alloc_many, ida_gen_params), + KUNIT_CASE_PARAM(test_alloc_min, ida_gen_params), + KUNIT_CASE_PARAM(test_alloc_group, ida_gen_params), + {} +}; + +static struct kunit_suite ida_suite = { + .name = "ida", + .test_cases = ida_test_cases, + .init = ida_test_init, + .exit = ida_test_exit, +}; + +kunit_test_suites(&ida_suite);
New functionality of the IDA (contiguous IDs allocations) requires some validation coverage. Add KUnit tests for simple scenarios: - counting single ID at different locations - counting different sets of IDs - ID allocation start at requested position - different contiguous ID allocations are supported More advanced tests for subtle corner cases may come later. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Matthew Wilcox <willy@infradead.org> --- lib/Kconfig.debug | 12 ++++ lib/Makefile | 1 + lib/ida_kunit.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 153 insertions(+) create mode 100644 lib/ida_kunit.c