public inbox for [email protected]
 help / color / mirror / Atom feed
* [ammarfaizi2-block:paulmck/linux-rcu/dev.2023.01.19a 55/61] kernel/rcu/rcutorture.c:3548 rcu_torture_init_srcu_lockdep() warn: for statement not indented
@ 2023-01-24  9:32 Dan Carpenter
  2023-01-24 10:05 ` Dan Carpenter
  2023-01-24 14:51 ` Paul E. McKenney
  0 siblings, 2 replies; 7+ messages in thread
From: Dan Carpenter @ 2023-01-24  9:32 UTC (permalink / raw)
  To: oe-kbuild, Paul E. McKenney
  Cc: lkp, oe-kbuild-all, Ammar Faizi, GNU/Weeb Mailing List

tree:   https://github.com/ammarfaizi2/linux-block paulmck/linux-rcu/dev.2023.01.19a
head:   e51eadce38349e7edb506a61e325cc562d49409f
commit: a5ae869a4ab737494a3c5c167d782b8eab3ba724 [55/61] squash! rcutorture: Add SRCU deadlock scenarios
config: x86_64-randconfig-m001-20230123 (https://download.01.org/0day-ci/archive/20230124/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>

smatch warnings:
kernel/rcu/rcutorture.c:3548 rcu_torture_init_srcu_lockdep() warn: for statement not indented

vim +3548 kernel/rcu/rcutorture.c

81fba7e62c82be Paul E. McKenney 2023-01-13  3514  static void rcu_torture_init_srcu_lockdep(void)
81fba7e62c82be Paul E. McKenney 2023-01-13  3515  {
81fba7e62c82be Paul E. McKenney 2023-01-13  3516  	int cyclelen;
81fba7e62c82be Paul E. McKenney 2023-01-13  3517  	int cyclelenmax;
81fba7e62c82be Paul E. McKenney 2023-01-13  3518  	int deadlock;
81fba7e62c82be Paul E. McKenney 2023-01-13  3519  	bool err = false;
81fba7e62c82be Paul E. McKenney 2023-01-13  3520  	int i;
81fba7e62c82be Paul E. McKenney 2023-01-13  3521  	int j;
81fba7e62c82be Paul E. McKenney 2023-01-13  3522  	int idx;
a5ae869a4ab737 Paul E. McKenney 2023-01-23  3523  	struct mutex *muts[] = { &mut0, &mut1, &mut2, &mut3, &mut4,
a5ae869a4ab737 Paul E. McKenney 2023-01-23  3524  				 &mut5, &mut6, &mut7, &mut8, &mut9 };
a5ae869a4ab737 Paul E. McKenney 2023-01-23  3525  	struct srcu_struct *srcus[] = { &srcu0, &srcu1, &srcu2, &srcu3, &srcu4,
a5ae869a4ab737 Paul E. McKenney 2023-01-23  3526  					&srcu5, &srcu6, &srcu7, &srcu8, &srcu9 };
81fba7e62c82be Paul E. McKenney 2023-01-13  3527  	int testtype;
81fba7e62c82be Paul E. McKenney 2023-01-13  3528  
81fba7e62c82be Paul E. McKenney 2023-01-13  3529  	if (!test_srcu_lockdep)
81fba7e62c82be Paul E. McKenney 2023-01-13  3530  		return;
81fba7e62c82be Paul E. McKenney 2023-01-13  3531  
a5ae869a4ab737 Paul E. McKenney 2023-01-23  3532  	deadlock = test_srcu_lockdep / 1000;
81fba7e62c82be Paul E. McKenney 2023-01-13  3533  	testtype = (test_srcu_lockdep / 10) % 100;
81fba7e62c82be Paul E. McKenney 2023-01-13  3534  	cyclelen = test_srcu_lockdep % 10;
81fba7e62c82be Paul E. McKenney 2023-01-13  3535  	cyclelenmax = min(ARRAY_SIZE(muts), ARRAY_SIZE(srcus));
81fba7e62c82be Paul E. McKenney 2023-01-13  3536  	WARN_ON_ONCE(ARRAY_SIZE(muts) != ARRAY_SIZE(srcus));
81fba7e62c82be Paul E. McKenney 2023-01-13  3537  	if (WARN_ONCE(deadlock != !!deadlock,
81fba7e62c82be Paul E. McKenney 2023-01-13  3538  		      "%s: test_srcu_lockdep=%d and deadlock digit %d must be zero or one.\n",
81fba7e62c82be Paul E. McKenney 2023-01-13  3539  		      __func__, test_srcu_lockdep, deadlock))
81fba7e62c82be Paul E. McKenney 2023-01-13  3540  		err = true;
81fba7e62c82be Paul E. McKenney 2023-01-13  3541  	if (WARN_ONCE(cyclelen <= 0,
81fba7e62c82be Paul E. McKenney 2023-01-13  3542  		      "%s: test_srcu_lockdep=%d and cycle-length digit %d must be greater than zero.\n",
81fba7e62c82be Paul E. McKenney 2023-01-13  3543  		      __func__, test_srcu_lockdep, cyclelen))
81fba7e62c82be Paul E. McKenney 2023-01-13  3544  		err = true;
81fba7e62c82be Paul E. McKenney 2023-01-13  3545  	if (err)
81fba7e62c82be Paul E. McKenney 2023-01-13  3546  		goto err_out;
81fba7e62c82be Paul E. McKenney 2023-01-13  3547  
a5ae869a4ab737 Paul E. McKenney 2023-01-23 @3548  	for (i = 0; i < cyclelen; i++)

This line looks like it should just be deleted.  The real loops are
later so i == cyclelen by the time it loops back here.  Should probably
create a static checker warning for nested loops where the inside loop
resets the outside iterator to zero.

a5ae869a4ab737 Paul E. McKenney 2023-01-23  3549  
81fba7e62c82be Paul E. McKenney 2023-01-13  3550  	if (testtype == 0) {
81fba7e62c82be Paul E. McKenney 2023-01-13  3551  		pr_info("%s: test_srcu_lockdep = %05d: SRCU %d-way %sdeadlock.\n",
81fba7e62c82be Paul E. McKenney 2023-01-13  3552  			__func__, test_srcu_lockdep, cyclelen, deadlock ? "" : "non-");
81fba7e62c82be Paul E. McKenney 2023-01-13  3553  		if (deadlock && cyclelen == 1)
81fba7e62c82be Paul E. McKenney 2023-01-13  3554  			pr_info("%s: Expect hang.\n", __func__);
81fba7e62c82be Paul E. McKenney 2023-01-13  3555  		for (i = 0; i < cyclelen; i++) {
81fba7e62c82be Paul E. McKenney 2023-01-13  3556  			j = i + 1;
81fba7e62c82be Paul E. McKenney 2023-01-13  3557  			if (i >= cyclelen - 1)
81fba7e62c82be Paul E. McKenney 2023-01-13  3558  				j = deadlock ? 0 : -1;
81fba7e62c82be Paul E. McKenney 2023-01-13  3559  
81fba7e62c82be Paul E. McKenney 2023-01-13  3560  			if (j >= 0)
81fba7e62c82be Paul E. McKenney 2023-01-13  3561  				pr_info("%s: srcu_read_lock(%d), synchronize_srcu(%d), srcu_read_unlock(%d)\n",
81fba7e62c82be Paul E. McKenney 2023-01-13  3562  					__func__, i, j, i);
81fba7e62c82be Paul E. McKenney 2023-01-13  3563  			else
81fba7e62c82be Paul E. McKenney 2023-01-13  3564  				pr_info("%s: srcu_read_lock(%d), srcu_read_unlock(%d)\n",
81fba7e62c82be Paul E. McKenney 2023-01-13  3565  					__func__, i, i);
81fba7e62c82be Paul E. McKenney 2023-01-13  3566  			idx = srcu_read_lock(srcus[i]);
81fba7e62c82be Paul E. McKenney 2023-01-13  3567  			if (j >= 0)
81fba7e62c82be Paul E. McKenney 2023-01-13  3568  				synchronize_srcu(srcus[j]);
81fba7e62c82be Paul E. McKenney 2023-01-13  3569  			srcu_read_unlock(srcus[i], idx);
81fba7e62c82be Paul E. McKenney 2023-01-13  3570  		}
81fba7e62c82be Paul E. McKenney 2023-01-13  3571  		return;
81fba7e62c82be Paul E. McKenney 2023-01-13  3572  	}
81fba7e62c82be Paul E. McKenney 2023-01-13  3573  
81fba7e62c82be Paul E. McKenney 2023-01-13  3574  	if (testtype == 1) {
81fba7e62c82be Paul E. McKenney 2023-01-13  3575  		pr_info("%s: test_srcu_lockdep = %05d: SRCU/mutex %d-way %sdeadlock.\n",
81fba7e62c82be Paul E. McKenney 2023-01-13  3576  			__func__, test_srcu_lockdep, cyclelen, deadlock ? "" : "non-");
81fba7e62c82be Paul E. McKenney 2023-01-13  3577  		for (i = 0; i < cyclelen; i++) {
81fba7e62c82be Paul E. McKenney 2023-01-13  3578  			j = i + 1;
81fba7e62c82be Paul E. McKenney 2023-01-13  3579  			if (i >= cyclelen - 1)
81fba7e62c82be Paul E. McKenney 2023-01-13  3580  				j = deadlock ? 0 : -1;
81fba7e62c82be Paul E. McKenney 2023-01-13  3581  
81fba7e62c82be Paul E. McKenney 2023-01-13  3582  			pr_info("%s: srcu_read_lock(%d), mutex_lock(%d), mutex_unlock(%d), srcu_read_unlock(%d)\n",
81fba7e62c82be Paul E. McKenney 2023-01-13  3583  				__func__, i, i, i, i);
81fba7e62c82be Paul E. McKenney 2023-01-13  3584  			idx = srcu_read_lock(srcus[i]);
81fba7e62c82be Paul E. McKenney 2023-01-13  3585  			mutex_lock(muts[i]);
81fba7e62c82be Paul E. McKenney 2023-01-13  3586  			mutex_unlock(muts[i]);
81fba7e62c82be Paul E. McKenney 2023-01-13  3587  			srcu_read_unlock(srcus[i], idx);
81fba7e62c82be Paul E. McKenney 2023-01-13  3588  
81fba7e62c82be Paul E. McKenney 2023-01-13  3589  			if (j >= 0) {
81fba7e62c82be Paul E. McKenney 2023-01-13  3590  				pr_info("%s: mutex_lock(%d), synchronize_srcu(%d), mutex_unlock(%d)\n",
81fba7e62c82be Paul E. McKenney 2023-01-13  3591  					__func__, i, j, i);
81fba7e62c82be Paul E. McKenney 2023-01-13  3592  				mutex_lock(muts[i]);
81fba7e62c82be Paul E. McKenney 2023-01-13  3593  				synchronize_srcu(srcus[j]);
81fba7e62c82be Paul E. McKenney 2023-01-13  3594  				mutex_unlock(muts[i]);
81fba7e62c82be Paul E. McKenney 2023-01-13  3595  			}
81fba7e62c82be Paul E. McKenney 2023-01-13  3596  		}
81fba7e62c82be Paul E. McKenney 2023-01-13  3597  		return;
81fba7e62c82be Paul E. McKenney 2023-01-13  3598  	}
81fba7e62c82be Paul E. McKenney 2023-01-13  3599  
81fba7e62c82be Paul E. McKenney 2023-01-13  3600  err_out:
81fba7e62c82be Paul E. McKenney 2023-01-13  3601  	pr_info("%s: test_srcu_lockdep = %05d does nothing.\n", __func__, test_srcu_lockdep);
a5ae869a4ab737 Paul E. McKenney 2023-01-23  3602  	pr_info("%s: test_srcu_lockdep = DTTC.\n", __func__);
81fba7e62c82be Paul E. McKenney 2023-01-13  3603  	pr_info("%s: D: Deadlock if nonzero.\n", __func__);
81fba7e62c82be Paul E. McKenney 2023-01-13  3604  	pr_info("%s: TT: Test number, 0=SRCU, 1=SRCU/mutex.\n", __func__);
81fba7e62c82be Paul E. McKenney 2023-01-13  3605  	pr_info("%s: C: Cycle length.\n", __func__);
81fba7e62c82be Paul E. McKenney 2023-01-13  3606  }

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


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

* Re: [ammarfaizi2-block:paulmck/linux-rcu/dev.2023.01.19a 55/61] kernel/rcu/rcutorture.c:3548 rcu_torture_init_srcu_lockdep() warn: for statement not indented
  2023-01-24  9:32 [ammarfaizi2-block:paulmck/linux-rcu/dev.2023.01.19a 55/61] kernel/rcu/rcutorture.c:3548 rcu_torture_init_srcu_lockdep() warn: for statement not indented Dan Carpenter
@ 2023-01-24 10:05 ` Dan Carpenter
  2023-01-25  8:10   ` Dan Carpenter
  2023-01-24 14:51 ` Paul E. McKenney
  1 sibling, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2023-01-24 10:05 UTC (permalink / raw)
  To: oe-kbuild, Paul E. McKenney
  Cc: lkp, oe-kbuild-all, Ammar Faizi, GNU/Weeb Mailing List

On Tue, Jan 24, 2023 at 12:32:14PM +0300, Dan Carpenter wrote:
> Should probably
> create a static checker warning for nested loops where the inside loop
> resets the outside iterator to zero.

Done!  I'll test that out tonight.

kernel/rcu/rcutorture.c:3582 rcu_torture_init_srcu_lockdep() warn: re-using outside iterator: 'i'

regards,
dan carpenter


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

* Re: [ammarfaizi2-block:paulmck/linux-rcu/dev.2023.01.19a 55/61] kernel/rcu/rcutorture.c:3548 rcu_torture_init_srcu_lockdep() warn: for statement not indented
  2023-01-24  9:32 [ammarfaizi2-block:paulmck/linux-rcu/dev.2023.01.19a 55/61] kernel/rcu/rcutorture.c:3548 rcu_torture_init_srcu_lockdep() warn: for statement not indented Dan Carpenter
  2023-01-24 10:05 ` Dan Carpenter
@ 2023-01-24 14:51 ` Paul E. McKenney
  1 sibling, 0 replies; 7+ messages in thread
From: Paul E. McKenney @ 2023-01-24 14:51 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, lkp, oe-kbuild-all, Ammar Faizi, GNU/Weeb Mailing List

On Tue, Jan 24, 2023 at 12:32:14PM +0300, Dan Carpenter wrote:
> tree:   https://github.com/ammarfaizi2/linux-block paulmck/linux-rcu/dev.2023.01.19a
> head:   e51eadce38349e7edb506a61e325cc562d49409f
> commit: a5ae869a4ab737494a3c5c167d782b8eab3ba724 [55/61] squash! rcutorture: Add SRCU deadlock scenarios
> config: x86_64-randconfig-m001-20230123 (https://download.01.org/0day-ci/archive/20230124/[email protected]/config)
> compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
> 
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <[email protected]>
> | Reported-by: Dan Carpenter <[email protected]>
> 
> smatch warnings:
> kernel/rcu/rcutorture.c:3548 rcu_torture_init_srcu_lockdep() warn: for statement not indented
> 
> vim +3548 kernel/rcu/rcutorture.c
> 
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3514  static void rcu_torture_init_srcu_lockdep(void)
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3515  {
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3516  	int cyclelen;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3517  	int cyclelenmax;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3518  	int deadlock;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3519  	bool err = false;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3520  	int i;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3521  	int j;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3522  	int idx;
> a5ae869a4ab737 Paul E. McKenney 2023-01-23  3523  	struct mutex *muts[] = { &mut0, &mut1, &mut2, &mut3, &mut4,
> a5ae869a4ab737 Paul E. McKenney 2023-01-23  3524  				 &mut5, &mut6, &mut7, &mut8, &mut9 };
> a5ae869a4ab737 Paul E. McKenney 2023-01-23  3525  	struct srcu_struct *srcus[] = { &srcu0, &srcu1, &srcu2, &srcu3, &srcu4,
> a5ae869a4ab737 Paul E. McKenney 2023-01-23  3526  					&srcu5, &srcu6, &srcu7, &srcu8, &srcu9 };
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3527  	int testtype;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3528  
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3529  	if (!test_srcu_lockdep)
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3530  		return;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3531  
> a5ae869a4ab737 Paul E. McKenney 2023-01-23  3532  	deadlock = test_srcu_lockdep / 1000;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3533  	testtype = (test_srcu_lockdep / 10) % 100;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3534  	cyclelen = test_srcu_lockdep % 10;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3535  	cyclelenmax = min(ARRAY_SIZE(muts), ARRAY_SIZE(srcus));
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3536  	WARN_ON_ONCE(ARRAY_SIZE(muts) != ARRAY_SIZE(srcus));
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3537  	if (WARN_ONCE(deadlock != !!deadlock,
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3538  		      "%s: test_srcu_lockdep=%d and deadlock digit %d must be zero or one.\n",
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3539  		      __func__, test_srcu_lockdep, deadlock))
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3540  		err = true;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3541  	if (WARN_ONCE(cyclelen <= 0,
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3542  		      "%s: test_srcu_lockdep=%d and cycle-length digit %d must be greater than zero.\n",
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3543  		      __func__, test_srcu_lockdep, cyclelen))
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3544  		err = true;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3545  	if (err)
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3546  		goto err_out;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3547  
> a5ae869a4ab737 Paul E. McKenney 2023-01-23 @3548  	for (i = 0; i < cyclelen; i++)
> 
> This line looks like it should just be deleted.  The real loops are
> later so i == cyclelen by the time it loops back here.  Should probably
> create a static checker warning for nested loops where the inside loop
> resets the outside iterator to zero.

You are quite right, I intended to have deleted that statement.

And the fact that it works is quite coincidental.  Thank you!

							Thanx, Paul

> a5ae869a4ab737 Paul E. McKenney 2023-01-23  3549  
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3550  	if (testtype == 0) {
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3551  		pr_info("%s: test_srcu_lockdep = %05d: SRCU %d-way %sdeadlock.\n",
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3552  			__func__, test_srcu_lockdep, cyclelen, deadlock ? "" : "non-");
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3553  		if (deadlock && cyclelen == 1)
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3554  			pr_info("%s: Expect hang.\n", __func__);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3555  		for (i = 0; i < cyclelen; i++) {
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3556  			j = i + 1;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3557  			if (i >= cyclelen - 1)
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3558  				j = deadlock ? 0 : -1;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3559  
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3560  			if (j >= 0)
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3561  				pr_info("%s: srcu_read_lock(%d), synchronize_srcu(%d), srcu_read_unlock(%d)\n",
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3562  					__func__, i, j, i);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3563  			else
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3564  				pr_info("%s: srcu_read_lock(%d), srcu_read_unlock(%d)\n",
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3565  					__func__, i, i);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3566  			idx = srcu_read_lock(srcus[i]);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3567  			if (j >= 0)
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3568  				synchronize_srcu(srcus[j]);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3569  			srcu_read_unlock(srcus[i], idx);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3570  		}
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3571  		return;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3572  	}
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3573  
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3574  	if (testtype == 1) {
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3575  		pr_info("%s: test_srcu_lockdep = %05d: SRCU/mutex %d-way %sdeadlock.\n",
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3576  			__func__, test_srcu_lockdep, cyclelen, deadlock ? "" : "non-");
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3577  		for (i = 0; i < cyclelen; i++) {
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3578  			j = i + 1;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3579  			if (i >= cyclelen - 1)
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3580  				j = deadlock ? 0 : -1;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3581  
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3582  			pr_info("%s: srcu_read_lock(%d), mutex_lock(%d), mutex_unlock(%d), srcu_read_unlock(%d)\n",
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3583  				__func__, i, i, i, i);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3584  			idx = srcu_read_lock(srcus[i]);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3585  			mutex_lock(muts[i]);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3586  			mutex_unlock(muts[i]);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3587  			srcu_read_unlock(srcus[i], idx);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3588  
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3589  			if (j >= 0) {
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3590  				pr_info("%s: mutex_lock(%d), synchronize_srcu(%d), mutex_unlock(%d)\n",
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3591  					__func__, i, j, i);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3592  				mutex_lock(muts[i]);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3593  				synchronize_srcu(srcus[j]);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3594  				mutex_unlock(muts[i]);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3595  			}
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3596  		}
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3597  		return;
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3598  	}
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3599  
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3600  err_out:
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3601  	pr_info("%s: test_srcu_lockdep = %05d does nothing.\n", __func__, test_srcu_lockdep);
> a5ae869a4ab737 Paul E. McKenney 2023-01-23  3602  	pr_info("%s: test_srcu_lockdep = DTTC.\n", __func__);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3603  	pr_info("%s: D: Deadlock if nonzero.\n", __func__);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3604  	pr_info("%s: TT: Test number, 0=SRCU, 1=SRCU/mutex.\n", __func__);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3605  	pr_info("%s: C: Cycle length.\n", __func__);
> 81fba7e62c82be Paul E. McKenney 2023-01-13  3606  }
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests
> 

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

* Re: [ammarfaizi2-block:paulmck/linux-rcu/dev.2023.01.19a 55/61] kernel/rcu/rcutorture.c:3548 rcu_torture_init_srcu_lockdep() warn: for statement not indented
  2023-01-24 10:05 ` Dan Carpenter
@ 2023-01-25  8:10   ` Dan Carpenter
  2023-01-25  8:32     ` Dan Carpenter
  0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2023-01-25  8:10 UTC (permalink / raw)
  To: oe-kbuild, Paul E. McKenney
  Cc: lkp, oe-kbuild-all, Ammar Faizi, GNU/Weeb Mailing List

On Tue, Jan 24, 2023 at 01:05:02PM +0300, Dan Carpenter wrote:
> On Tue, Jan 24, 2023 at 12:32:14PM +0300, Dan Carpenter wrote:
> > Should probably
> > create a static checker warning for nested loops where the inside loop
> > resets the outside iterator to zero.
> 
> Done!  I'll test that out tonight.
> 
> kernel/rcu/rcutorture.c:3582 rcu_torture_init_srcu_lockdep() warn: re-using outside iterator: 'i'

This check is delightful.  :)  I'm going to send patches for these.

