All of lore.kernel.org
 help / color / mirror / Atom feed
From: Herbert Xu <herbert@gondor.apana.org.au>
To: David Miller <davem@davemloft.net>,
	tgraf@suug.ch, netdev@vger.kernel.org
Subject: [v1 PATCH 2/2] rhashtable: Fix rhashtable_remove failures
Date: Sun, 15 Mar 2015 21:12:05 +1100	[thread overview]
Message-ID: <E1YX5Wf-0005aA-JC@gondolin.me.apana.org.au> (raw)
In-Reply-To: 20150315101004.GA21383@gondor.apana.org.au

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 <herbert@gondor.apana.org.au>
---

 lib/rhashtable.c |   17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

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);
 	}
 

  parent reply	other threads:[~2015-03-15 10:12 UTC|newest]

Thread overview: 113+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-13  9:56 [PATCH 0/6] rhashtable: Fixes + cleanups + preparation for multiple rehash Herbert Xu
2015-03-13  9:57 ` [PATCH 1/6] rhashtable: Fix walker behaviour during rehash Herbert Xu
2015-03-13 15:50   ` Thomas Graf
2015-03-13 23:42     ` Herbert Xu
2015-03-14  0:06       ` Thomas Graf
2015-03-13  9:57 ` [PATCH 2/6] rhashtable: Use SINGLE_DEPTH_NESTING Herbert Xu
2015-03-13 15:40   ` Thomas Graf
2015-03-13  9:57 ` [PATCH 3/6] rhashtable: Move seed init into bucket_table_alloc Herbert Xu
2015-03-13 10:03   ` Daniel Borkmann
2015-03-13 11:33   ` David Laight
2015-03-13 11:40     ` Herbert Xu
2015-03-13 15:40   ` Thomas Graf
2015-03-13  9:57 ` [PATCH 4/6] rhashtable: Free bucket tables asynchronously after rehash Herbert Xu
2015-03-13 15:42   ` Thomas Graf
2015-03-13  9:57 ` [PATCH 5/6] rhashtable: Add rehash counter to bucket_table Herbert Xu
2015-03-13 13:51   ` Thomas Graf
2015-03-14  2:49     ` Herbert Xu
2015-03-13  9:57 ` [PATCH 6/6] rhashtable: Move future_tbl into struct bucket_table Herbert Xu
2015-03-13 16:13   ` Thomas Graf
2015-03-13 13:57 ` [PATCH 0/6] rhashtable: Fixes + cleanups + preparation for multiple rehash Thomas Graf
2015-03-13 16:25 ` David Miller
2015-03-14  2:51   ` Herbert Xu
2015-03-14  2:53 ` [v2 PATCH " Herbert Xu
2015-03-14  2:57   ` [v2 PATCH 1/6] rhashtable: Fix walker behaviour during rehash Herbert Xu
2015-03-14  2:57   ` [v2 PATCH 2/6] rhashtable: Use SINGLE_DEPTH_NESTING Herbert Xu
2015-03-14  2:57   ` [v2 PATCH 3/6] rhashtable: Move seed init into bucket_table_alloc Herbert Xu
2015-03-14  2:57   ` [v2 PATCH 4/6] rhashtable: Free bucket tables asynchronously after rehash Herbert Xu
2015-03-14  2:57   ` [v2 PATCH 5/6] rhashtable: Add rehash counter to bucket_table Herbert Xu
2015-03-14  2:57   ` [v2 PATCH 6/6] rhashtable: Move future_tbl into struct bucket_table Herbert Xu
2015-03-15  5:36   ` [v2 PATCH 0/6] rhashtable: Fixes + cleanups + preparation for multiple rehash David Miller
2015-03-15 10:10     ` [v1 PATCH 0/6] rhashtable: Fix two bugs caused by multiple rehash preparation Herbert Xu
2015-03-15 10:12       ` [v1 PATCH 1/2] rhashtable: Fix use-after-free in rhashtable_walk_stop Herbert Xu
2015-03-15 10:12       ` Herbert Xu [this message]
2015-03-15 10:43       ` [v1 PATCH 0/14] rhashtable: Kill shift/Key netlink namespace/Merge jhash Herbert Xu
2015-03-15 10:44         ` [v1 PATCH 1/14] rhashtable: Remove shift from bucket_table Herbert Xu
2015-03-17 10:51           ` David Laight
2015-03-17 10:56             ` tgraf
2015-03-17 11:00               ` Herbert Xu
2015-03-17 11:22                 ` tgraf
2015-03-17 11:27                   ` Herbert Xu
2015-03-17 11:57                     ` tgraf
2015-03-17 12:13                       ` David Laight
2015-03-17 12:18                         ` 'tgraf@suug.ch'
2015-03-17 12:20                         ` Herbert Xu
2015-03-17 12:40                           ` 'tgraf@suug.ch'
2015-03-17 13:06                             ` David Laight
2015-03-17 21:56                             ` Herbert Xu
2015-03-18  9:51                               ` 'tgraf@suug.ch'
2015-03-18  9:55                                 ` Herbert Xu
2015-03-18 10:08                                   ` 'tgraf@suug.ch'
2015-03-18 10:12                                     ` Herbert Xu
2015-03-18 10:26                                       ` David Laight
2015-03-18 10:44                                       ` 'tgraf@suug.ch'
2015-03-17 11:22                 ` David Laight
2015-03-17 11:25                   ` Herbert Xu
2015-03-15 10:44         ` [v1 PATCH 2/14] rhashtable: Introduce max_size/min_size Herbert Xu
2015-03-15 15:12           ` Sergei Shtylyov
2015-03-15 20:21             ` Herbert Xu
2015-03-15 10:44         ` [v1 PATCH 3/14] netlink: Use rhashtable max_size instead of max_shift Herbert Xu
2015-03-15 10:44         ` [v1 PATCH 4/14] tipc: " Herbert Xu
2015-03-15 15:13           ` Sergei Shtylyov
2015-03-15 10:44         ` [v1 PATCH 5/14] test_rhashtable: " Herbert Xu
2015-03-16  3:50           ` David Miller
2015-03-15 10:44         ` [v1 PATCH 6/14] rhashtable: Remove max_shift and min_shift Herbert Xu
2015-03-15 10:44         ` [v1 PATCH 7/14] netfilter: Use rhashtable_lookup instead of lookup_compare Herbert Xu
2015-03-16  8:28           ` Thomas Graf
2015-03-16  9:14             ` Herbert Xu
2015-03-16  9:28               ` Thomas Graf
2015-03-16 11:13               ` Patrick McHardy
2015-03-20  8:55                 ` Herbert Xu
2015-03-20  9:22                   ` Patrick McHardy
2015-03-20  9:27                     ` Herbert Xu
2015-03-20  9:59                       ` Patrick McHardy
2015-03-20 10:16                         ` Herbert Xu
2015-03-20 10:27                           ` Patrick McHardy
2015-03-20 21:47                             ` Herbert Xu
2015-03-20 21:56                               ` Thomas Graf
2015-03-20 21:57                                 ` Herbert Xu
2015-03-20 22:07                                   ` Thomas Graf
2015-03-20 22:10                                     ` Herbert Xu
2015-03-20 22:23                                       ` Thomas Graf
2015-03-20 22:25                                         ` Herbert Xu
2015-03-20 22:36                                           ` Thomas Graf
2015-03-21  5:25                                             ` Patrick McHardy
2015-03-21  5:23                               ` Patrick McHardy
2015-03-20  9:36               ` Herbert Xu
2015-03-20 10:02                 ` Patrick McHardy
2015-03-15 10:44         ` [v1 PATCH 8/14] rhashtable: Fix support of objects with no accessible keys Herbert Xu
2015-03-15 10:44         ` [v1 PATCH 9/14] netlink: Move namespace into hash key Herbert Xu
2015-03-15 10:44         ` [v1 PATCH 10/14] rhashtable: Rip out obsolete compare interface Herbert Xu
2015-03-16  9:35           ` Thomas Graf
2015-03-15 10:44         ` [v1 PATCH 11/14] rhashtable: Allow hashfn to be unset Herbert Xu
2015-03-15 10:44         ` [v1 PATCH 12/14] netlink: Use default rhashtable hashfn Herbert Xu
2015-03-15 10:44         ` [v1 PATCH 13/14] tipc: " Herbert Xu
2015-03-15 10:44         ` [v1 PATCH 14/14] netfilter: " Herbert Xu
2015-03-16  4:01         ` [v1 PATCH 0/14] rhashtable: Kill shift/Key netlink namespace/Merge jhash David Miller
2015-03-16  4:18           ` Herbert Xu
2015-03-16  4:30             ` David Miller
2015-03-16  4:33               ` Herbert Xu
2015-03-16  4:40                 ` David Miller
2015-03-16 11:26                   ` Herbert Xu
2015-03-16 20:25                     ` David Miller
2015-03-18  9:01         ` [v2 PATCH 1/6] rhashtable: Remove shift from bucket_table Herbert Xu
2015-03-18  9:01         ` [v2 PATCH 2/6] rhashtable: Introduce max_size/min_size Herbert Xu
2015-03-18 10:55           ` Thomas Graf
2015-03-18 16:47             ` David Miller
2015-03-18 16:51             ` David Laight
2015-03-18  9:01         ` [v2 PATCH 3/6] netlink: Use rhashtable max_size instead of max_shift Herbert Xu
2015-03-18  9:01         ` [v2 PATCH 4/6] tipc: Use rhashtable max/min_size instead of max/min_shift Herbert Xu
2015-03-18  9:01         ` [v2 PATCH 5/6] test_rhashtable: Use rhashtable max_size instead of max_shift Herbert Xu
2015-03-18  9:01         ` [v2 PATCH 6/6] rhashtable: Remove max_shift and min_shift Herbert Xu
2015-03-15 10:43       ` [v1 PATCH 0/6] rhashtable: Fix two bugs caused by multiple rehash preparation Herbert Xu
2015-03-16  2:23       ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E1YX5Wf-0005aA-JC@gondolin.me.apana.org.au \
    --to=herbert@gondor.apana.org.au \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=tgraf@suug.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.