Re: [PATCH net] inet: frags: rework rhashtable dismantle

On Mon, Oct 01, 2018 at 10:58:21AM -0700, Eric Dumazet wrote:

 void inet_frags_exit_net(struct netns_frags *nf)
 {
+	struct rhashtable_iter hti;
+	struct inet_frag_queue *fq;
+
+	/* Since we want to cleanup the hashtable, make sure that
+	 * we wont trigger an automatic shrinking while in our
+	 * rhashtable_walk_next() loop.
+	 * Also make sure that no resize is in progress.
+	 */
 	nf->high_thresh = 0; /* prevent creation of new frags */
+	nf->rhashtable.p.automatic_shrinking = false;
+	cancel_work_sync(&nf->rhashtable.run_work);
 
-	rhashtable_free_and_destroy(&nf->rhashtable, inet_frags_free_cb, NULL);
+	rhashtable_walk_enter(&nf->rhashtable, &hti);
+	rhashtable_walk_start(&hti);
+	while ((fq = rhashtable_walk_next(&hti)) != NULL) {
+		if (IS_ERR(fq)) /* should not happen */
+			break;
+		if (!del_timer_sync(&fq->timer))
+			continue;
+
+		spin_lock_bh(&fq->lock);
+		inet_frag_kill(fq);
+		spin_unlock_bh(&fq->lock);
+
+		inet_frag_put(fq);
+		if (need_resched()) {
+			rhashtable_walk_stop(&hti);
+			cond_resched();
+			rhashtable_walk_start(&hti);
+		}
+	}

The walk interface was designed to handle read-only iteration
through the hash table.  While this probably works since the
actual freeing is delayed by RCU, it seems to be rather fragile.

How about using the dead flag but instead of putting it in the
rhashtable put it in netns_frags and have the timers check on that
before calling rhashtable_remove?

Cheers,
-- 
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt