From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH v2 net-next] neighbor: Improve garbage collection Date: Sun, 9 Dec 2018 21:14:39 -0800 Message-ID: <0e9345e1-fbed-3f36-e11f-0826188521f0@gmail.com> References: <20181207202457.24574-1-dsahern@kernel.org> <20181207.160347.151827133614133030.davem@davemloft.net> <54aa78ec-9a5b-b798-d93d-7daf2e6b1a96@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, roopa@cumulusnetworks.com, dsahern@gmail.com To: David Miller , dsahern@kernel.org Return-path: Received: from mail-pf1-f196.google.com ([209.85.210.196]:34372 "EHLO mail-pf1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726224AbeLJFOl (ORCPT ); Mon, 10 Dec 2018 00:14:41 -0500 Received: by mail-pf1-f196.google.com with SMTP id h3so4807470pfg.1 for ; Sun, 09 Dec 2018 21:14:41 -0800 (PST) In-Reply-To: <54aa78ec-9a5b-b798-d93d-7daf2e6b1a96@gmail.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 12/09/2018 09:12 PM, Eric Dumazet wrote: > What protects gc_list linkage ? > > We can not use list_del_init(&n->gc_list); or > list_add_tail(&n->gc_list, &n->tbl->gc_list); > > if tbl->lock is not held. > > It seems to me this patch needs more care. > I am playing with a LOCKDEP assist : diff --git a/net/core/neighbour.c b/net/core/neighbour.c index c3b58712e98b9157ca717951da40eb5ae2fe810b..213e56a3816918e599f9a658bcb851711c578f7b 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -122,6 +122,7 @@ static void neigh_mark_dead(struct neighbour *n) { n->dead = 1; if (!list_empty(&n->gc_list)) { + WARN_ON_ONCE(debug_locks && !lockdep_is_held(&n->tbl->lock)); list_del_init(&n->gc_list); atomic_dec(&n->tbl->gc_entries); } @@ -138,10 +139,12 @@ static void neigh_change_state(struct neighbour *n, u8 new) * add to the gc list if new state is not permanent */ if (new_is_perm && on_gc_list) { + WARN_ON_ONCE(debug_locks && !lockdep_is_held(&n->tbl->lock)); list_del_init(&n->gc_list); atomic_dec(&n->tbl->gc_entries); } else if (!new_is_perm && !on_gc_list) { /* add entries to the tail; cleaning removes from the front */ + WARN_ON_ONCE(debug_locks && !lockdep_is_held(&n->tbl->lock)); list_add_tail(&n->gc_list, &n->tbl->gc_list); atomic_inc(&n->tbl->gc_entries); } @@ -391,8 +394,10 @@ static struct neighbour *neigh_alloc(struct neigh_table *tbl, refcount_set(&n->refcnt, 1); n->dead = 1; - if (!permanent) + if (!permanent) { + WARN_ON_ONCE(debug_locks && !lockdep_is_held(&n->tbl->lock)); list_add_tail(&n->gc_list, &n->tbl->gc_list); + } else INIT_LIST_HEAD(&n->gc_list);