Commit 565e8640 authored by Herbert Xu's avatar Herbert Xu Committed by David S. Miller

rhashtable: Fix rhashtable_remove failures

The commit 9d901bc0 ("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 c4db8848
("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: default avatarHerbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 963ecbd4
...@@ -511,28 +511,25 @@ static bool __rhashtable_remove(struct rhashtable *ht, ...@@ -511,28 +511,25 @@ static bool __rhashtable_remove(struct rhashtable *ht,
*/ */
bool rhashtable_remove(struct rhashtable *ht, struct rhash_head *obj) bool rhashtable_remove(struct rhashtable *ht, struct rhash_head *obj)
{ {
struct bucket_table *tbl, *old_tbl; struct bucket_table *tbl;
bool ret; bool ret;
rcu_read_lock(); rcu_read_lock();
old_tbl = rht_dereference_rcu(ht->tbl, ht); tbl = rht_dereference_rcu(ht->tbl, ht);
ret = __rhashtable_remove(ht, old_tbl, obj);
/* Because we have already taken (and released) the bucket /* Because we have already taken (and released) the bucket
* lock in old_tbl, if we find that future_tbl is not yet * lock in old_tbl, if we find that future_tbl is not yet
* visible then that guarantees the entry to still be in * 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; while (!(ret = __rhashtable_remove(ht, tbl, obj)) &&
if (!ret && old_tbl != tbl) (tbl = rht_dereference_rcu(tbl->future_tbl, ht)))
ret = __rhashtable_remove(ht, tbl, obj); ;
if (ret) { if (ret) {
bool no_resize_running = tbl == old_tbl;
atomic_dec(&ht->nelems); 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); schedule_work(&ht->run_work);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment