[PATCH 0/2 net-next v2] Minor rhashtable fixes

[PATCH 0/2 net-next v2] Minor rhashtable fixes

[Thomas Graf]

Thomas Graf (2):
  rhashtable: Avoid calculating hash again to unlock
  rhashtable: Annotate RCU locking of walkers

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

---
v2: Dropped patch 2

[PATCH 1/2 net-next] rhashtable: Avoid calculating hash again to unlock

[Thomas Graf]

Caching the lock pointer avoids having to hash on the object
again to unlock the bucket locks.

Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
 lib/rhashtable.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index c523d3a..e396d7e 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -384,14 +384,16 @@ static bool __rhashtable_insert(struct rhashtable *ht, struct rhash_head *obj,
 	struct rhash_head *head;
 	bool no_resize_running;
 	unsigned hash;
+	spinlock_t *old_lock;
 	bool success = true;
 
 	rcu_read_lock();
 
 	old_tbl = rht_dereference_rcu(ht->tbl, ht);
 	hash = head_hashfn(ht, old_tbl, obj);
+	old_lock = bucket_lock(old_tbl, hash);
 
-	spin_lock_bh(bucket_lock(old_tbl, hash));
+	spin_lock_bh(old_lock);
 
 	/* Because we have already taken the bucket lock in old_tbl,
 	 * if we find that future_tbl is not yet visible then that
@@ -428,13 +430,10 @@ static bool __rhashtable_insert(struct rhashtable *ht, struct rhash_head *obj,
 		schedule_work(&ht->run_work);
 
 exit:
-	if (tbl != old_tbl) {
-		hash = head_hashfn(ht, tbl, obj);
+	if (tbl != old_tbl)
 		spin_unlock(bucket_lock(tbl, hash));
-	}
 
-	hash = head_hashfn(ht, old_tbl, obj);
-	spin_unlock_bh(bucket_lock(old_tbl, hash));
+	spin_unlock_bh(old_lock);
 
 	rcu_read_unlock();
 
-- 
1.9.3

[PATCH 2/2 net-next] rhashtable: Annotate RCU locking of walkers

[Thomas Graf]

Fixes the following sparse warnings:

lib/rhashtable.c:767:5: warning: context imbalance in 'rhashtable_walk_start' - wrong count at exit
lib/rhashtable.c:849:6: warning: context imbalance in 'rhashtable_walk_stop' - unexpected unlock

Fixes: f2dba9c6ff0d ("rhashtable: Introduce rhashtable_walk_*")
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
 lib/rhashtable.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index e396d7e..09a7ada 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -759,6 +759,7 @@ EXPORT_SYMBOL_GPL(rhashtable_walk_exit);
  * by calling rhashtable_walk_next.
  */
 int rhashtable_walk_start(struct rhashtable_iter *iter)
+	__acquires(RCU)
 {
 	struct rhashtable *ht = iter->ht;
 
@@ -846,6 +847,7 @@ EXPORT_SYMBOL_GPL(rhashtable_walk_next);
  * Finish a hash table walk.
  */
 void rhashtable_walk_stop(struct rhashtable_iter *iter)
+	__releases(RCU)
 {
 	struct rhashtable *ht;
 	struct bucket_table *tbl = iter->walker->tbl;
-- 
1.9.3

Re: [PATCH 1/2 net-next] rhashtable: Avoid calculating hash again to unlock

[David Miller]

From: Thomas Graf <tgraf@suug.ch>
Date: Mon, 16 Mar 2015 10:42:26 +0100

> Caching the lock pointer avoids having to hash on the object
> again to unlock the bucket locks.
> 
> Signed-off-by: Thomas Graf <tgraf@suug.ch>

I'd like to hold off on this.

My understanding is that Herbert plans to expand the table size
synchonously during insert in emergency situations, and in that regime
some of these optimization won't be valid.

Re: [PATCH 2/2 net-next] rhashtable: Annotate RCU locking of walkers

[David Miller]

From: Thomas Graf <tgraf@suug.ch>
Date: Mon, 16 Mar 2015 10:42:27 +0100

> Fixes the following sparse warnings:
> 
> lib/rhashtable.c:767:5: warning: context imbalance in 'rhashtable_walk_start' - wrong count at exit
> lib/rhashtable.c:849:6: warning: context imbalance in 'rhashtable_walk_stop' - unexpected unlock
> 
> Fixes: f2dba9c6ff0d ("rhashtable: Introduce rhashtable_walk_*")
> Signed-off-by: Thomas Graf <tgraf@suug.ch>

Applied, thanks.

Re: [PATCH 1/2 net-next] rhashtable: Avoid calculating hash again to unlock

[Herbert Xu]

On Mon, Mar 16, 2015 at 04:22:02PM -0400, David Miller wrote:
> From: Thomas Graf <tgraf@suug.ch>
> Date: Mon, 16 Mar 2015 10:42:26 +0100
> 
> > Caching the lock pointer avoids having to hash on the object
> > again to unlock the bucket locks.
> > 
> > Signed-off-by: Thomas Graf <tgraf@suug.ch>
> 
> I'd like to hold off on this.
> 
> My understanding is that Herbert plans to expand the table size
> synchonously during insert in emergency situations, and in that regime
> some of these optimization won't be valid.

Actually this patch does not conflict with my multiple rehashing
work (the final scheme I used only requires two locks) so please
feel free to merge it and I will rebase my stuff.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 1/2 net-next] rhashtable: Avoid calculating hash again to unlock

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 17 Mar 2015 07:48:36 +1100

> On Mon, Mar 16, 2015 at 04:22:02PM -0400, David Miller wrote:
>> From: Thomas Graf <tgraf@suug.ch>
>> Date: Mon, 16 Mar 2015 10:42:26 +0100
>> 
>> > Caching the lock pointer avoids having to hash on the object
>> > again to unlock the bucket locks.
>> > 
>> > Signed-off-by: Thomas Graf <tgraf@suug.ch>
>> 
>> I'd like to hold off on this.
>> 
>> My understanding is that Herbert plans to expand the table size
>> synchonously during insert in emergency situations, and in that regime
>> some of these optimization won't be valid.
> 
> Actually this patch does not conflict with my multiple rehashing
> work (the final scheme I used only requires two locks) so please
> feel free to merge it and I will rebase my stuff.

Ok, done, thanks.