Re: [PATCH v1] LoongArch: Fix __smp_mb__{before,after}_atomic()

Tiezhu Yang <[email protected]>
Newsgroups dev.linux.lists.loongarch,org.kernel.vger.bpf,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On 2026/8/17 上午11:59, Tiezhu Yang wrote:
> When testing the BPF selftest "sudo ./test_progs -t timer_lockup", there
> is a kernel lockup and panic:

...

> With this patch, the lockless "store-before-load" ordering is enforced by
> the DBAR instruction. The BPF timer_lockup selftest was stressed for 5000
> consecutive loops on a physical LoongArch machine without encountering any
> further lockups or warnings:
> 
>    for i in {1..5000}; do sudo ./test_progs -t timer_lockup; done

...

> diff --git a/arch/loongarch/include/asm/barrier.h b/arch/loongarch/include/asm/barrier.h
> index 4b663f197706..adfe343dfa65 100644
> --- a/arch/loongarch/include/asm/barrier.h
> +++ b/arch/loongarch/include/asm/barrier.h
> @@ -57,8 +57,8 @@
>   #define __WEAK_LLSC_MB		"			\n"
>   #endif
>   
> -#define __smp_mb__before_atomic()	barrier()
> -#define __smp_mb__after_atomic()	barrier()
> +#define __smp_mb__before_atomic()	__smp_mb()
> +#define __smp_mb__after_atomic()	__smp_mb()
>   
>   /**
>    * array_index_mask_nospec() - generate a ~0 mask when index < size, 0 otherwise

Hi all,

To address the soft lockup while avoiding the performance overhead of
executing two separate instructions, I think there is a more elegant
way to fix this directly in the BPF core helper:

We can replace atomic_inc() and smp_mb__after_atomic() with a single
atomic_fetch_add() in bpf_timer_cancel(). This consolidates the logic
into a single native atomic operation with full ordering, which not
only guarantees the store-load order to eliminate the deadlock but
also improves the performance for weak memory model architectures.

Any thoughts on this approach?

```
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index c18f1e16edee..ee2b3a4dcc05 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1591,9 +1591,7 @@ BPF_CALL_1(bpf_timer_cancel, struct bpf_async_kern 
*, async)
          */
         if (!cur_t)
                 goto drop;
-       atomic_inc(&t->cancelling);
-       /* Need full barrier after relaxed atomic_inc */
-       smp_mb__after_atomic();
+       atomic_fetch_add(1, &t->cancelling);
         inc = true;
         if (atomic_read(&cur_t->cancelling)) {
                 /* We're cancelling timer t, while some other timer 
callback is
```

I tested the above diff, the BPF timer_lockup selftest was stressed
for 5000 consecutive loops on a physical LoongArch machine without
encountering any further lockups or warnings.

If you are OK with this change of bpf code, I will send a bpf patch
later.

Thanks,
Tiezhu
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.