Re: [PATCH] lib/atomic64_test.c: convert to use KUnit

On Tue, May 3, 2022 at 3:23 AM Daniel Latypov dlatypov@google.com wrote:

The test currently is a bunch of checks (implemented using BUG_ON()) that can be built into the kernel or as a module.

Convert it to a KUnit test, which can also run in both modes. From a user's perspective, this change adds a CONFIG_KUNIT=y dep and changes the output format of the test [1]. The test itself is the same.

This hopefully makes the test easier to run and more consistent with similar tests in lib/. Since it has no dependencies, it can be run without explicitly setting up a .kunitconfig via $ ./tools/testing/kunit/kunit.py run atomic ... [13:53:44] Starting KUnit Kernel (1/1)... [13:53:44] ============================================================ [13:53:47] =================== atomic (2 subtests) ==================== [13:53:47] [PASSED] test_atomic [13:53:47] [PASSED] test_atomic64 [13:53:47] ===================== [PASSED] atomic ====================== [13:53:47] ============================================================ [13:53:47] Testing complete. Passed: 2, Failed: 0, Crashed: 0, Skipped: 0, Errors: 0 [13:53:47] Elapsed time: 13.902s total, 1.629s configuring, 9.331s building, 2.852s running

It can be run on ARCH=x86_64 (and others) via: $ ./tools/testing/kunit/kunit.py run --arch=x86_64 atomic

The message about which platform the test ran on won't show up in kunit.py, but still gets printed out in dmesg, e.g.

...

TAP version 14 1..1 # Subtest: atomic 1..2 ok 1 - test_atomic ok 2 - test_atomic64 # atomic: ran on x86-64 platform with CX8 and with SSE # atomic: pass:2 fail:0 skip:0 total:2 # Totals: pass:2 fail:0 skip:0 total:2 ok 1 - atomic

Signed-off-by: Daniel Latypov dlatypov@google.com

[1] https://www.kernel.org/doc/html/latest/dev-tools/ktap.html

This looks good to me. It's nice to see these tests being standardised.

Meta:

1. this patch applies on top of the kunit branch,

https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/?h...

(To anyone else trying it: make sure you update this first, as the prerequisites only just landed.)

2. checkpatch complains about aligning with parens, but it wants me to

indent the `#ifdef CONFIG_X86_64` which seems inappropriate in context.

Huh... checkpatch isn't showing any such warning on my system.

3. this file doesn't seem to have a clear maintainer, so I assume this

conversion is fine to go through the kunit branch. The only observable differences are the new CONFIG_KUNIT=y dep and more standardized (KTAP) output format.

---

I'm happy to vouch for this not breaking anything, even though I'm definitely not an expert on the atomics code.

Reviewed-by: David Gow davidgow@google.com

-- David

lib/Kconfig.debug | 4 +- lib/atomic64_test.c | 107 +++++++++++++++++++++----------------------- 2 files changed, 55 insertions(+), 56 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 075cd25363ac..4cf8d5feda0a 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2223,7 +2223,9 @@ config PERCPU_TEST If unsure, say N.

config ATOMIC64_SELFTEST

•   tristate "Perform an atomic64_t self-test"
  

•   tristate "Perform an atomic64_t self-test" if !KUNIT_ALL_TESTS
  

•   depends on KUNIT
  

•   default KUNIT_ALL_TESTS
    help
      Enable this option to test the atomic64_t functions at boot or
      at module load time.
  

diff --git a/lib/atomic64_test.c b/lib/atomic64_test.c index d9d170238165..46cb0130f8d0 100644 --- a/lib/atomic64_test.c +++ b/lib/atomic64_test.c @@ -5,13 +5,9 @@

• Copyright © 2010 Luca Barbieri

*/

-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <kunit/test.h>

-#include <linux/init.h> -#include <linux/bug.h> -#include <linux/kernel.h> #include <linux/atomic.h> -#include <linux/module.h>

#ifdef CONFIG_X86 #include <asm/cpufeature.h> /* for boot_cpu_has below */ @@ -23,9 +19,7 @@ do { \ r = v0; \ atomic##bit##_##op(val, &v); \ r c_op val; \

•   WARN(atomic##bit##_read(&v) != r, "%Lx != %Lx\n",       \
  

•           (unsigned long long)atomic##bit##_read(&v),     \
  

•           (unsigned long long)r);                         \
  

