Re: [PATCH v2 2/6] rcu: Make Tiny call_rcu() safe to call from any context
[email protected] Mon, 03 Aug 2026 14:35:03 +0000
| Newsgroups | dev.linux.lists.linux-rt-devel,org.kernel.vger.bpf,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 2 potential issue(= s) to consider: - [High] Relocating `debug_rcu_head_queue()` into `rcu_do_enqueue()` bypass= es double-free detection for deferred callbacks, silently corrupting the lo= ckless list and inducing a permanent hard livelock during `rcu_defer_drain(= )`. - [Medium] Invalid `WARN_ON_ONCE` in `call_rcu()` falsely flags safe NMI ex= ecutions due to a flawed concurrency assumption. -- --- Patch [2]: [PATCH v2 2/6] rcu: Make Tiny call_rcu() safe to call from a= ny context --- commit 866e1596ae5a7314657b9b0164dfabf922016c70 Author: Puranjay Mohan <[email protected]> rcu: Make Tiny call_rcu() safe to call from any context Give Tiny call_rcu() the same treatment as Tree RCU. When interrupts are disabled and the scheduler is up, stage the callback on a lockless list that an irq_work re-issues later. One global list and irq_work suffice since Tiny RCU is uniprocessor, and there is no CPU-offline drain. > diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c > index dccccd6be9411..5736b964d8ee1 100644 > --- a/kernel/rcu/tiny.c > +++ b/kernel/rcu/tiny.c [ ... ] > @@ -42,8 +44,86 @@ static struct rcu_ctrlblk rcu_ctrlblk =3D { [ ... ] > +static void call_rcu_defer(struct rcu_head *head, rcu_callback_t func) > +{ > + /* A re-entrant call_rcu() during the drain would livelock it; drop it.= */ > + if (rcu_defer_draining && !in_nmi()) { > + WARN_ONCE(1, "call_rcu() re-entered during callback drain; leaking cal= lback\n"); > + return; > + } > + head->func =3D func; > + if (llist_add((struct llist_node *)head, &rcu_defer_list)) [Severity: High] Does moving the debug_rcu_head_queue() check into rcu_do_enqueue() bypass double-free detection for deferred callbacks? If a double-free occurs, llist_add() will be called twice on the same node = here, corrupting the lockless list by creating a circular link (head->next =3D he= ad). When the irq_work executes rcu_defer_drain(), won't the llist_for_each_safe= () iteration infinitely hang on the circular list before the delayed debug che= ck can prevent the livelock? > + irq_work_queue(&rcu_defer_iw); > +} [ ... ] > @@ -157,29 +237,15 @@ EXPORT_SYMBOL_GPL(synchronize_rcu); > */ > void call_rcu(struct rcu_head *head, rcu_callback_t func) > { > - static atomic_t doublefrees; > - unsigned long flags; [ ... ] > + if (should_rcu_defer()) { > + call_rcu_defer(head, func); > return; > } > =20 > - head->func =3D func; > - head->next =3D NULL; > - > - local_irq_save(flags); > - *rcu_ctrlblk.curtail =3D head; > - rcu_ctrlblk.curtail =3D &head->next; > - local_irq_restore(flags); > + /* An NMI reaching here entered with irqs enabled, so the enqueue can r= ace. */ > + WARN_ON_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && in_nmi()); [Severity: Medium] Is this concurrency assumption backwards?=20 If an NMI interrupted a context where IRQs were enabled, that interrupted context could not have been in the middle of rcu_do_enqueue() because rcu_do_enqueue() disables IRQs via local_irq_save(). Since it's impossible for the NMI to race with rcu_do_enqueue() in this scenario, will this falsely trigger kernel warnings during safe NMI executions? > - if (unlikely(is_idle_task(current))) { > - /* force scheduling for rcu_qs() */ > - resched_cpu(0); > - } > + rcu_do_enqueue(head, func); > } > EXPORT_SYMBOL_GPL(call_rcu); --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803134839.2103= [email protected]?part=3D2