All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] bitmap: convert self-test to KUnit
@ 2025-12-22 13:39 Tamir Duberstein
  2025-12-22 13:39 ` [PATCH v2 1/3] test_bitmap: extract benchmark module Tamir Duberstein
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Tamir Duberstein @ 2025-12-22 13:39 UTC (permalink / raw)
  To: David Gow, John Hubbard, Andrew Morton, Geert Uytterhoeven,
	Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
	Naveen N Rao, Yury Norov, Rasmus Villemoes, Shuah Khan, Kees Cook,
	Christophe Leroy
  Cc: Muhammad Usama Anjum, linux-kernel, linux-m68k, linuxppc-dev,
	linux-kselftest, Tamir Duberstein, Alexander Potapenko

This is the last remaining "Test Module" kselftest, the rest having been
converted to KUnit.

Relative to v1 this keeps benchmarks out of KUnit in light of Yury's
concerns:

On Sat, Feb 8, 2025 at 12:53 PM Yury Norov <yury.norov@gmail.com> wrote:
>
> [...]
>
> This is my evidence: sometimes people report performance or whatever
> issues on their systems, suspecting bitmaps guilty. I ask them to run
> the bitmap or find_bit test to narrow the problem. Sometimes I need to
> test a hardware I have no access to, and I have to (kindly!) ask
people
> to build a small test and run it. I don't want to ask them to rebuild
> the whole kernel, or even to build something else.
>
> https://lore.kernel.org/all/YuWk3titnOiQACzC@yury-laptop/

I tested this using:

$ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 bitmap

There was a previous attempt[2] to do this in July 2024. Please bear
with me as I try to understand and address the objections from that
time. I've spoken with Muhammad Usama Anjum, the author of that series,
and received their approval to "take over" this work. Here we go...

On 7/26/24 11:45 PM, John Hubbard wrote:
> 
> This changes the situation from "works for Linus' tab completion
> case", to "causes a tab completion problem"! :)
> 
> I think a tests/ subdir is how we eventually decided to do this [1],
> right?
> 
> So:
> 
>     lib/tests/bitmap_kunit.c
> 
> [1] https://lore.kernel.org/20240724201354.make.730-kees@kernel.org

This is true and unfortunate, but not trivial to fix because new
kallsyms tests were placed in lib/tests in commit 84b4a51fce4c
("selftests: add new kallsyms selftests")  *after* the KUnit filename
best practices were adopted.

I propose that the KUnit maintainers blaze this trail using
`string_kunit.c` which currently still lives in lib/ despite the KUnit
docs giving it as an example at lib/tests/.

On 7/27/24 12:24 AM, Shuah Khan wrote:
> 
> This change will take away the ability to run bitmap tests during
> boot on a non-kunit kernel.
> 
> Nack on this change. I wan to see all tests that are being removed
> from lib because they have been converted - also it doesn't make
> sense to convert some tests like this one that add the ability test
> during boot.

This point was also discussed in another thread[3] in which:

On 7/27/24 12:35 AM, Shuah Khan wrote:
> 
> Please make sure you aren't taking away the ability to run these tests during
> boot. 
>
> It doesn't make sense to convert every single test especially when it
> is intended to be run during boot without dependencies - not as a kunit test
> but a regression test during boot.
> 
> bitmap is one example - pay attention to the config help test - bitmap
> one clearly states it runs regression testing during boot. Any test that
> says that isn't a candidate for conversion.
> 
> I am going to nack any such conversions.

The crux of the argument seems to be that the config help text is taken
to describe the author's intent with the fragment "at boot". I think
this may be a case of confirmation bias: I see at least the following
KUnit tests with "at boot" in their help text:
- CPUMASK_KUNIT_TEST
- BITFIELD_KUNIT
- CHECKSUM_KUNIT
- UTIL_MACROS_KUNIT

It seems to me that the inference being made is that any test that runs
"at boot" is intended to be run by both developers and users, but I find
no evidence that bitmap in particular would ever provide additional
value when run by users.

There's further discussion about KUnit not being "ideal for cases where
people would want to check a subsystem on a running kernel", but I find
no evidence that bitmap in particular is actually testing the running
kernel; it is a unit test of the bitmap functions, which is also stated
in the config help text.

David Gow made many of the same points in his final reply[4], which was
never replied to.

Link: https://lore.kernel.org/all/20250207-printf-kunit-convert-v2-0-057b23860823@gmail.com/T/#u [0]
Link: https://lore.kernel.org/all/20250207-scanf-kunit-convert-v4-0-a23e2afaede8@gmail.com/T/#u [1]
Link: https://lore.kernel.org/all/20240726110658.2281070-1-usama.anjum@collabora.com/T/#u [2]
Link: https://lore.kernel.org/all/327831fb-47ab-4555-8f0b-19a8dbcaacd7@collabora.com/T/#u [3]
Link: https://lore.kernel.org/all/CABVgOSmMoPD3JfzVd4VTkzGL2fZCo8LfwzaVSzeFimPrhgLa5w@mail.gmail.com/ [4]

Thanks for your attention.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Changes in v2:
- Rebase on v6.19-rc1, dropping the first patch.
- Extract benchmarks into new module and deduplicate
  `test_bitmap_{read,write}_perf`.
- Remove tc_err() and use KUnit assertions.
- Parameterize `test_bitmap_cut` and `test_bitmap_parse{,list}`.
- Drop KUnit boilerplate from BITMAP_KUNIT_TEST help text.
- Drop arch changes.
- Link to v1: https://lore.kernel.org/r/20250207-bitmap-kunit-convert-v1-0-c520675343b6@gmail.com

---
Tamir Duberstein (3):
      test_bitmap: extract benchmark module
      bitmap: convert self-test to KUnit
      bitmap: break kunit into test cases

 MAINTAINERS                           |   3 +-
 lib/Kconfig.debug                     |  16 +-
 lib/Makefile                          |   5 +-
 lib/bitmap_benchmark.c                |  89 +++++
 lib/{test_bitmap.c => bitmap_kunit.c} | 630 ++++++++++++++--------------------
 tools/testing/selftests/lib/Makefile  |   2 +-
 tools/testing/selftests/lib/bitmap.sh |   3 -
 tools/testing/selftests/lib/config    |   1 -
 8 files changed, 360 insertions(+), 389 deletions(-)
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20250207-bitmap-kunit-convert-92d3147b2eee

Best regards,
--  
Tamir Duberstein <tamird@gmail.com>


^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v2 3/3] bitmap: break kunit into test cases
@ 2025-12-23 15:36 kernel test robot
  0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-12-23 15:36 UTC (permalink / raw)
  To: oe-kbuild; +Cc: lkp

:::::: 
:::::: Manual check reason: "__compiletime_assert_NNN"
:::::: 

BCC: lkp@intel.com
CC: llvm@lists.linux.dev
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20251222-bitmap-kunit-convert-v2-3-6a61a5330eff@gmail.com>
References: <20251222-bitmap-kunit-convert-v2-3-6a61a5330eff@gmail.com>
TO: Tamir Duberstein <tamird@kernel.org>
TO: David Gow <davidgow@google.com>
TO: John Hubbard <jhubbard@nvidia.com>
TO: Andrew Morton <akpm@linux-foundation.org>
CC: Linux Memory Management List <linux-mm@kvack.org>
TO: Geert Uytterhoeven <geert@linux-m68k.org>
TO: Madhavan Srinivasan <maddy@linux.ibm.com>
TO: Michael Ellerman <mpe@ellerman.id.au>
TO: Nicholas Piggin <npiggin@gmail.com>
TO: Naveen N Rao <naveen@kernel.org>
TO: Yury Norov <yury.norov@gmail.com>
TO: Rasmus Villemoes <linux@rasmusvillemoes.dk>
TO: Shuah Khan <skhan@linuxfoundation.org>
TO: Kees Cook <kees@kernel.org>
TO: Christophe Leroy <chleroy@kernel.org>
CC: Muhammad Usama Anjum <usama.anjum@collabora.com>
CC: linux-kernel@vger.kernel.org
CC: linux-m68k@lists.linux-m68k.org
CC: linuxppc-dev@lists.ozlabs.org
CC: linux-kselftest@vger.kernel.org
CC: Tamir Duberstein <tamird@gmail.com>

Hi Tamir,

kernel test robot noticed the following build errors:

[auto build test ERROR on 8f0b4cce4481fb22653697cced8d0d04027cb1e8]

url:    https://github.com/intel-lab-lkp/linux/commits/Tamir-Duberstein/test_bitmap-extract-benchmark-module/20251222-214306
base:   8f0b4cce4481fb22653697cced8d0d04027cb1e8
patch link:    https://lore.kernel.org/r/20251222-bitmap-kunit-convert-v2-3-6a61a5330eff%40gmail.com
patch subject: [PATCH v2 3/3] bitmap: break kunit into test cases
:::::: branch date: 26 hours ago
:::::: commit date: 26 hours ago
config: x86_64-randconfig-076-20251223 (https://download.01.org/0day-ci/archive/20251223/202512232357.nliFXSgy-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251223/202512232357.nliFXSgy-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/r/202512232357.nliFXSgy-lkp@intel.com/

All errors (new ones prefixed by >>):

>> lib/bitmap_kunit.c:1163:2: error: call to '__compiletime_assert_615' declared with 'error' attribute: BUILD_BUG_ON failed: !__builtin_constant_p(~var)
    1163 |         BUILD_BUG_ON(!__builtin_constant_p(~var));
         |         ^
   include/linux/build_bug.h:50:2: note: expanded from macro 'BUILD_BUG_ON'
      50 |         BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
         |         ^
   include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
      39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
         |                                     ^
   include/linux/compiler_types.h:630:2: note: expanded from macro 'compiletime_assert'
     630 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |         ^
   include/linux/compiler_types.h:618:2: note: expanded from macro '_compiletime_assert'
     618 |         __compiletime_assert(condition, msg, prefix, suffix)
         |         ^
   include/linux/compiler_types.h:611:4: note: expanded from macro '__compiletime_assert'
     611 |                         prefix ## suffix();                             \
         |                         ^
   <scratch space>:93:1: note: expanded from here
      93 | __compiletime_assert_615
         | ^
   1 error generated.


vim +1163 lib/bitmap_kunit.c

291f93ca339f5b lib/test_bitmap.c  Barry Song        2021-08-06  1112  
2356d198d2b4dd lib/test_bitmap.c  Yury Norov        2023-07-17  1113  /*
2356d198d2b4dd lib/test_bitmap.c  Yury Norov        2023-07-17  1114   * FIXME: Clang breaks compile-time evaluations when KASAN and GCOV are enabled.
2356d198d2b4dd lib/test_bitmap.c  Yury Norov        2023-07-17  1115   * To workaround it, GCOV is force-disabled in Makefile for this configuration.
2356d198d2b4dd lib/test_bitmap.c  Yury Norov        2023-07-17  1116   */
8ef3340af7139f lib/bitmap_kunit.c Tamir Duberstein  2025-12-22  1117  static void test_bitmap_const_eval(struct kunit *kunittest)
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1118  {
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1119  	DECLARE_BITMAP(bitmap, BITS_PER_LONG);
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1120  	unsigned long initvar = BIT(2);
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1121  	unsigned long bitopvar = 0;
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1122  	unsigned long var = 0;
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1123  	int res;
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1124  
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1125  	/*
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1126  	 * Compilers must be able to optimize all of those to compile-time
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1127  	 * constants on any supported optimization level (-O2, -Os) and any
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1128  	 * architecture. Otherwise, trigger a build bug.
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1129  	 * The whole function gets optimized out then, there's nothing to do
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1130  	 * in runtime.
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1131  	 */
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1132  
7adaf37f7f104a lib/test_bitmap.c  Alexander Lobakin 2024-03-27  1133  	/* Equals to `unsigned long bitmap[1] = { GENMASK(6, 5), }` */
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1134  	bitmap_clear(bitmap, 0, BITS_PER_LONG);
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1135  	if (!test_bit(7, bitmap))
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1136  		bitmap_set(bitmap, 5, 2);
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1137  
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1138  	/* Equals to `unsigned long bitopvar = BIT(20)` */
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1139  	__change_bit(31, &bitopvar);
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1140  	bitmap_shift_right(&bitopvar, &bitopvar, 11, BITS_PER_LONG);
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1141  
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1142  	/* Equals to `unsigned long var = BIT(25)` */
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1143  	var |= BIT(25);
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1144  	if (var & BIT(0))
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1145  		var ^= GENMASK(9, 6);
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1146  
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1147  	/* __const_hweight<32|64>(GENMASK(6, 5)) == 2 */
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1148  	res = bitmap_weight(bitmap, 20);
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1149  	BUILD_BUG_ON(!__builtin_constant_p(res));
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1150  	BUILD_BUG_ON(res != 2);
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1151  
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1152  	/* !(BIT(31) & BIT(18)) == 1 */
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1153  	res = !test_bit(18, &bitopvar);
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1154  	BUILD_BUG_ON(!__builtin_constant_p(res));
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1155  	BUILD_BUG_ON(!res);
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1156  
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1157  	/* BIT(2) & GENMASK(14, 8) == 0 */
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1158  	res = initvar & GENMASK(14, 8);
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1159  	BUILD_BUG_ON(!__builtin_constant_p(res));
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1160  	BUILD_BUG_ON(res);
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1161  
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1162  	/* ~BIT(25) */
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24 @1163  	BUILD_BUG_ON(!__builtin_constant_p(~var));
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1164  	BUILD_BUG_ON(~var != ~BIT(25));
7adaf37f7f104a lib/test_bitmap.c  Alexander Lobakin 2024-03-27  1165  
7adaf37f7f104a lib/test_bitmap.c  Alexander Lobakin 2024-03-27  1166  	/* ~BIT(25) | BIT(25) == ~0UL */
7adaf37f7f104a lib/test_bitmap.c  Alexander Lobakin 2024-03-27  1167  	bitmap_complement(&var, &var, BITS_PER_LONG);
7adaf37f7f104a lib/test_bitmap.c  Alexander Lobakin 2024-03-27  1168  	__assign_bit(25, &var, true);
7adaf37f7f104a lib/test_bitmap.c  Alexander Lobakin 2024-03-27  1169  
7adaf37f7f104a lib/test_bitmap.c  Alexander Lobakin 2024-03-27  1170  	/* !(~(~0UL)) == 1 */
7adaf37f7f104a lib/test_bitmap.c  Alexander Lobakin 2024-03-27  1171  	res = bitmap_full(&var, BITS_PER_LONG);
7adaf37f7f104a lib/test_bitmap.c  Alexander Lobakin 2024-03-27  1172  	BUILD_BUG_ON(!__builtin_constant_p(res));
7adaf37f7f104a lib/test_bitmap.c  Alexander Lobakin 2024-03-27  1173  	BUILD_BUG_ON(!res);
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1174  }
dc34d5036692c6 lib/test_bitmap.c  Alexander Lobakin 2022-06-24  1175  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-12-23 15:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-22 13:39 [PATCH v2 0/3] bitmap: convert self-test to KUnit Tamir Duberstein
2025-12-22 13:39 ` [PATCH v2 1/3] test_bitmap: extract benchmark module Tamir Duberstein
2025-12-22 13:39 ` [PATCH v2 2/3] bitmap: convert self-test to KUnit Tamir Duberstein
2025-12-22 13:39 ` [PATCH v2 3/3] bitmap: break kunit into test cases Tamir Duberstein
2025-12-23 13:22   ` kernel test robot
2025-12-23 14:18   ` kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2025-12-23 15:36 kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.