Re: [PATCH] softirq: WARN_ON !preemptible() not check softirq cnt in bh disable on RT

[Sebastian Andrzej Siewior]

On 2026-03-11 18:40:07 [+0800], Xin Zhao wrote:
> hi, Sebastian
Hi Xin,

> > > Although the original check of this_cpu_read(softirq_ctrl.cnt) can also
> > > prevent the WARN_ON print during the boot process, it may hide some issues
> > > that should be exposed immediately. Because softirq_ctrl.cnt maybe 0 when
> > > __local_bh_disabled_ip() is called in !preemptible() context.
> > 
> > That is actually the point. If it is known that the call chain does not
> > origin from bh-disabled context the it is fine. Well, not fine if you
> > stick to the details but good enough if you don't to constantly complain
> > to everyone about the little things which don't make a difference.
> 
> I just think that using (system_state != SYSTEM_BOOTING) for conditional checks
> is more reasonable than using (softirq_ctrl.cnt).

We had users which which did
	local_irq_disable();
	local_bh_disable();

and we decided not to bother if there is no reason to.

> > > In RT-Linux, __local_bh_disable_ip() will be used by numerous _bh variants
> > > locks and local_bh_disable(). Since locks call __might_resched() check, we
> > > analyze the scenario of using local_bh_disable() in !preemptible() context.
> > > 
> > > If CONFIG_PREEMPT_RT_NEEDS_BH_LOCK is not enabled, __local_bh_disable_ip()
> > > does not enter the local_lock lock and thus using local_bh_disable() in
> > > !preemptible() context does not lead to might sleep problem, but using
> > > local_bh_disable() in !preemptible() state is not meaningful in RT-Linux.
> > 
> > but it does not cause a locking or scheduling problem either.
> > 
> > > In non RT-Linux, we use local_irq_save() followed by local_bh_disable() to
> > > keep soft interrupts disabled after restoring interrupts. However, in
> > > RT-Linux, when CONFIG_PREEMPT_RT_NEEDS_BH_LOCK is not enabled,
> > > local_bh_disable() merely increments the softirq_ctrl.cnt counter without
> > > actually disabling the soft interrupt behavior, because other tasks on the
> > > CPU can preempt the task that wants to disable soft interrupts and execute
> > > soft interrupt-related logic.
> > > Consider the sequence diagram below:
> > > Task A                        Task B
> > >                               __local_bh_enable_ip()
> > >                                 __do_softirq()
> > >                                   handle_softirqs()
> > >                                     ...
> > >                                     local_irq_enable();
> > >                                     ...
> > > local_irq_save()
> > > local_bh_disable()
> > > local_irq_restore()
> > >                                     h->action(); -- it is serving softirq
> > > local_bh_enable()
> > 
> > Okay. How is this a problem? You can enter this scenario event even
> > without disabling interrupts within Task A.
> 
> I should use the situation where CONFIG_PREEMPT_RT_NEEDS_BH_LOCK is enabled to
> illustrate the example above. People would expect that soft interrupts on this
> CPU would not execute after calling local_bh_disable(), but as shown in the
> example, this cannot actually be guaranteed.

If CONFIG_PREEMPT_RT_NEEDS_BH_LOCK is enabled then the scenario is
possible but the DEBUG_LOCKS_WARN_ON will trigger a warning.

> Using (softirq_ctrl.cnt) as a condition for WARN_ON will only report issues in
> scenarios similar to the one shown in the example. In many other cases, during
> the execution of Task A, if there is no soft interrupt-related logic running on
> the CPU, this WARN_ON may not get printed, thus hiding potential issues in the
> code.
> 
> If Task A truly expects that soft interrupts remain disabled after calling
> local_bh_disable(), and this expectation may not be met, we should detect this
> risk early and prompt the user, just like the might_sleep() checks, providing
> warnings proactively rather than indicating an issue after it has occurred.

Hmm. Is there actually anything wrong in tree?
Longterm I would intend to make !CONFIG_PREEMPT_RT_NEEDS_BH_LOCK
default.

> Xin Zhao

Sebastian