diff mbox series

[1/4] rhashtable: use cmpxchg() in nested_table_alloc()

Message ID 155416006508.9540.3368212864327714370.stgit@noble.brown
State Accepted
Delegated to: David Miller
Headers show
Series Convert rhashtable to use bitlocks | expand

Commit Message

NeilBrown April 1, 2019, 11:07 p.m. UTC
nested_table_alloc() relies on the fact that there is
at most one spinlock allocated for every slot in the top
level nested table, so it is not possible for two threads
to try to allocate the same table at the same time.

This assumption is a little fragile (it is not explicit) and is
unnecessary as cmpxchg() can be used instead.

A future patch will replace the spinlocks by per-bucket bitlocks,
and then we won't be able to protect the slot pointer with a spinlock.

So replace rcu_assign_pointer() with cmpxchg() - which has equivalent
barrier properties.
If it the cmp fails, free the table that was just allocated.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 lib/rhashtable.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Herbert Xu April 8, 2019, 2:34 a.m. UTC | #1
On Tue, Apr 02, 2019 at 10:07:45AM +1100, NeilBrown wrote:
> nested_table_alloc() relies on the fact that there is
> at most one spinlock allocated for every slot in the top
> level nested table, so it is not possible for two threads
> to try to allocate the same table at the same time.
> 
> This assumption is a little fragile (it is not explicit) and is
> unnecessary as cmpxchg() can be used instead.
> 
> A future patch will replace the spinlocks by per-bucket bitlocks,
> and then we won't be able to protect the slot pointer with a spinlock.
> 
> So replace rcu_assign_pointer() with cmpxchg() - which has equivalent
> barrier properties.
> If it the cmp fails, free the table that was just allocated.
> 
> Signed-off-by: NeilBrown <neilb@suse.com>

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
diff mbox series

Patch

diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 811d51b7cb86..6c4f5c8e9baa 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -131,9 +131,11 @@  static union nested_table *nested_table_alloc(struct rhashtable *ht,
 			INIT_RHT_NULLS_HEAD(ntbl[i].bucket);
 	}
 
-	rcu_assign_pointer(*prev, ntbl);
-
-	return ntbl;
+	if (cmpxchg(prev, NULL, ntbl) == NULL)
+		return ntbl;
+	/* Raced with another thread. */
+	kfree(ntbl);
+	return rcu_dereference(*prev);
 }
 
 static struct bucket_table *nested_bucket_table_alloc(struct rhashtable *ht,