drivers/gpu/drm/msm/adreno/a5xx_gpu.c:95 a5xx_submit_in_rb() warn: re-using outside iterator: 'i'
drivers/gpu/drm/vc4/vc4_kms.c:561 vc4_ctm_atomic_check() warn: re-using outside iterator: 'i'
drivers/net/ethernet/cavium/liquidio/lio_ethtool.c:2755 cn23xx_read_csr_reg() warn: re-using outside iterator: 'i'
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:936 mvpp2_bm_pool_update_priv_fc() warn: re-using outside iterator: 'i'
drivers/net/wireless/marvell/mwifiex/11n.c:912 mwifiex_update_ampdu_txwinsize() warn: re-using outside iterator: 'i'
drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c:870 mwifiex_update_ampdu_rxwinsize() warn: re-using outside iterator: 'i'
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c:1265 rtl8188fu_phy_iq_calibrate() warn: re-using outside iterator: 'i'
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c:1168 rtl8723bu_phy_iq_calibrate() warn: re-using outside iterator: 'i'
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:3298 rtl8xxxu_gen1_phy_iq_calibrate() warn: re-using outside iterator: 'i'
drivers/net/wireless/realtek/rtlwifi/rtl8188ee/phy.c:1989 rtl88e_phy_iq_calibrate() warn: re-using outside iterator: 'i'
drivers/net/wireless/realtek/rtlwifi/rtl8192c/phy_common.c:1429 rtl92c_phy_iq_calibrate() warn: re-using outside iterator: 'i'
drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c:2294 rtl92d_phy_iq_calibrate() warn: re-using outside iterator: 'i'
drivers/net/wireless/realtek/rtlwifi/rtl8723ae/phy.c:1393 rtl8723e_phy_iq_calibrate() warn: re-using outside iterator: 'i'
drivers/net/wireless/realtek/rtlwifi/rtl8723be/phy.c:2321 rtl8723be_phy_iq_calibrate() warn: re-using outside iterator: 'i'
drivers/staging/media/atomisp/pci/sh_css.c:8538 ia_css_stream_destroy() warn: re-using outside iterator: 'i'
drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c:1717 PHY_IQCalibrate_8723B() warn: re-using outside iterator: 'i'
sound/pci/lx6464es/lx_core.c:497 lx_buffer_ask() warn: re-using outside iterator: 'i'

