From patchwork Sun Mar 15 10:12:05 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Herbert Xu X-Patchwork-Id: 450256 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 4691E1400B6 for ; Sun, 15 Mar 2015 21:12:19 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751978AbbCOKMO (ORCPT ); Sun, 15 Mar 2015 06:12:14 -0400 Received: from ringil.hengli.com.au ([178.18.16.133]:60374 "EHLO ringil.hengli.com.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751856AbbCOKMK (ORCPT ); Sun, 15 Mar 2015 06:12:10 -0400 Received: from gondolin.me.apana.org.au ([192.168.0.6]) by norbury.hengli.com.au with esmtp (Exim 4.80 #3 (Debian)) id 1YX5Wf-0003wq-O0; Sun, 15 Mar 2015 21:12:05 +1100 Received: from herbert by gondolin.me.apana.org.au with local (Exim 4.80) (envelope-from ) id 1YX5Wf-0005aA-JC; Sun, 15 Mar 2015 21:12:05 +1100 Subject: [v1 PATCH 2/2] rhashtable: Fix rhashtable_remove failures References: <20150315101004.GA21383@gondor.apana.org.au> To: David Miller , tgraf@suug.ch, netdev@vger.kernel.org Message-Id: From: Herbert Xu Date: Sun, 15 Mar 2015 21:12:05 +1100 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org The commit 9d901bc05153bbf33b5da2cd6266865e531f0545 ("rhashtable: Free bucket tables asynchronously after rehash") causes gratuitous failures in rhashtable_remove. The reason is that it inadvertently introduced multiple rehashing from the perspective of readers. IOW it is now possible to see more than two tables during a single RCU critical section. Fortunately the other reader rhashtable_lookup already deals with this correctly thanks to c4db8848af6af92f90462258603be844baeab44d ("rhashtable: rhashtable: Move future_tbl into struct bucket_table") so only rhashtable_remove is broken by this change. This patch fixes this by looping over every table from the first one to the last or until we find the element that we were trying to delete. Incidentally the simple test for detecting rehashing to prevent starting another shrinking no longer works. Since it isn't needed anyway (the work queue and the mutex serves as a natural barrier to unnecessary rehashes) I've simply killed the test. Signed-off-by: Herbert Xu --- lib/rhashtable.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/lib/rhashtable.c b/lib/rhashtable.c index b916679..c523d3a 100644 --- a/lib/rhashtable.c +++ b/lib/rhashtable.c @@ -511,28 +511,25 @@ static bool __rhashtable_remove(struct rhashtable *ht, */ bool rhashtable_remove(struct rhashtable *ht, struct rhash_head *obj) { - struct bucket_table *tbl, *old_tbl; + struct bucket_table *tbl; bool ret; rcu_read_lock(); - old_tbl = rht_dereference_rcu(ht->tbl, ht); - ret = __rhashtable_remove(ht, old_tbl, obj); + tbl = rht_dereference_rcu(ht->tbl, ht); /* Because we have already taken (and released) the bucket * lock in old_tbl, if we find that future_tbl is not yet * visible then that guarantees the entry to still be in - * old_tbl if it exists. + * the old tbl if it exists. */ - tbl = rht_dereference_rcu(old_tbl->future_tbl, ht) ?: old_tbl; - if (!ret && old_tbl != tbl) - ret = __rhashtable_remove(ht, tbl, obj); + while (!(ret = __rhashtable_remove(ht, tbl, obj)) && + (tbl = rht_dereference_rcu(tbl->future_tbl, ht))) + ; if (ret) { - bool no_resize_running = tbl == old_tbl; - atomic_dec(&ht->nelems); - if (no_resize_running && rht_shrink_below_30(ht, tbl)) + if (rht_shrink_below_30(ht, tbl)) schedule_work(&ht->run_work); }