mbox series

[bpf-next,0/2] bpf: safeguard hashtab locking in NMI context

Message ID 20201029071925.3103400-1-songliubraving@fb.com
Headers show
Series bpf: safeguard hashtab locking in NMI context | expand

Message

Song Liu Oct. 29, 2020, 7:19 a.m. UTC
LOCKDEP NMI warning highlighted potential deadlock of hashtab in NMI
context:

[   74.828971] ================================
[   74.828972] WARNING: inconsistent lock state
[   74.828973] 5.9.0-rc8+ #275 Not tainted
[   74.828974] --------------------------------
[   74.828975] inconsistent {INITIAL USE} -> {IN-NMI} usage.
[   74.828976] taskset/1174 [HC2[2]:SC0[0]:HE0:SE1] takes:
[...]
[   74.828999]  Possible unsafe locking scenario:
[   74.828999]
[   74.829000]        CPU0
[   74.829001]        ----
[   74.829001]   lock(&htab->buckets[i].raw_lock);
[   74.829003]   <Interrupt>
[   74.829004]     lock(&htab->buckets[i].raw_lock);

Please refer to patch 1/2 for full trace.

This warning is a false alert, as "INITIAL USE" and "IN-NMI" in the tests
are from different hashtab. On the other hand, in theory, it is possible
to deadlock when a hashtab is access from both non-NMI and NMI context.
Patch 1/2 fixes this false alert by assigning separate lockdep class to
each hashtab. Patch 2/2 introduces map_locked counters, which is similar to
bpf_prog_active counter, to avoid hashtab deadlock in NMI context.

Song Liu (2):
  bpf: use separate lockdep class for each hashtab
  bpf: Avoid hashtab deadlock with map_locked

 kernel/bpf/hashtab.c | 126 +++++++++++++++++++++++++++++++------------
 1 file changed, 92 insertions(+), 34 deletions(-)

--
2.24.1

Comments

patchwork-bot+netdevbpf@kernel.org Oct. 30, 2020, 8:20 p.m. UTC | #1
Hello:

This series was applied to bpf/bpf-next.git (refs/heads/master):

On Thu, 29 Oct 2020 00:19:23 -0700 you wrote:
> LOCKDEP NMI warning highlighted potential deadlock of hashtab in NMI

> context:

> 

> [   74.828971] ================================

> [   74.828972] WARNING: inconsistent lock state

> [   74.828973] 5.9.0-rc8+ #275 Not tainted

> [   74.828974] --------------------------------

> [   74.828975] inconsistent {INITIAL USE} -> {IN-NMI} usage.

> [   74.828976] taskset/1174 [HC2[2]:SC0[0]:HE0:SE1] takes:

> [...]

> [   74.828999]  Possible unsafe locking scenario:

> [   74.828999]

> [   74.829000]        CPU0

> [   74.829001]        ----

> [   74.829001]   lock(&htab->buckets[i].raw_lock);

> [   74.829003]   <Interrupt>

> [   74.829004]     lock(&htab->buckets[i].raw_lock);

> 

> [...]


Here is the summary with links:
  - [bpf-next,1/2] bpf: use separate lockdep class for each hashtab
    https://git.kernel.org/bpf/bpf-next/c/c50eb518e262
  - [bpf-next,2/2] bpf: Avoid hashtab deadlock with map_locked
    https://git.kernel.org/bpf/bpf-next/c/20b6cc34ea74

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html