| Message ID | 20180222181035.2163332-1-yhs@fb.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf,v2] bpf: fix rcu lockdep warning for lpm_trie map_free callback | expand |
On Thu, 2018-02-22 at 10:10 -0800, Yonghong Song wrote: > Commit 9a3efb6b661f ("bpf: fix memory leak in lpm_trie map_free callback function") > fixed a memory leak and removed unnecessary locks in map_free callback function. > Unfortrunately, it introduced a lockdep warning. When lockdep checking is turned on, > running tools/testing/selftests/bpf/test_lpm_map will have: > > Fixes: 9a3efb6b661f ("bpf: fix memory leak in lpm_trie map_free callback function") > Reported-by: Eric Dumazet <edumazet@google.com> > Suggested-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Yonghong Song <yhs@fb.com> > --- > kernel/bpf/lpm_trie.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > v1 -> v2: > . fix sparse warning which is introduced by v1, suggested by Eric. > > diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c > index a75e02c..b4b5b81 100644 > --- a/kernel/bpf/lpm_trie.c > +++ b/kernel/bpf/lpm_trie.c > @@ -569,8 +569,7 @@ static void trie_free(struct bpf_map *map) > slot = &trie->root; > > for (;;) { > - node = rcu_dereference_protected(*slot, > - lockdep_is_held(&trie->lock)); > + node = rcu_dereference_protected(*slot, 1); > if (!node) > goto out; SGTM, thanks ! Reviewed-by: Eric Dumazet <edumazet@google.com>
From: Yonghong Song <yhs@fb.com> Date: Thu, 22 Feb 2018 10:10:35 -0800 > Commit 9a3efb6b661f ("bpf: fix memory leak in lpm_trie map_free callback function") > fixed a memory leak and removed unnecessary locks in map_free callback function. > Unfortrunately, it introduced a lockdep warning. When lockdep checking is turned on, > running tools/testing/selftests/bpf/test_lpm_map will have: > > [ 98.294321] ============================= > [ 98.294807] WARNING: suspicious RCU usage > [ 98.295359] 4.16.0-rc2+ #193 Not tainted > [ 98.295907] ----------------------------- > [ 98.296486] /home/yhs/work/bpf/kernel/bpf/lpm_trie.c:572 suspicious rcu_dereference_check() usage! > [ 98.297657] > [ 98.297657] other info that might help us debug this: > [ 98.297657] > [ 98.298663] > [ 98.298663] rcu_scheduler_active = 2, debug_locks = 1 > [ 98.299536] 2 locks held by kworker/2:1/54: > [ 98.300152] #0: ((wq_completion)"events"){+.+.}, at: [<00000000196bc1f0>] process_one_work+0x157/0x5c0 > [ 98.301381] #1: ((work_completion)(&map->work)){+.+.}, at: [<00000000196bc1f0>] process_one_work+0x157/0x5c0 > > Since actual trie tree removal happens only after no other > accesses to the tree are possible, replacing > rcu_dereference_protected(*slot, lockdep_is_held(&trie->lock)) > with > rcu_dereference_protected(*slot, 1) > fixed the issue. > > Fixes: 9a3efb6b661f ("bpf: fix memory leak in lpm_trie map_free callback function") > Reported-by: Eric Dumazet <edumazet@google.com> > Suggested-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Yonghong Song <yhs@fb.com> Acked-by: David S. Miller <davem@davemloft.net>
On 02/22/2018 07:10 PM, Yonghong Song wrote: > Commit 9a3efb6b661f ("bpf: fix memory leak in lpm_trie map_free callback function") > fixed a memory leak and removed unnecessary locks in map_free callback function. > Unfortrunately, it introduced a lockdep warning. When lockdep checking is turned on, > running tools/testing/selftests/bpf/test_lpm_map will have: > > [ 98.294321] ============================= > [ 98.294807] WARNING: suspicious RCU usage > [ 98.295359] 4.16.0-rc2+ #193 Not tainted > [ 98.295907] ----------------------------- > [ 98.296486] /home/yhs/work/bpf/kernel/bpf/lpm_trie.c:572 suspicious rcu_dereference_check() usage! > [ 98.297657] > [ 98.297657] other info that might help us debug this: > [ 98.297657] > [ 98.298663] > [ 98.298663] rcu_scheduler_active = 2, debug_locks = 1 > [ 98.299536] 2 locks held by kworker/2:1/54: > [ 98.300152] #0: ((wq_completion)"events"){+.+.}, at: [<00000000196bc1f0>] process_one_work+0x157/0x5c0 > [ 98.301381] #1: ((work_completion)(&map->work)){+.+.}, at: [<00000000196bc1f0>] process_one_work+0x157/0x5c0 > > Since actual trie tree removal happens only after no other > accesses to the tree are possible, replacing > rcu_dereference_protected(*slot, lockdep_is_held(&trie->lock)) > with > rcu_dereference_protected(*slot, 1) > fixed the issue. > > Fixes: 9a3efb6b661f ("bpf: fix memory leak in lpm_trie map_free callback function") > Reported-by: Eric Dumazet <edumazet@google.com> > Suggested-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Yonghong Song <yhs@fb.com> Applied to bpf tree, thanks everyone!
diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c index a75e02c..b4b5b81 100644 --- a/kernel/bpf/lpm_trie.c +++ b/kernel/bpf/lpm_trie.c @@ -569,8 +569,7 @@ static void trie_free(struct bpf_map *map) slot = &trie->root; for (;;) { - node = rcu_dereference_protected(*slot, - lockdep_is_held(&trie->lock)); + node = rcu_dereference_protected(*slot, 1); if (!node) goto out;
Commit 9a3efb6b661f ("bpf: fix memory leak in lpm_trie map_free callback function") fixed a memory leak and removed unnecessary locks in map_free callback function. Unfortrunately, it introduced a lockdep warning. When lockdep checking is turned on, running tools/testing/selftests/bpf/test_lpm_map will have: [ 98.294321] ============================= [ 98.294807] WARNING: suspicious RCU usage [ 98.295359] 4.16.0-rc2+ #193 Not tainted [ 98.295907] ----------------------------- [ 98.296486] /home/yhs/work/bpf/kernel/bpf/lpm_trie.c:572 suspicious rcu_dereference_check() usage! [ 98.297657] [ 98.297657] other info that might help us debug this: [ 98.297657] [ 98.298663] [ 98.298663] rcu_scheduler_active = 2, debug_locks = 1 [ 98.299536] 2 locks held by kworker/2:1/54: [ 98.300152] #0: ((wq_completion)"events"){+.+.}, at: [<00000000196bc1f0>] process_one_work+0x157/0x5c0 [ 98.301381] #1: ((work_completion)(&map->work)){+.+.}, at: [<00000000196bc1f0>] process_one_work+0x157/0x5c0 Since actual trie tree removal happens only after no other accesses to the tree are possible, replacing rcu_dereference_protected(*slot, lockdep_is_held(&trie->lock)) with rcu_dereference_protected(*slot, 1) fixed the issue. Fixes: 9a3efb6b661f ("bpf: fix memory leak in lpm_trie map_free callback function") Reported-by: Eric Dumazet <edumazet@google.com> Suggested-by: Eric Dumazet <edumazet@google.com> Signed-off-by: Yonghong Song <yhs@fb.com> --- kernel/bpf/lpm_trie.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) v1 -> v2: . fix sparse warning which is introduced by v1, suggested by Eric.