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

[Xin Zhao]

hi, Sebastian

On 2026-03-11 9:33 UTC, Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> wrote:

> On 2026-03-10 19:55:34 [+0800], Xin Zhao wrote:
> > In RT-Linux, when enabling CONFIG_PREEMPT_RT_NEEDS_BH_LOCK, calling
> > __local_bh_disable_ip() when !preemptible() is illegal because it uses
> > local_lock, which might sleep. The only exception is during the
> > cgroup_init() logic in start_kernel() while preemption is disabled,
> > cgroup_init() calls cgroup_idr_alloc(), which calls spin_lock_bh().
> > It is sufficient to only exclude the system startup phase in macro
> > DEBUG_LOCKS_WARN_ON.
> 
> No, cgroup_init() should not be quoted as an exception. It is okay to
> exclude cases while the scheduler is not active because here lock
> contention can not happen.

Yes, I shouldn't quote cgroup_init().

> > 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).

> > 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.

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.

Xin Zhao