From: "Paul E. McKenney" <paulmck@kernel.org>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: rcu@vger.kernel.org
Subject: Re: [bug report] rcu: Restrict access to RCU CPU stall notifiers
Date: Tue, 7 Nov 2023 19:44:37 -0800 [thread overview]
Message-ID: <48a6763a-fe83-4d65-b99d-1ee79c1cb1db@paulmck-laptop> (raw)
In-Reply-To: <5da8a140-8988-43e6-a2c9-31534b90ec01@moroto.mountain>
On Tue, Nov 07, 2023 at 06:19:37PM +0300, Dan Carpenter wrote:
> Hello Paul E. McKenney,
>
> The patch 7939f0d7b28c: "rcu: Restrict access to RCU CPU stall
> notifiers" from Nov 1, 2023 (linux-next), leads to the following
> Smatch static checker warning:
>
> kernel/rcu/rcutorture.c:2502 rcu_torture_stall()
> error: uninitialized symbol 'ret'.
>
> kernel/rcu/rcutorture.c
> 2446 static int rcu_torture_stall(void *args)
> 2447 {
> 2448 int idx;
> 2449 int ret;
> 2450 unsigned long stop_at;
> 2451
> 2452 VERBOSE_TOROUT_STRING("rcu_torture_stall task started");
> 2453 if (rcu_cpu_stall_notifiers) {
> 2454 ret = rcu_stall_chain_notifier_register(&rcu_torture_stall_block);
> 2455 if (ret)
> 2456 pr_info("%s: rcu_stall_chain_notifier_register() returned %d, %sexpected.\n",
> 2457 __func__, ret, !IS_ENABLED(CONFIG_RCU_STALL_COMMON) ? "un" : "");
>
> ret is only initialized if rcu_cpu_stall_notifiers is true.
>
> 2458 }
> 2459 if (stall_cpu_holdoff > 0) {
> 2460 VERBOSE_TOROUT_STRING("rcu_torture_stall begin holdoff");
> 2461 schedule_timeout_interruptible(stall_cpu_holdoff * HZ);
> 2462 VERBOSE_TOROUT_STRING("rcu_torture_stall end holdoff");
> 2463 }
> 2464 if (!kthread_should_stop() && stall_gp_kthread > 0) {
> 2465 VERBOSE_TOROUT_STRING("rcu_torture_stall begin GP stall");
> 2466 rcu_gp_set_torture_wait(stall_gp_kthread * HZ);
> 2467 for (idx = 0; idx < stall_gp_kthread + 2; idx++) {
> 2468 if (kthread_should_stop())
> 2469 break;
> 2470 schedule_timeout_uninterruptible(HZ);
> 2471 }
> 2472 }
> 2473 if (!kthread_should_stop() && stall_cpu > 0) {
> 2474 VERBOSE_TOROUT_STRING("rcu_torture_stall begin CPU stall");
> 2475 stop_at = ktime_get_seconds() + stall_cpu;
> 2476 /* RCU CPU stall is expected behavior in following code. */
> 2477 idx = cur_ops->readlock();
> 2478 if (stall_cpu_irqsoff)
> 2479 local_irq_disable();
> 2480 else if (!stall_cpu_block)
> 2481 preempt_disable();
> 2482 pr_alert("%s start on CPU %d.\n",
> 2483 __func__, raw_smp_processor_id());
> 2484 while (ULONG_CMP_LT((unsigned long)ktime_get_seconds(),
> 2485 stop_at))
> 2486 if (stall_cpu_block) {
> 2487 #ifdef CONFIG_PREEMPTION
> 2488 preempt_schedule();
> 2489 #else
> 2490 schedule_timeout_uninterruptible(HZ);
> 2491 #endif
> 2492 } else if (stall_no_softlockup) {
> 2493 touch_softlockup_watchdog();
> 2494 }
> 2495 if (stall_cpu_irqsoff)
> 2496 local_irq_enable();
> 2497 else if (!stall_cpu_block)
> 2498 preempt_enable();
> 2499 cur_ops->readunlock(idx);
> 2500 }
> 2501 pr_alert("%s end.\n", __func__);
> --> 2502 if (!ret) {
>
> Uninitialized here
Good catch, thank you!!!
> 2503 if (rcu_cpu_stall_notifiers) {
>
> But maybe we can just reverse the tests or delete one of the conditions?
>
> 2504 ret = rcu_stall_chain_notifier_unregister(&rcu_torture_stall_block);
> 2505 if (ret)
> 2506 pr_info("%s: rcu_stall_chain_notifier_unregister() returned %d.\n", __func__, ret);
> 2507 }
> 2508 }
> 2509 torture_shutdown_absorb("rcu_torture_stall");
> 2510 while (!kthread_should_stop())
> 2511 schedule_timeout_interruptible(10 * HZ);
> 2512 return 0;
> 2513 }
How does the following patch look, to be folded into the original with
attribution?
Thanx, Paul
------------------------------------------------------------------------
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 9f0e6c1cad443..50ac86a348768 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -2503,12 +2503,10 @@ static int rcu_torture_stall(void *args)
cur_ops->readunlock(idx);
}
pr_alert("%s end.\n", __func__);
- if (!ret) {
- if (rcu_cpu_stall_notifiers) {
- ret = rcu_stall_chain_notifier_unregister(&rcu_torture_stall_block);
- if (ret)
- pr_info("%s: rcu_stall_chain_notifier_unregister() returned %d.\n", __func__, ret);
- }
+ if (rcu_cpu_stall_notifiers && !ret) {
+ ret = rcu_stall_chain_notifier_unregister(&rcu_torture_stall_block);
+ if (ret)
+ pr_info("%s: rcu_stall_chain_notifier_unregister() returned %d.\n", __func__, ret);
}
torture_shutdown_absorb("rcu_torture_stall");
while (!kthread_should_stop())
prev parent reply other threads:[~2023-11-08 3:44 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-07 15:19 [bug report] rcu: Restrict access to RCU CPU stall notifiers Dan Carpenter
2023-11-08 3:44 ` Paul E. McKenney [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=48a6763a-fe83-4d65-b99d-1ee79c1cb1db@paulmck-laptop \
--to=paulmck@kernel.org \
--cc=dan.carpenter@linaro.org \
--cc=rcu@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).