quoted

+/*
+ * Test bitmap should be big enough to include the cases when start is not in
+ * the first word, and start+nbits lands in the following word.
+ */
+#define TEST_BIT_LEN (BITS_PER_LONG * 3)

Why not just 1000? Is your code safe against unaligned bitmaps?

done

quoted

+#define TEST_BYTE_LEN (BITS_TO_LONGS(TEST_BIT_LEN) * sizeof(unsigned long))

BITS_TO_BYTES

done

quoted

+static void __init test_set_get_value(void)

test_bitmap_read_write. Here, and in subjects for #1 and #2.

done

quoted

+{
+     DECLARE_BITMAP(bitmap, TEST_BIT_LEN);
+     DECLARE_BITMAP(exp_bitmap, TEST_BIT_LEN);
+     /* Prevent constant folding. */
+     volatile unsigned long zero_bits = 0;

Use READ_ONCE() instead of volatile

done

quoted

+     unsigned long val, bit;
+     int i;
+
+     /* Setting/getting zero bytes should not crash the kernel. */
+     bitmap_write(NULL, 0, 0, zero_bits);
+     val = bitmap_read(NULL, 0, zero_bits);
+     expect_eq_ulong(0, val);

No, val is undefined.

Why? bitmap_read(..., ..., 0) always returns 0.

quoted

+
+     /*
+      * Ensure that bitmap_read() reads the same value that was previously
+      * written, and two consequent values are correctly merged.
+      * The resulting bit pattern is asymmetric to rule out possible issues
+      * with bit numeration order.
+      */
+     for (i = 0; i < TEST_BIT_LEN - 7; i++) {

Can you add some empty lines in the block below in sake of
readability? Maybe after expect()?

Done

(snip)

I suggested the other test:

val = DEADBEEF;
for (nbits = 1; nbits <= BITS_PER_LONG; nbits++)
        for (start = 0; start < 1000; i++) {
                if (start + nbits >= 1000)
                        break;;

                v = val & GENMASK(nbits - 1, 0);

                memset(bitmap, 0xaa /* also 0xff and 0x00 */, TEST_BYTE_LEN);
                memset(exp_bitmap, 0xaa, TEST_BYTE_LEN);

                for (n = 0; n < nbits; n++)
                        __assign_bit(v & BIT(n), exp_bitmap, start + n);

                bitmap_write(bitmap, v, start, nbits);
                expect_eq_bitmap(exp_bitmap, bitmap, 1000);

                r = bitmap_read(bitmap, start, nbits);
                expect_eq(r, v);
        }

I factored out test cases that can benefit from different background
patterns into a separate function and added yours there. Thanks!

quoted

@@ -1249,6 +1328,8 @@ static void __init selftest(void)
      test_for_each_clear_bitrange_from();
      test_for_each_set_clump8();
      test_for_each_set_bit_wrap();
+
+     test_set_get_value();

This should append the test_bitmap_* section

Done

Thanks,
Yury

Thanks a lot for your comments!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel