* [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 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
* 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
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