linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] ww_mutex: convert self-test to KUnit
       [not found] <20250210-ww_mutex-kunit-convert-v1-1-972f0201f71e@gmail.com>
@ 2025-02-12 11:53 ` Dan Carpenter
  2025-02-12 14:33   ` Tamir Duberstein
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2025-02-12 11:53 UTC (permalink / raw)
  To: oe-kbuild, Tamir Duberstein, David Gow, Peter Zijlstra,
	Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long, Andrew Morton,
	Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt
  Cc: lkp, oe-kbuild-all, Linux Memory Management List, linux-kernel,
	linux-kselftest, llvm, Tamir Duberstein

Hi Tamir,

kernel test robot noticed the following build warnings:

url:    https://github.com/intel-lab-lkp/linux/commits/Tamir-Duberstein/ww_mutex-convert-self-test-to-KUnit/20250211-000245
base:   a64dcfb451e254085a7daee5fe51bf22959d52d3
patch link:    https://lore.kernel.org/r/20250210-ww_mutex-kunit-convert-v1-1-972f0201f71e%40gmail.com
patch subject: [PATCH] ww_mutex: convert self-test to KUnit
config: i386-randconfig-141-20250212 (https://download.01.org/0day-ci/archive/20250212/202502121806.CS6r741y-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)

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>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202502121806.CS6r741y-lkp@intel.com/

smatch warnings:
kernel/locking/ww_mutex_kunit.c:238 test_abba_gen_params() warn: shift has higher precedence than mask
kernel/locking/ww_mutex_kunit.c:249 test_abba() warn: shift has higher precedence than mask

vim +238 kernel/locking/ww_mutex_kunit.c

70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  231  
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  232  static const void *test_abba_gen_params(const void *prev, char *desc)
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  233  {
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  234  	static unsigned int storage;
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  235  	const unsigned int *next = gen_range(&storage, 0b00, 0b11, prev);
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  236  
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  237  	if (next != NULL) {
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10 @238  		const bool trylock = *next & 0b01 >> 0;
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  239  		const bool resolve = *next & 0b10 >> 1;

The shifts here are weird...  A zero shift is strange but even the 1 shift
is odd.  The current code is equivalent to:

	const bool resolve = *next & (0b10 >> 1);

But changing it to:

	const bool resolve = (*next & 0b10) >> 1;

Doesn't make sense either...  Probably that makes less sense actually.
What are you trying to communicate with this code?

daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  240  
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  241  		snprintf(desc, KUNIT_PARAM_DESC_SIZE, "trylock=%d,resolve=%d", trylock, resolve);
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  242  	}
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  243  	return next;
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  244  }
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  245  
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  246  static void test_abba(struct kunit *test)
70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  247  {
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  248  	const unsigned int *param = test->param_value;
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10 @249  	const bool trylock = *param & 0b01 >> 0;
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  250  	const bool resolve = *param & 0b10 >> 1;

Same.

70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  251  	struct test_abba abba;
70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  252  	struct ww_acquire_ctx ctx;
daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  253  	int err;
70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  254  
70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  255  	ww_mutex_init(&abba.a_mutex, &ww_class);
70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  256  	ww_mutex_init(&abba.b_mutex, &ww_class);
70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  257  	INIT_WORK_ONSTACK(&abba.work, test_abba_work);

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



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

* Re: [PATCH] ww_mutex: convert self-test to KUnit
  2025-02-12 11:53 ` [PATCH] ww_mutex: convert self-test to KUnit Dan Carpenter
@ 2025-02-12 14:33   ` Tamir Duberstein
  2025-02-12 15:31     ` Dan Carpenter
  0 siblings, 1 reply; 3+ messages in thread
From: Tamir Duberstein @ 2025-02-12 14:33 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, David Gow, Peter Zijlstra, Ingo Molnar, Will Deacon,
	Boqun Feng, Waiman Long, Andrew Morton, Shuah Khan,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	lkp, oe-kbuild-all, Linux Memory Management List, linux-kernel,
	linux-kselftest, llvm

Hi Dan,

On Wed, Feb 12, 2025 at 6:53 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> Hi Tamir,
>
> kernel test robot noticed the following build warnings:
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Tamir-Duberstein/ww_mutex-convert-self-test-to-KUnit/20250211-000245
> base:   a64dcfb451e254085a7daee5fe51bf22959d52d3
> patch link:    https://lore.kernel.org/r/20250210-ww_mutex-kunit-convert-v1-1-972f0201f71e%40gmail.com
> patch subject: [PATCH] ww_mutex: convert self-test to KUnit
> config: i386-randconfig-141-20250212 (https://download.01.org/0day-ci/archive/20250212/202502121806.CS6r741y-lkp@intel.com/config)
> compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
>
> 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>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202502121806.CS6r741y-lkp@intel.com/
>
> smatch warnings:
> kernel/locking/ww_mutex_kunit.c:238 test_abba_gen_params() warn: shift has higher precedence than mask
> kernel/locking/ww_mutex_kunit.c:249 test_abba() warn: shift has higher precedence than mask
>
> vim +238 kernel/locking/ww_mutex_kunit.c
>
> 70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  231
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  232  static const void *test_abba_gen_params(const void *prev, char *desc)
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  233  {
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  234        static unsigned int storage;
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  235        const unsigned int *next = gen_range(&storage, 0b00, 0b11, prev);
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  236
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  237        if (next != NULL) {
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10 @238                const bool trylock = *next & 0b01 >> 0;
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  239                const bool resolve = *next & 0b10 >> 1;
>
> The shifts here are weird...  A zero shift is strange but even the 1 shift
> is odd.  The current code is equivalent to:
>
>         const bool resolve = *next & (0b10 >> 1);
>
> But changing it to:
>
>         const bool resolve = (*next & 0b10) >> 1;
>
> Doesn't make sense either...  Probably that makes less sense actually.
> What are you trying to communicate with this code?

Yeah, the bit shifting here is not necessary. I'll replace this with a
proper bitfield.

> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  240
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  241                snprintf(desc, KUNIT_PARAM_DESC_SIZE, "trylock=%d,resolve=%d", trylock, resolve);
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  242        }
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  243        return next;
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  244  }
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  245
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  246  static void test_abba(struct kunit *test)
> 70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  247  {
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  248        const unsigned int *param = test->param_value;
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10 @249        const bool trylock = *param & 0b01 >> 0;
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  250        const bool resolve = *param & 0b10 >> 1;
>
> Same.
>
> 70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  251        struct test_abba abba;
> 70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  252        struct ww_acquire_ctx ctx;
> daf92a37bd1117 kernel/locking/ww_mutex_kunit.c Tamir Duberstein  2025-02-10  253        int err;
> 70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  254
> 70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  255        ww_mutex_init(&abba.a_mutex, &ww_class);
> 70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  256        ww_mutex_init(&abba.b_mutex, &ww_class);
> 70207686e492fb kernel/locking/test-ww_mutex.c  Chris Wilson      2016-12-01  257        INIT_WORK_ONSTACK(&abba.work, test_abba_work);
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
>

As an aside, how can I compile with the warning settings used by
kernel test robot?

Thanks.
Tamir


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

* Re: [PATCH] ww_mutex: convert self-test to KUnit
  2025-02-12 14:33   ` Tamir Duberstein
@ 2025-02-12 15:31     ` Dan Carpenter
  0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2025-02-12 15:31 UTC (permalink / raw)
  To: Tamir Duberstein
  Cc: oe-kbuild, David Gow, Peter Zijlstra, Ingo Molnar, Will Deacon,
	Boqun Feng, Waiman Long, Andrew Morton, Shuah Khan,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	lkp, oe-kbuild-all, Linux Memory Management List, linux-kernel,
	linux-kselftest, llvm

On Wed, Feb 12, 2025 at 09:33:46AM -0500, Tamir Duberstein wrote:
> >
> 
> As an aside, how can I compile with the warning settings used by
> kernel test robot?
> 

This is a Smatch warning.

https://github.com/error27/smatch
https://github.com/error27/smatch/blob/master/Documentation/smatch.rst

Run smatch/smatch_scripts/kchecker kernel/locking/ww_mutex_kunit.c

regards,
dan carpenter



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

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

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20250210-ww_mutex-kunit-convert-v1-1-972f0201f71e@gmail.com>
2025-02-12 11:53 ` [PATCH] ww_mutex: convert self-test to KUnit Dan Carpenter
2025-02-12 14:33   ` Tamir Duberstein
2025-02-12 15:31     ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox