Re: [PATCH 6.18.y] rcu-tasks: Defer IRQ-disabled callback enqueue to irq_work

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] RCU-tasks callbacks queued with IRQs disabled prior to subsystem initialization are permanently leaked and never executed.
--

--- Patch [1]: [PATCH 6.18.y] rcu-tasks: Defer IRQ-disabled callback enqueue to irq_work ---
commit 0fb74f52f7f1a3087f9fed2016be045b93a6a7a9
Author: Matt Fleming <[email protected]>

rcu-tasks: Defer IRQ-disabled callback enqueue to irq_work
    
This commit splits the rcu-tasks enqueue path, routing callers with IRQs
disabled through a lockless per-CPU llist drained by a hard irq_work. This
prevents acquiring cbs_gbl_lock under unknown outer locks.

Link: https://lore.kernel.org/r/[email protected]

> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index 2dc044fd126eb..56b51946bc6b7 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h

[ ... ]

> @@ -276,6 +283,7 @@ static void cblist_init_generic(struct rcu_tasks *rtp)
>  		if (rcu_segcblist_empty(&rtpcp->cblist))
>  			rcu_segcblist_init(&rtpcp->cblist);
>  		INIT_WORK(&rtpcp->rtp_work, rcu_tasks_invoke_cbs_wq);
> +		init_llist_head(&rtpcp->rtp_irq_bypass_list);

[Severity: High]
Does this initialization overwrite any callbacks queued early in boot?

If call_rcu_tasks_generic() is invoked with IRQs disabled prior to
subsystem initialization, it adds the callback to rtp_irq_bypass_list
and queues the irq_work. Since interrupts are globally disabled early in
boot, the irq_work won't run until after rcu_init().

When rcu_init() calls cblist_init_generic(), will this unconditionally
clear the list head and cause pending early-boot callbacks to be
permanently leaked?

[ ... ]

> @@ -363,43 +492,23 @@ static void call_rcu_tasks_generic(struct rcu_head *rhp, rcu_callback_t func,
>  	chosen_cpu = cpumask_next(ideal_cpu - 1, cpu_possible_mask);
>  	WARN_ON_ONCE(chosen_cpu >= rcu_task_cpu_ids);
>  	rtpcp = per_cpu_ptr(rtp->rtpcpu, chosen_cpu);
> -	if (!raw_spin_trylock_rcu_node(rtpcp)) { // irqs already disabled.

[ ... ]

> -	// Queuing callbacks before initialization not yet supported.
> -	if (WARN_ON_ONCE(!rcu_segcblist_is_enabled(&rtpcp->cblist)))
> -		rcu_segcblist_init(&rtpcp->cblist);

[ ... ]

> -	rcu_segcblist_enqueue(&rtpcp->cblist, rhp);
> +	if (irqsoff) {
> +		llist_add((struct llist_node *)rhp, &rtpcp->rtp_irq_bypass_list);
> +		rcu_read_unlock();
> +		local_irq_restore(flags);
> +		irq_work_queue(&rtpcp->rtp_irq_bypass_work);
> +		return;
>  	}

[Severity: High]
Can this lead to silent data loss if called during early boot?

By routing IRQ-disabled callers directly to the bypass list here, we
circumvent the pre-existing fallback (removed above) that safely
initialized the cblist on demand. Could this result in early-boot
callbacks being queued to an uninitialized list that will subsequently
be wiped out by cblist_init_generic() during rcu_init()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1