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:12:59 -0800 Message-ID: <54aa78ec-9a5b-b798-d93d-7daf2e6b1a96@gmail.com> References: <20181207202457.24574-1-dsahern@kernel.org> <20181207.160347.151827133614133030.davem@davemloft.net> 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-pl1-f195.google.com ([209.85.214.195]:40883 "EHLO mail-pl1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726389AbeLJFNC (ORCPT ); Mon, 10 Dec 2018 00:13:02 -0500 Received: by mail-pl1-f195.google.com with SMTP id u18so4657922plq.7 for ; Sun, 09 Dec 2018 21:13:01 -0800 (PST) In-Reply-To: <20181207.160347.151827133614133030.davem@davemloft.net> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 12/07/2018 04:03 PM, David Miller wrote: > From: David Ahern > Date: Fri, 7 Dec 2018 12:24:57 -0800 > >> From: David Ahern >> >> The existing garbage collection algorithm has a number of problems: > ... >> This patch addresses these problems as follows: >> >> 1. Use of a separate list_head to track entries that can be garbage >> collected along with a separate counter. PERMANENT entries are not >> added to this list. >> >> The gc_thresh parameters are only compared to the new counter, not the >> total entries in the table. The forced_gc function is updated to only >> walk this new gc_list looking for entries to evict. >> >> 2. Entries are added to the list head at the tail and removed from the >> front. >> >> 3. Entries are only evicted if they were last updated more than 5 seconds >> ago, adhering to the original intent of gc_thresh2. >> >> 4. Forced gc is stopped once the number of gc_entries drops below >> gc_thresh2. >> >> 5. Since gc checks do not apply to PERMANENT entries, gc levels are skipped >> when allocating a new neighbor for a PERMANENT entry. By extension this >> means there are no explicit limits on the number of PERMANENT entries >> that can be created, but this is no different than FIB entries or FDB >> entries. >> >> Signed-off-by: David Ahern >> --- >> v2 >> - remove on_gc_list boolean in favor of !list_empty >> - fix neigh_alloc to add new entry to tail of list_head > > Again, looks great, applied. > 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.