•   KUNIT_ASSERT_EQ(test, atomic##bit##_read(&v), r);       \
  

} while (0)

/* @@ -46,8 +40,8 @@ do { \ atomic##bit##_set(&v, v0); \ r = v0; \ r c_op val; \

•   BUG_ON(atomic##bit##_##op(val, &v) != r);               \
  

•   BUG_ON(atomic##bit##_read(&v) != r);                    \
  

•   KUNIT_ASSERT_EQ(test, atomic##bit##_##op(val, &v), r);  \
  

•   KUNIT_ASSERT_EQ(test, atomic##bit##_read(&v), r);       \
  

} while (0)

#define TEST_FETCH(bit, op, c_op, val) \ @@ -55,8 +49,8 @@ do { \ atomic##bit##_set(&v, v0); \ r = v0; \ r c_op val; \

•   BUG_ON(atomic##bit##_##op(val, &v) != v0);              \
  

•   BUG_ON(atomic##bit##_read(&v) != r);                    \
  

•   KUNIT_ASSERT_EQ(test, atomic##bit##_##op(val, &v), v0); \
  

•   KUNIT_ASSERT_EQ(test, atomic##bit##_read(&v), r);       \
  

} while (0)

#define RETURN_FAMILY_TEST(bit, op, c_op, val) \ @@ -72,8 +66,8 @@ do { \ #define TEST_ARGS(bit, op, init, ret, expect, args...) \ do { \ atomic##bit##_set(&v, init); \

•   BUG_ON(atomic##bit##_##op(&v, ##args) != ret);          \
  

•   BUG_ON(atomic##bit##_read(&v) != expect);               \
  

•   KUNIT_ASSERT_EQ(test, atomic##bit##_##op(&v, ##args), ret);\
  

•   KUNIT_ASSERT_EQ(test, atomic##bit##_read(&v), expect);  \
  

} while (0)

#define XCHG_FAMILY_TEST(bit, init, new) \ @@ -101,7 +95,7 @@ do { \ i, (i) - one, (i) - one); \ } while (0)

-static __init void test_atomic(void) +static void test_atomic(struct kunit *test) { int v0 = 0xaaa31337; int v1 = 0xdeadbeef; @@ -144,7 +138,7 @@ static __init void test_atomic(void) }

#define INIT(c) do { atomic64_set(&v, c); r = c; } while (0) -static __init void test_atomic64(void) +static void test_atomic64(struct kunit *test) { long long v0 = 0xaaa31337c001d00dLL; long long v1 = 0xdeadbeefdeafcafeLL; @@ -156,12 +150,12 @@ static __init void test_atomic64(void)

    atomic64_t v = ATOMIC64_INIT(v0);
    long long r = v0;

•   BUG_ON(v.counter != r);
  

•   KUNIT_ASSERT_EQ(test, v.counter, r);
  
    atomic64_set(&v, v1);
    r = v1;
  

•   BUG_ON(v.counter != r);
  

•   BUG_ON(atomic64_read(&v) != r);
  

•   KUNIT_ASSERT_EQ(test, v.counter, r);
  

•   KUNIT_ASSERT_EQ(test, atomic64_read(&v), r);
  
    TEST(64, add, +=, onestwos);
    TEST(64, add, +=, -one);
  

@@ -190,12 +184,12 @@ static __init void test_atomic64(void) INIT(v0); atomic64_inc(&v); r += one;

•   BUG_ON(v.counter != r);
  

•   KUNIT_ASSERT_EQ(test, v.counter, r);
  
    INIT(v0);
    atomic64_dec(&v);
    r -= one;
  

•   BUG_ON(v.counter != r);
  

•   KUNIT_ASSERT_EQ(test, v.counter, r);
  
    INC_RETURN_FAMILY_TEST(64, v0);
    DEC_RETURN_FAMILY_TEST(64, v0);
  

@@ -204,73 +198,76 @@ static __init void test_atomic64(void) CMPXCHG_FAMILY_TEST(64, v0, v1, v2);

    INIT(v0);

•   BUG_ON(atomic64_add_unless(&v, one, v0));
  

•   BUG_ON(v.counter != r);
  

•   KUNIT_ASSERT_FALSE(test, atomic64_add_unless(&v, one, v0));
  

•   KUNIT_ASSERT_EQ(test, v.counter, r);
  
    INIT(v0);
  

•   BUG_ON(!atomic64_add_unless(&v, one, v1));
  

•   KUNIT_ASSERT_TRUE(test, atomic64_add_unless(&v, one, v1));
    r += one;
  

•   BUG_ON(v.counter != r);
  

•   KUNIT_ASSERT_EQ(test, v.counter, r);
  
    INIT(onestwos);
  

•   BUG_ON(atomic64_dec_if_positive(&v) != (onestwos - 1));
  

•   KUNIT_ASSERT_EQ(test, atomic64_dec_if_positive(&v), (onestwos - 1));
    r -= one;
  

•   BUG_ON(v.counter != r);
  

•   KUNIT_ASSERT_EQ(test, v.counter, r);
  
    INIT(0);
  

•   BUG_ON(atomic64_dec_if_positive(&v) != -one);
  

•   BUG_ON(v.counter != r);
  

•   KUNIT_ASSERT_EQ(test, atomic64_dec_if_positive(&v), -one);
  

•   KUNIT_ASSERT_EQ(test, v.counter, r);
  
    INIT(-one);
  

•   BUG_ON(atomic64_dec_if_positive(&v) != (-one - one));
  

•   BUG_ON(v.counter != r);
  

•   KUNIT_ASSERT_EQ(test, atomic64_dec_if_positive(&v), (-one - one));
  

•   KUNIT_ASSERT_EQ(test, v.counter, r);
  
    INIT(onestwos);
  

•   BUG_ON(!atomic64_inc_not_zero(&v));
  

•   KUNIT_ASSERT_TRUE(test, atomic64_inc_not_zero(&v));
    r += one;
  

•   BUG_ON(v.counter != r);
  

•   KUNIT_ASSERT_EQ(test, v.counter, r);
  
    INIT(0);
  

•   BUG_ON(atomic64_inc_not_zero(&v));
  

•   BUG_ON(v.counter != r);
  

•   KUNIT_ASSERT_FALSE(test, atomic64_inc_not_zero(&v));
  

•   KUNIT_ASSERT_EQ(test, v.counter, r);
  
    INIT(-one);
  

•   BUG_ON(!atomic64_inc_not_zero(&v));
  

•   KUNIT_ASSERT_TRUE(test, atomic64_inc_not_zero(&v));
    r += one;
  

•   BUG_ON(v.counter != r);
  

•   KUNIT_ASSERT_EQ(test, v.counter, r);
  
    /* Confirm the return value fits in an int, even if the value doesn't */
    INIT(v3);
  

