Received: by 2002:ac0:a591:0:0:0:0:0 with SMTP id m17-v6csp1477012imm; Fri, 6 Jul 2018 00:25:32 -0700 (PDT) X-Google-Smtp-Source: AAOMgpdkA4wJnIlPffK23JEFOXdRiI77aDKogqr7PMBTghG/buuXX2pgNSdr9pLkIn0OyTGuea7r X-Received: by 2002:a65:418b:: with SMTP id a11-v6mr8408013pgq.118.1530861932162; Fri, 06 Jul 2018 00:25:32 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1530861932; cv=none; d=google.com; s=arc-20160816; b=WQVotl9YaASjPakvelm5/fJfBLVO5WnLNbixQfIYMa0BuiCnFsHmWhbSTYjnJBJ+ag tU3fdpV/Ax5fL7gfvzUGrtkuv+8iEjSXpaIePmLCYUNzeNJ0ZL/sJ/b/7ZRRLdC5BSsw 6Pl0Tv1OgjpMFx8iRT/9ZxoFBwVa0LUdy6hR82CjC60OVTsnOMoOb6B1n2VfOA1ACZ4J RtEEMCKEYcZ4lTjToG6PTypl4zDpXxamfTU358J7mbvc8C7bxX4mDHd+P8J0V5mNo7cE Fe3bw/GKFX9+1g2Kx9i4uvf1WyRIRgnlhJ0DrVDrA3poSNBS18tNUuh7sF4aTyK8KwlG DYBA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:cc:subject:date:to :from:arc-authentication-results; bh=EZTyvCoe8PpLQ39oV7xppItGVD+pNCpJuBt54Y4G1+o=; b=QQxgxdMp92kRosQckM9tJNBtIn2N2K1jUX0pSwU8hASQl1x1E8EUN3iaE2Bp1iXYFX 4KjSuSDrK5YhBWN01Jei9IwkV6pbM6So/omdAdGbUO11j4ehyTQVOzQF+x4Mq1itIii+ DmWxSwjIW6j5Kzf6ZhQWEOFod0PPGhZV34+GbTqHTyDa+REOhMusW2XLELH/HX88Y289 L2p75qc3t6JOf8Lv+9ry+GtH9vJDH+s/DZtFZZpSk0UlNL8gsozF8hrsMeHNycWGfBQQ xQAPEeCA9YVI+CIEH5KKejteRTWMnX2JO5YZv72yyWtAEw3hrQCVMipDhxtvqKsFOk9D 0SUg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id w133-v6si8280357pfd.313.2018.07.06.00.25.17; Fri, 06 Jul 2018 00:25:32 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754215AbeGFHXU (ORCPT + 99 others); Fri, 6 Jul 2018 03:23:20 -0400 Received: from mx2.suse.de ([195.135.220.15]:48900 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753476AbeGFHXR (ORCPT ); Fri, 6 Jul 2018 03:23:17 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 70B79AFCC; Fri, 6 Jul 2018 07:23:16 +0000 (UTC) From: NeilBrown To: Thomas Graf , Herbert Xu Date: Fri, 06 Jul 2018 17:22:30 +1000 Subject: [PATCH 2/5] rhashtable: don't hold lock on first table throughout insertion. Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org Message-ID: <153086175009.24852.7782466383056542839.stgit@noble> In-Reply-To: <153086169828.24852.10332573315056854948.stgit@noble> References: <153086169828.24852.10332573315056854948.stgit@noble> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org rhashtable_try_insert() currently hold a lock on the bucket in the first table, while also locking buckets in subsequent tables. This is unnecessary and looks like a hold-over from some earlier version of the implementation. As insert and remove always lock a bucket in each table in turn, and as insert only inserts in the final table, there cannot be any races that are not covered by simply locking a bucket in each table in turn. When an insert call reaches that last table it can be sure that there is no match entry in any other table as it has searched them all, and insertion never happens anywhere but in the last table. The fact that code tests for the existence of future_tbl while holding a lock on the relevant bucket ensures that two threads inserting the same key will make compatible decisions about which is the "last" table. This simplifies the code and allows the ->rehash field to be discarded. We still need a way to ensure that a dead bucket_table is never re-linked by rhashtable_walk_stop(). This can be achieved by calling call_rcu() inside the locked region, and checking ->rcu.func in rhashtable_walk_stop(). If it is not NULL, then the bucket table is empty and dead. Signed-off-by: NeilBrown --- include/linux/rhashtable.h | 13 ------------- lib/rhashtable.c | 44 +++++++++++--------------------------------- 2 files changed, 11 insertions(+), 46 deletions(-) diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h index 96ebc2690027..26ed3b299028 100644 --- a/include/linux/rhashtable.h +++ b/include/linux/rhashtable.h @@ -63,7 +63,6 @@ struct bucket_table { unsigned int size; unsigned int nest; - unsigned int rehash; u32 hash_rnd; unsigned int locks_mask; spinlock_t *locks; @@ -802,12 +801,6 @@ static inline int rhltable_insert( * @obj: pointer to hash head inside object * @params: hash table parameters * - * Locks down the bucket chain in both the old and new table if a resize - * is in progress to ensure that writers can't remove from the old table - * and can't insert to the new table during the atomic operation of search - * and insertion. Searches for duplicates in both the old and new table if - * a resize is in progress. - * * This lookup function may only be used for fixed key hash table (key_len * parameter set). It will BUG() if used inappropriately. * @@ -863,12 +856,6 @@ static inline void *rhashtable_lookup_get_insert_fast( * @obj: pointer to hash head inside object * @params: hash table parameters * - * Locks down the bucket chain in both the old and new table if a resize - * is in progress to ensure that writers can't remove from the old table - * and can't insert to the new table during the atomic operation of search - * and insertion. Searches for duplicates in both the old and new table if - * a resize is in progress. - * * Lookups may occur in parallel with hashtable mutations and resizing. * * Will trigger an automatic deferred table resizing if residency in the diff --git a/lib/rhashtable.c b/lib/rhashtable.c index c6eaeaff16d1..27a5ffa993d4 100644 --- a/lib/rhashtable.c +++ b/lib/rhashtable.c @@ -289,10 +289,9 @@ static int rhashtable_rehash_chain(struct rhashtable *ht, while (!(err = rhashtable_rehash_one(ht, old_hash))) ; - if (err == -ENOENT) { - old_tbl->rehash++; + if (err == -ENOENT) err = 0; - } + spin_unlock_bh(old_bucket_lock); return err; @@ -339,13 +338,16 @@ static int rhashtable_rehash_table(struct rhashtable *ht) spin_lock(&ht->lock); list_for_each_entry(walker, &old_tbl->walkers, list) walker->tbl = NULL; - spin_unlock(&ht->lock); /* Wait for readers. All new readers will see the new * table, and thus no references to the old table will * remain. + * We do this inside the locked region so that + * rhashtable_walk_stop() can check ->rcu.func and know + * not to re-link the table. */ call_rcu(&old_tbl->rcu, bucket_table_free_rcu); + spin_unlock(&ht->lock); return rht_dereference(new_tbl->future_tbl, ht) ? -EAGAIN : 0; } @@ -591,36 +593,14 @@ static void *rhashtable_try_insert(struct rhashtable *ht, const void *key, struct bucket_table *new_tbl; struct bucket_table *tbl; unsigned int hash; - spinlock_t *lock; void *data; - tbl = rcu_dereference(ht->tbl); - - /* All insertions must grab the oldest table containing - * the hashed bucket that is yet to be rehashed. - */ - for (;;) { - hash = rht_head_hashfn(ht, tbl, obj, ht->p); - lock = rht_bucket_lock(tbl, hash); - spin_lock_bh(lock); - - if (tbl->rehash <= hash) - break; - - spin_unlock_bh(lock); - tbl = rht_dereference_rcu(tbl->future_tbl, ht); - } + new_tbl = rcu_dereference(ht->tbl); - data = rhashtable_lookup_one(ht, tbl, hash, key, obj); - new_tbl = rhashtable_insert_one(ht, tbl, hash, obj, data); - if (PTR_ERR(new_tbl) != -EEXIST) - data = ERR_CAST(new_tbl); - - while (!IS_ERR_OR_NULL(new_tbl)) { + do { tbl = new_tbl; hash = rht_head_hashfn(ht, tbl, obj, ht->p); - spin_lock_nested(rht_bucket_lock(tbl, hash), - SINGLE_DEPTH_NESTING); + spin_lock(rht_bucket_lock(tbl, hash)); data = rhashtable_lookup_one(ht, tbl, hash, key, obj); new_tbl = rhashtable_insert_one(ht, tbl, hash, obj, data); @@ -628,9 +608,7 @@ static void *rhashtable_try_insert(struct rhashtable *ht, const void *key, data = ERR_CAST(new_tbl); spin_unlock(rht_bucket_lock(tbl, hash)); - } - - spin_unlock_bh(lock); + } while (!IS_ERR_OR_NULL(new_tbl)); if (PTR_ERR(data) == -EAGAIN) data = ERR_PTR(rhashtable_insert_rehash(ht, tbl) ?: @@ -964,7 +942,7 @@ void rhashtable_walk_stop(struct rhashtable_iter *iter) ht = iter->ht; spin_lock(&ht->lock); - if (tbl->rehash < tbl->size) + if (tbl->rcu.func == NULL) list_add(&iter->walker.list, &tbl->walkers); else iter->walker.tbl = NULL;