regards,
dan carpenter


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

* Re: [ammarfaizi2-block:paulmck/linux-rcu/dev.2023.01.19a 55/61] kernel/rcu/rcutorture.c:3548 rcu_torture_init_srcu_lockdep() warn: for statement not indented
  2023-01-25  8:10   ` Dan Carpenter
@ 2023-01-25  8:32     ` Dan Carpenter
  2023-01-25  8:45       ` Ammar Faizi
  2023-01-25 14:56       ` Paul E. McKenney
  0 siblings, 2 replies; 7+ messages in thread
From: Dan Carpenter @ 2023-01-25  8:32 UTC (permalink / raw)
  To: oe-kbuild, Paul E. McKenney
  Cc: lkp, oe-kbuild-all, Ammar Faizi, GNU/Weeb Mailing List

On Wed, Jan 25, 2023 at 11:10:58AM +0300, Dan Carpenter wrote:
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c:1265 rtl8188fu_phy_iq_calibrate() warn: re-using outside iterator: 'i'
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c:1168 rtl8723bu_phy_iq_calibrate() warn: re-using outside iterator: 'i'
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:3298 rtl8xxxu_gen1_phy_iq_calibrate() warn: re-using outside iterator: 'i'
> drivers/net/wireless/realtek/rtlwifi/rtl8188ee/phy.c:1989 rtl88e_phy_iq_calibrate() warn: re-using outside iterator: 'i'
> drivers/net/wireless/realtek/rtlwifi/rtl8192c/phy_common.c:1429 rtl92c_phy_iq_calibrate() warn: re-using outside iterator: 'i'
> drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c:2294 rtl92d_phy_iq_calibrate() warn: re-using outside iterator: 'i'
> drivers/net/wireless/realtek/rtlwifi/rtl8723ae/phy.c:1393 rtl8723e_phy_iq_calibrate() warn: re-using outside iterator: 'i'
> drivers/net/wireless/realtek/rtlwifi/rtl8723be/phy.c:2321 rtl8723be_phy_iq_calibrate() warn: re-using outside iterator: 'i'

