From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl0-f68.google.com ([209.85.160.68]:43796 "EHLO mail-pl0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932537AbeBVNhe (ORCPT ); Thu, 22 Feb 2018 08:37:34 -0500 Received: by mail-pl0-f68.google.com with SMTP id f23so2873085plr.10 for ; Thu, 22 Feb 2018 05:37:34 -0800 (PST) Message-ID: <1519306651.55655.52.camel@gmail.com> Subject: Re: [PATCH bpf] bpf: fix rcu lockdep warning for lpm_trie map_free callback From: Eric Dumazet To: Yonghong Song , ast@fb.com, daniel@iogearbox.net, edumazet@google.com, netdev@vger.kernel.org Cc: kernel-team@fb.com Date: Thu, 22 Feb 2018 05:37:31 -0800 In-Reply-To: <20180222063818.3215104-1-yhs@fb.com> References: <20180222063818.3215104-1-yhs@fb.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: netdev-owner@vger.kernel.org List-ID: On Wed, 2018-02-21 at 22:38 -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: > > [ 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, this patch simply converted all > rcu protected pointer access to normal access, which removed the > above warning. > > Fixes: 9a3efb6b661f ("bpf: fix memory leak in lpm_trie map_free callback function") > Reported-by: Eric Dumazet > Signed-off-by: Yonghong Song > --- > kernel/bpf/lpm_trie.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c > index a75e02c..0c15813 100644 > --- a/kernel/bpf/lpm_trie.c > +++ b/kernel/bpf/lpm_trie.c > @@ -552,7 +552,7 @@ static struct bpf_map *trie_alloc(union bpf_attr *attr) > static void trie_free(struct bpf_map *map) > { > struct lpm_trie *trie = container_of(map, struct lpm_trie, map); > - struct lpm_trie_node __rcu **slot; > + struct lpm_trie_node **slot; > struct lpm_trie_node *node; > > /* Wait for outstanding programs to complete > @@ -569,23 +569,22 @@ static void trie_free(struct bpf_map *map) > slot = &trie->root; > > for (;;) { > - node = rcu_dereference_protected(*slot, > - lockdep_is_held(&trie->lock)); > + node = *slot; Hi Yonghong It is not sparse compliant. kernel/bpf/lpm_trie.c:573:30: warning: incorrect type in assignment (different address spaces) kernel/bpf/lpm_trie.c:573:30:    expected struct lpm_trie_node *node kernel/bpf/lpm_trie.c:573:30:    got struct lpm_trie_node [noderef] * In my local tree, I simply did node = rcu_dereference_protected(*slot, 1); Since we are the last user of the whole tree after the prior synchronize_rcu();