•   r_int = atomic64_inc_not_zero(&v);
  

•   BUG_ON(!r_int);
  

•   KUNIT_ASSERT_NE(test, r_int, 0);
  

}

-static __init int test_atomics_init(void) -{

•   test_atomic();
  

•   test_atomic64();
  

+static struct kunit_case atomic_test_cases[] = {

•   KUNIT_CASE(test_atomic),
  

•   KUNIT_CASE(test_atomic64),
  

•   {},
  

+};

+static void atomic_suite_exit(struct kunit_suite *suite) +{ #ifdef CONFIG_X86

•   pr_info("passed for %s platform %s CX8 and %s SSE\n",
  

•   kunit_info(suite, "ran on %s platform %s CX8 and %s SSE\n",
  

#ifdef CONFIG_X86_64

•           "x86-64",
  

•              "x86-64",
  

#elif defined(CONFIG_X86_CMPXCHG64)

•           "i586+",
  

•              "i586+",
  

#else

•           "i386+",
  

•              "i386+",
  

#endif

•          boot_cpu_has(X86_FEATURE_CX8) ? "with" : "without",
  

•          boot_cpu_has(X86_FEATURE_XMM) ? "with" : "without");
  

-#else

•   pr_info("passed\n");
  

•              boot_cpu_has(X86_FEATURE_CX8) ? "with" : "without",
  

•              boot_cpu_has(X86_FEATURE_XMM) ? "with" : "without");
  

#endif

•   return 0;
  

}

-static __exit void test_atomics_exit(void) {} +static struct kunit_suite atomic_test_suite = {

•   .name = "atomic",
  

•   .test_cases = atomic_test_cases,
  

•   .suite_exit = atomic_suite_exit,
  

+};

-module_init(test_atomics_init); -module_exit(test_atomics_exit); +kunit_test_suites(&atomic_test_suite);

MODULE_LICENSE("GPL");

base-commit: 38289a26e1b8a37755f3e07056ca416c1ee2a2e8

2.36.0.464.gb9c8b46e94-goog