These are all "false positives" (kind of).  It's 8 copies of code which
does:

	for (i = 0; i < 3; i++) {
		...
		if (i == 2) {
			for (i = 0; i < 8; i++)
				foo = bar[i];
		}
	}

So the loop exits with i == 8 instead of i == 3 but that doesn't matter.
Ugh...  Did the original author do this deliberately or did it just work
by chance?  I really prefer buggy code to code to ugly code.  With buggy
code it's a technical matter and easy to fix, but with ugly code that's
like a big debate and a social problem...

On the other hand, in this case, I don't think anyone is going to defend
the original style from these drivers so I may as well patch it.

regards,
dan carpenter

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

* Re: [ammarfaizi2-block:paulmck/linux-rcu/dev.2023.01.19a 55/61] kernel/rcu/rcutorture.c:3548 rcu_torture_init_srcu_lockdep() warn: for statement not indented
  2023-01-25  8:32     ` Dan Carpenter
@ 2023-01-25  8:45       ` Ammar Faizi
  2023-01-25 14:56       ` Paul E. McKenney
  1 sibling, 0 replies; 7+ messages in thread
From: Ammar Faizi @ 2023-01-25  8:45 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, Paul E. McKenney, lkp, oe-kbuild-all,
	GNU/Weeb Mailing List

On Wed, Jan 25, 2023 at 11:32:21AM +0300, Dan Carpenter wrote:
> These are all "false positives" (kind of).  It's 8 copies of code which
> does:
> 
> 	for (i = 0; i < 3; i++) {
> 		...
> 		if (i == 2) {
> 			for (i = 0; i < 8; i++)
> 				foo = bar[i];
> 		}
> 	}
> 
> So the loop exits with i == 8 instead of i == 3 but that doesn't matter.
> Ugh...  Did the original author do this deliberately or did it just work
> by chance?  I really prefer buggy code to code to ugly code.  With buggy
> code it's a technical matter and easy to fix, but with ugly code that's
> like a big debate and a social problem...
> 
> On the other hand, in this case, I don't think anyone is going to defend
> the original style from these drivers so I may as well patch it.

Yeah, that pattern doesn't look like intentional. I agree with you.

-- 
Ammar Faizi


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

* Re: [ammarfaizi2-block:paulmck/linux-rcu/dev.2023.01.19a 55/61] kernel/rcu/rcutorture.c:3548 rcu_torture_init_srcu_lockdep() warn: for statement not indented
  2023-01-25  8:32     ` Dan Carpenter
  2023-01-25  8:45       ` Ammar Faizi
@ 2023-01-25 14:56       ` Paul E. McKenney
  1 sibling, 0 replies; 7+ messages in thread
