Re: [PATCH v2 net-next] neighbor: Improve garbage collection

[Eric Dumazet]

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);