| Message ID | 20260311070416.972667-28-hch@lst.de (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [01/27] xor: assert that xor_blocks is not from preemptible user context | expand |
On Wed, Mar 11, 2026 at 08:03:59AM +0100, Christoph Hellwig wrote: > diff --git a/lib/raid/Kconfig b/lib/raid/Kconfig > index 4359971ebd04..97c123806466 100644 > --- a/lib/raid/Kconfig > +++ b/lib/raid/Kconfig > @@ -6,3 +6,14 @@ config XOR_BLOCKS > # selected by architectures that provide an optimized XOR implementation > config XOR_BLOCKS_ARCH > bool > + > +config XOR_KUNIT_TEST > + tristate "KUnit tests for xor_gen" if !KUNIT_ALL_TESTS > + depends on KUNIT > + default KUNIT_ALL_TESTS > + select XOR_BLOCKS > + help > + Unit tests for the XOR library functions. > + > + This is intended to help people writing architecture-specific > + optimized versions. If unsure, say N. The convention for KUnit tests is actually to depend on the code they test, not select it, so that it's easy to enable only the tests that are relevant to a particular kernel build. So instead of "select XOR_BLOCKS", this should use "depends on KUNIT && XOR_BLOCKS". (Yes, I got this wrong in the crypto and CRC tests. I recently fixed it in the crypto tests, and I have pending patches that fix the CRC test.) There should also be a lib/raid/.kunitconfig file containing something like: CONFIG_KUNIT=y CONFIG_BTRFS_FS=y CONFIG_XOR_KUNIT_TEST=y (CONFIG_BTRFS_FS is there because it's one of the visible symbols that select the hidden symbol XOR_BLOCKS.) > +static u32 rand32(void) > +{ > + return prandom_u32_state(&rng); > +} > + > +static u32 rand32_below(u32 ceil) > +{ > + return __limit_random_u32_below(ceil, prandom_u32_state(&rng)); > +} > + [...] > + > +/* Generate a random length that is a multiple of 512. */ > +static unsigned int generate_random_length(unsigned int max_length) > +{ > + return (rand32_below(max_length / 512) + 1) * 512; > +} > + > +/* Generate a random alignment that is a multiple of 32. */ > +static unsigned int generate_random_alignment(unsigned int max_alignment) > +{ > + return (rand32_below((max_alignment + 1) / 32)) * 32; > +} As per my comment on patch 26, these should just use a simple mod operations so that the new random.c helper function (which conflates cryptographic and non-cryptographic random numbers) isn't needed. Maybe: return (rand32() % (max_length + 1)) & ~511; and return (rand32() % (max_alignment + 1)) & ~63; > +/* Test that xor_gen gives the same result as a reference implementation. */ > +static void xor_test(struct kunit *test) > +{ > + void *aligned_buffers[XOR_KUNIT_MAX_BUFFERS]; > + size_t i; > + > + for (i = 0; i < XOR_KUNIT_NUM_TEST_ITERS; i++) { > + unsigned int nr_buffers = > + rand32_below(XOR_KUNIT_MAX_BUFFERS) + 1; > + unsigned int len = generate_random_length(XOR_KUNIT_MAX_BYTES); > + unsigned int max_alignment, align = 0; > + void *buffers; > + > + if (rand32() % 8 == 0) > + /* Refresh the data occasionally. */ > + xor_generate_random_data(); > + > + /* > + * If we're not using the entire buffer size, inject randomize > + * alignment into the buffer. > + */ > + max_alignment = XOR_KUNIT_MAX_BYTES - len; > + if (max_alignment) { > + int j; > + > + align = generate_random_alignment(max_alignment); > + for (j = 0; j < nr_buffers; j++) > + aligned_buffers[j] = test_buffers[j] + > + generate_random_alignment(max_alignment); > + buffers = aligned_buffers; > + } else { > + buffers = test_buffers; > + } This isn't taking advantage of the guard pages properly, since it rarely selects buffers that go all the way up to the guard page. If the guard page testing is going to be included (which is a good idea; the crypto and CRC tests have it and they already caught a bug using it), then the data should be placed at the very end of the buffers more often, like what the CRC test does. > + /* > + * Compute the XOR, and verify that it equals the XOR computed > + * by a simple byte-at-a-time reference implementation. > + */ > + xor_ref(test_ref + align, buffers, nr_buffers, len); > + xor_gen(test_dest + align, buffers, nr_buffers, len); > + KUNIT_EXPECT_MEMEQ_MSG(test, test_ref, test_dest, len, > + "Wrong result with buffers=%u, len=%u, align=%s", > + nr_buffers, len, str_yes_no(max_alignment)); When align != 0, this does the comparison at the wrong offset. The message also shows "align=no" if fully aligned buffers were used and "align=yes" if they were not, which is a bit confusing. Maybe replace align=%s with randalign=%s. > +MODULE_DESCRIPTION("Unit tests and benchmarks for the XOR library functions"); There's no benchmark included (yet), so that should be left out of the description. Also, I tried running this test on different architectures, and in qemu-system-sparc64 it crashes with an alignment fault in xor_vis_5(). It goes away if the minimum tested alignment is increased from 32 bytes to 64 bytes. lib/raid/xor/sparc/xor-sparc64.S has a comment that documents a requirement of "!(((long)dest | (long)sourceN) & (64 - 1))", i.e. 64-byte alignment. So, it seems the assumption that 32 bytes is the maximum required alignment over all architectures is not correct. The tested alignment will need to be increased to 64 bytes, and the kerneldoc for xor_gen() will need to be updated as well. - Eric
diff --git a/lib/raid/Kconfig b/lib/raid/Kconfig index 4359971ebd04..97c123806466 100644 --- a/lib/raid/Kconfig +++ b/lib/raid/Kconfig @@ -6,3 +6,14 @@ config XOR_BLOCKS # selected by architectures that provide an optimized XOR implementation config XOR_BLOCKS_ARCH bool + +config XOR_KUNIT_TEST + tristate "KUnit tests for xor_gen" if !KUNIT_ALL_TESTS + depends on KUNIT + default KUNIT_ALL_TESTS + select XOR_BLOCKS + help + Unit tests for the XOR library functions. + + This is intended to help people writing architecture-specific + optimized versions. If unsure, say N. diff --git a/lib/raid/xor/Makefile b/lib/raid/xor/Makefile index 7b748ddda9d4..74185bdc3dd8 100644 --- a/lib/raid/xor/Makefile +++ b/lib/raid/xor/Makefile @@ -30,7 +30,7 @@ xor-$(CONFIG_SPARC64) += sparc/xor-sparc64.o sparc/xor-sparc64-glue.o xor-$(CONFIG_S390) += s390/xor.o xor-$(CONFIG_X86_32) += x86/xor-avx.o x86/xor-sse.o x86/xor-mmx.o xor-$(CONFIG_X86_64) += x86/xor-avx.o x86/xor-sse.o - +obj-y += tests/ CFLAGS_arm/xor-neon.o += $(CC_FLAGS_FPU) CFLAGS_REMOVE_arm/xor-neon.o += $(CC_FLAGS_NO_FPU) diff --git a/lib/raid/xor/tests/Makefile b/lib/raid/xor/tests/Makefile new file mode 100644 index 000000000000..661e8f6ffd1f --- /dev/null +++ b/lib/raid/xor/tests/Makefile @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0-only + +obj-$(CONFIG_XOR_KUNIT_TEST) += xor_kunit.o diff --git a/lib/raid/xor/tests/xor_kunit.c b/lib/raid/xor/tests/xor_kunit.c new file mode 100644 index 000000000000..23ee415e914c --- /dev/null +++ b/lib/raid/xor/tests/xor_kunit.c @@ -0,0 +1,180 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Unit test the XOR library functions. + * + * Copyright 2024 Google LLC + * Copyright 2026 Christoph Hellwig + * + * Based on the CRC tests by Eric Biggers <ebiggers@google.com>. + */ +#include <kunit/test.h> +#include <linux/prandom.h> +#include <linux/string_choices.h> +#include <linux/vmalloc.h> +#include <linux/raid/xor.h> + +#define XOR_KUNIT_SEED 42 +#define XOR_KUNIT_MAX_BYTES 16384 +#define XOR_KUNIT_MAX_BUFFERS 64 +#define XOR_KUNIT_NUM_TEST_ITERS 1000 + +static struct rnd_state rng; +static void *test_buffers[XOR_KUNIT_MAX_BUFFERS]; +static void *test_dest; +static void *test_ref; +static size_t test_buflen; + +static u32 rand32(void) +{ + return prandom_u32_state(&rng); +} + +static u32 rand32_below(u32 ceil) +{ + return __limit_random_u32_below(ceil, prandom_u32_state(&rng)); +} + +/* Reference implementation using dumb byte-wise XOR */ +static void xor_ref(void *dest, void **srcs, unsigned int src_cnt, + unsigned int bytes) +{ + unsigned int off, idx; + u8 *d = dest; + + for (off = 0; off < bytes; off++) { + for (idx = 0; idx < src_cnt; idx++) { + u8 *src = srcs[idx]; + + d[off] ^= src[off]; + } + } +} + +/* Generate a random length that is a multiple of 512. */ +static unsigned int generate_random_length(unsigned int max_length) +{ + return (rand32_below(max_length / 512) + 1) * 512; +} + +/* Generate a random alignment that is a multiple of 32. */ +static unsigned int generate_random_alignment(unsigned int max_alignment) +{ + return (rand32_below((max_alignment + 1) / 32)) * 32; +} + +static void xor_generate_random_data(void) +{ + int i; + + prandom_bytes_state(&rng, test_dest, test_buflen); + memcpy(test_ref, test_dest, test_buflen); + for (i = 0; i < XOR_KUNIT_MAX_BUFFERS; i++) + prandom_bytes_state(&rng, test_buffers[i], test_buflen); +} + +/* Test that xor_gen gives the same result as a reference implementation. */ +static void xor_test(struct kunit *test) +{ + void *aligned_buffers[XOR_KUNIT_MAX_BUFFERS]; + size_t i; + + for (i = 0; i < XOR_KUNIT_NUM_TEST_ITERS; i++) { + unsigned int nr_buffers = + rand32_below(XOR_KUNIT_MAX_BUFFERS) + 1; + unsigned int len = generate_random_length(XOR_KUNIT_MAX_BYTES); + unsigned int max_alignment, align = 0; + void *buffers; + + if (rand32() % 8 == 0) + /* Refresh the data occasionally. */ + xor_generate_random_data(); + + /* + * If we're not using the entire buffer size, inject randomize + * alignment into the buffer. + */ + max_alignment = XOR_KUNIT_MAX_BYTES - len; + if (max_alignment) { + int j; + + align = generate_random_alignment(max_alignment); + for (j = 0; j < nr_buffers; j++) + aligned_buffers[j] = test_buffers[j] + + generate_random_alignment(max_alignment); + buffers = aligned_buffers; + } else { + buffers = test_buffers; + } + + /* + * Compute the XOR, and verify that it equals the XOR computed + * by a simple byte-at-a-time reference implementation. + */ + xor_ref(test_ref + align, buffers, nr_buffers, len); + xor_gen(test_dest + align, buffers, nr_buffers, len); + KUNIT_EXPECT_MEMEQ_MSG(test, test_ref, test_dest, len, + "Wrong result with buffers=%u, len=%u, align=%s", + nr_buffers, len, str_yes_no(max_alignment)); + } +} + +static struct kunit_case xor_test_cases[] = { + KUNIT_CASE(xor_test), + {}, +}; + +static int xor_suite_init(struct kunit_suite *suite) +{ + int i; + + /* + * Allocate the test buffer using vmalloc() with a page-aligned length + * so that it is immediately followed by a guard page. This allows + * buffer overreads to be detected, even in assembly code. + */ + test_buflen = round_up(XOR_KUNIT_MAX_BYTES, PAGE_SIZE); + test_ref = vmalloc(test_buflen); + if (!test_ref) + return -ENOMEM; + test_dest = vmalloc(test_buflen); + if (!test_dest) + goto out_free_ref; + for (i = 0; i < XOR_KUNIT_MAX_BUFFERS; i++) { + test_buffers[i] = vmalloc(test_buflen); + if (!test_buffers[i]) + goto out_free_buffers; + } + + prandom_seed_state(&rng, XOR_KUNIT_SEED); + xor_generate_random_data(); + return 0; + +out_free_buffers: + while (--i >= 0) + vfree(test_buffers[i]); + vfree(test_dest); +out_free_ref: + vfree(test_ref); + return -ENOMEM; +} + +static void xor_suite_exit(struct kunit_suite *suite) +{ + int i; + + vfree(test_ref); + vfree(test_dest); + for (i = 0; i < XOR_KUNIT_MAX_BUFFERS; i++) + vfree(test_buffers[i]); +} + +static struct kunit_suite xor_test_suite = { + .name = "xor", + .test_cases = xor_test_cases, + .suite_init = xor_suite_init, + .suite_exit = xor_suite_exit, +}; +kunit_test_suite(xor_test_suite); + +MODULE_DESCRIPTION("Unit tests and benchmarks for the XOR library functions"); +MODULE_LICENSE("GPL");
Add a test case for the XOR routines loosely based on the CRC kunit test. Signed-off-by: Christoph Hellwig <hch@lst.de> --- lib/raid/Kconfig | 11 ++ lib/raid/xor/Makefile | 2 +- lib/raid/xor/tests/Makefile | 3 + lib/raid/xor/tests/xor_kunit.c | 180 +++++++++++++++++++++++++++++++++ 4 files changed, 195 insertions(+), 1 deletion(-) create mode 100644 lib/raid/xor/tests/Makefile create mode 100644 lib/raid/xor/tests/xor_kunit.c