From: Paul E. McKenney @ 2023-01-25 14:56 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, lkp, oe-kbuild-all, Ammar Faizi, GNU/Weeb Mailing List

On Wed, Jan 25, 2023 at 11:32:21AM +0300, Dan Carpenter wrote:
> On Wed, Jan 25, 2023 at 11:10:58AM +0300, Dan Carpenter wrote:
> > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c:1265 rtl8188fu_phy_iq_calibrate() warn: re-using outside iterator: 'i'
> > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c:1168 rtl8723bu_phy_iq_calibrate() warn: re-using outside iterator: 'i'
> > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:3298 rtl8xxxu_gen1_phy_iq_calibrate() warn: re-using outside iterator: 'i'
> > drivers/net/wireless/realtek/rtlwifi/rtl8188ee/phy.c:1989 rtl88e_phy_iq_calibrate() warn: re-using outside iterator: 'i'
> > drivers/net/wireless/realtek/rtlwifi/rtl8192c/phy_common.c:1429 rtl92c_phy_iq_calibrate() warn: re-using outside iterator: 'i'
> > drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c:2294 rtl92d_phy_iq_calibrate() warn: re-using outside iterator: 'i'
> > drivers/net/wireless/realtek/rtlwifi/rtl8723ae/phy.c:1393 rtl8723e_phy_iq_calibrate() warn: re-using outside iterator: 'i'
> > drivers/net/wireless/realtek/rtlwifi/rtl8723be/phy.c:2321 rtl8723be_phy_iq_calibrate() warn: re-using outside iterator: 'i'
> 
> These are all "false positives" (kind of).  It's 8 copies of code which
> does:
> 
> 	for (i = 0; i < 3; i++) {
> 		...
> 		if (i == 2) {
> 			for (i = 0; i < 8; i++)
> 				foo = bar[i];
> 		}
> 	}
> 
> So the loop exits with i == 8 instead of i == 3 but that doesn't matter.
> Ugh...  Did the original author do this deliberately or did it just work
> by chance?  I really prefer buggy code to code to ugly code.  With buggy
> code it's a technical matter and easy to fix, but with ugly code that's
> like a big debate and a social problem...
> 
> On the other hand, in this case, I don't think anyone is going to defend
> the original style from these drivers so I may as well patch it.

Looks suspicious to me!  ;-)

							Thanx, Paul

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

end of thread, other threads:[~2023-01-25 14:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-24  9:32 [ammarfaizi2-block:paulmck/linux-rcu/dev.2023.01.19a 55/61] kernel/rcu/rcutorture.c:3548 rcu_torture_init_srcu_lockdep() warn: for statement not indented Dan Carpenter
2023-01-24 10:05 ` Dan Carpenter
2023-01-25  8:10   ` Dan Carpenter
2023-01-25  8:32     ` Dan Carpenter
2023-01-25  8:45       ` Ammar Faizi
2023-01-25 14:56       ` Paul E. McKenney
2023-01-24 14:51 ` Paul E. McKenney

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