Re: [PATCH bpf v1 RESEND] bpf: Fix timer_lockup deadlock using atomic_fetch_inc()

[Tiezhu Yang]

On 2026/9/2 上午9:13, Kumar Kartikeya Dwivedi wrote:
> On Wed Sep 2, 2026 at 2:51 AM CEST, Tiezhu Yang wrote:
> > When testing the BPF selftest "sudo ./test_progs -t timer_lockup", there
> > is a kernel lockup and panic on LoongArch:
> >
> >    watchdog: BUG: soft lockup - CPU#1 stuck for 8s! [test_progs:39601]
> >    Kernel panic - not syncing: softlockup: hung tasks
> >    ...
> >    Call Trace:
> >    ...
> >    [<9000000000c40e98>] panic+0x44/0x48
> >    [<9000000000e8b000>] watchdog_timer_fn+0x500/0x520
> >    [<9000000000dfd9a4>] __hrtimer_run_queues+0xc4/0x530
> >    [<9000000000dffe90>] hrtimer_interrupt+0x140/0x320
> >    ...
> >    [<9000000002ad9e4c>] _raw_spin_unlock_irqrestore+0x8c/0xc0
> >    [<9000000000dfe9e0>] hrtimer_try_to_cancel.part.0+0x70/0x350
> >    [<9000000000dfed58>] hrtimer_cancel+0x38/0x80
> >    [<9000000000f6d944>] bpf_timer_cancel+0x94/0x1e0
> >    [<ffff80000200fad0>] bpf_prog_108ab87b32f22e44_timer_cb1+0xb0/0xfc
> >    [<9000000000f6b838>] bpf_timer_cb+0x98/0x170
> >    [<9000000000dfdaac>] __hrtimer_run_queues+0x1cc/0x530
> >    [<9000000000dfde94>] hrtimer_run_softirq+0x84/0xd0
> >    ...
> >    [<900000000271d9e0>] bpf_test_run+0x1c0/0x5c0
> >    [<900000000271f548>] bpf_prog_test_run_skb+0x6e8/0xe20
> >    [<9000000000f39940>] __sys_bpf+0x1690/0x2c50
> >    [<9000000000f3af28>] sys_bpf+0x28/0x40
> >    [<9000000002ac2d68>] do_syscall+0x108/0x5e0
> >    [<9000000000c6a850>] handle_syscall+0xd0/0x170
> >
> > In bpf_timer_cancel() of kernel/bpf/helpers.c, it explicitly notes that
> > "Need full barrier after relaxed atomic_inc" to ensure global visibility
> > of the cancelling state before performing the lockless dependency checks.
> >
> > However, on weakly-ordered architectures such as LoongArch, the current
> > combination of a relaxed atomic_inc() followed by smp_mb__after_atomic()
> > fails to guarantee the physical store-load ordering because the latter
> > currently expands to an empty compiler barrier rather than a hardware
> > data barrier on LoongArch.
> >
> > This allows a subsequent read to bypass the prior write due to store-load
> > reordering, enabling concurrent CPUs to simultaneously bypass the software
> > deadlock detection, enter hrtimer_cancel(), and then trigger a severe ABBA
> > deadlock in the hrtimer core.
> >
> > Instead of relying on arch-specific macro implementations which may vary
> > in strictness, fix this issue directly in the BPF core helper by replacing
> > atomic_inc() and smp_mb__after_atomic() with a single atomic_fetch_inc()
> > to provide full ordering natively.
>
> This paragraph is completely bogus. LoongArch's smp_mb__after_atomic() had a
> bug. The specification in LKMM for smp_mb__after_atomic() is that it should be a
> full barrier, which can be relaxed if the prior atomic operation already
> provides the necessary ordering.
>
> As I already said, there is no point in changing or "optimizing" this atomic
> inc. Just let it be. You will see no measurable difference for this function.
>
> Just accept that it was a bug, and fix the lowering for your arch. Plenty of
> other logic in the kernel uses this primitive, so I think you folks were just
> lucky this wasn't hit before by something else.

Thanks for your feedback.

I looked into tools/memory-model/Documentation/ordering.txt,
as it explicitly clarifies, atomic_inc() does not guarantee
full ordering on weakly-ordered architectures, therefore
smp_mb__after_atomic() must emit a hardware data barrier to
comply with the LKMM specification, rather than expanding to
a plain compiler barrier.

I also noticed there are hundreds of smp_mb__after_atomic()
call sites across the core kernel (kernel/, drivers/, mm/,
net/, fs/) that currently lack necessary hardware data barriers
on LoongArch, which have potential risks.

So please disregard this bpf patch, I will send a v2 patch to
remove the definition of __smp_mb__{before,after}_atomic() in
arch/loongarch/include/asm/barrier.h, so that these two macros
can automatically fall back to the generic definition in
include/asm-generic/barrier.h:

#ifndef __smp_mb__before_atomic
#define __smp_mb__before_atomic()	__smp_mb()
#endif

#ifndef __smp_mb__after_atomic
#define __smp_mb__after_atomic()	__smp_mb()
#endif

Thanks,
Tiezhu