From mboxrd@z Thu Jan 1 00:00:00 1970 From: Herbert Xu Subject: Re: [PATCH net-next 4/6] inet: frags: use rhashtables for reassembly units Date: Fri, 30 Mar 2018 15:18:14 +0800 Message-ID: <20180330071814.GA27025@gondor.apana.org.au> References: <20180330052241.206667-2-edumazet@google.com> <20180330052241.206667-5-edumazet@google.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "David S . Miller" , netdev , Florian Westphal , Thomas Graf , Jesper Dangaard Brouer , Alexander Aring , Stefan Schmidt , Eric Dumazet , Nikolay Aleksandrov To: Eric Dumazet Return-path: Received: from orcrist.hmeau.com ([104.223.48.154]:50784 "EHLO deadmen.hmeau.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752548AbeC3HSb (ORCPT ); Fri, 30 Mar 2018 03:18:31 -0400 Content-Disposition: inline In-Reply-To: <20180330052241.206667-5-edumazet@google.com> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, Mar 29, 2018 at 10:22:39PM -0700, Eric Dumazet wrote: > > void inet_frags_exit_net(struct netns_frags *nf) > { > - struct inet_frags *f =nf->f; > - unsigned int seq; > - int i; > - > - nf->low_thresh = 0; > + struct rhashtable_iter hti; > + struct inet_frag_queue *fq; > > -evict_again: > - local_bh_disable(); > - seq = read_seqbegin(&f->rnd_seqlock); > + nf->low_thresh = 0; /* prevent creation of new frags */ > > - for (i = 0; i < INETFRAGS_HASHSZ ; i++) > - inet_evict_bucket(f, &f->hash[i]); > + rhashtable_walk_enter(&nf->rhashtable, &hti); > + do { > + rhashtable_walk_start(&hti); > > - local_bh_enable(); > - cond_resched(); > + while ((fq = rhashtable_walk_next(&hti)) && !IS_ERR(fq)) { > + if (refcount_inc_not_zero(&fq->refcnt)) { > + spin_lock_bh(&fq->lock); > + inet_frag_kill(fq); > + spin_unlock_bh(&fq->lock); > + inet_frag_put(fq); > + } > + } > > - if (read_seqretry(&f->rnd_seqlock, seq) || > - sum_frag_mem_limit(nf)) > - goto evict_again; > + rhashtable_walk_stop(&hti); > + } while (cond_resched(), fq == ERR_PTR(-EAGAIN)); > + rhashtable_walk_exit(&hti); > + rhashtable_destroy(&nf->rhashtable); > } > EXPORT_SYMBOL(inet_frags_exit_net); Instead of using the walk interface, how about rhashtable_free_and_destroy? > void inet_frag_kill(struct inet_frag_queue *fq) > { > if (del_timer(&fq->timer)) > refcount_dec(&fq->refcnt); > > if (!(fq->flags & INET_FRAG_COMPLETE)) { > - fq_unlink(fq); > + struct netns_frags *nf = fq->net; > + > + fq->flags |= INET_FRAG_COMPLETE; > + rhashtable_remove_fast(&nf->rhashtable, &fq->node, nf->f->rhash_params); > refcount_dec(&fq->refcnt); > } > } This means that the hash won't inline properly. Don't know big of an issue it is to you. But you could fix it by doing the same hack as rhashtable by making inet_frag_kill an inline function and take the rhash_params as an explicit argument. > -static struct inet_frag_queue *inet_frag_create(struct netns_frags *nf, > - struct inet_frags *f, > - void *arg) > -{ > - struct inet_frag_queue *q; > + timer_setup(&fq->timer, f->frag_expire, 0); > + spin_lock_init(&fq->lock); > + refcount_set(&fq->refcnt, 3); > + mod_timer(&fq->timer, jiffies + nf->timeout); > > - q = inet_frag_alloc(nf, f, arg); > - if (!q) > + err = rhashtable_insert_fast(&nf->rhashtable, &fq->node, > + f->rhash_params); Ditto. > -struct inet_frag_queue *inet_frag_find(struct netns_frags *nf, > - struct inet_frags *f, void *key, > - unsigned int hash) > +/* TODO : call from rcu_read_lock() and no longer use refcount_inc_not_zero() */ > +struct inet_frag_queue *inet_frag_find(struct netns_frags *nf, void *key) > { > - struct inet_frag_bucket *hb; > - struct inet_frag_queue *q; > - int depth = 0; > - > - if (frag_mem_limit(nf) > nf->low_thresh) > - inet_frag_schedule_worker(f); > - > - hash &= (INETFRAGS_HASHSZ - 1); > - hb = &f->hash[hash]; > - > - spin_lock(&hb->chain_lock); > - hlist_for_each_entry(q, &hb->chain, list) { > - if (q->net == nf && f->match(q, key)) { > - refcount_inc(&q->refcnt); > - spin_unlock(&hb->chain_lock); > - return q; > - } > - depth++; > - } > - spin_unlock(&hb->chain_lock); > + struct inet_frag_queue *fq; > > - if (depth <= INETFRAGS_MAXDEPTH) > - return inet_frag_create(nf, f, key); > + rcu_read_lock(); > > - if (inet_frag_may_rebuild(f)) { > - if (!f->rebuild) > - f->rebuild = true; > - inet_frag_schedule_worker(f); > + fq = rhashtable_lookup(&nf->rhashtable, key, nf->f->rhash_params); Ditto. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt