Skip to contentSkip to conversation

rhashtable rehashing fixes

PATCH In review Thomas Graf 3 participants 5 messages Apr 22, 2015

Subsystems (4)
Networking Network drivers Library code Rhashtable
lore
Share & tools
lore

Thomas Graf

author maintainer
#0 lore raw

v2 cover rhashtable rehashing fixes

Some rhashtable rehashing bugs found while testing with the next rhashtable self-test queued up for the next devel cycle:

https://github.com/tgraf/net-next/commits/rht

v2:
- Moved schedule_work() call into rhashtable_insert_rehash()

Thomas Graf (2):
rhashtable: Schedule async resize when sync realloc fails
rhashtable: Do not schedule more than one rehash if we can't grow
further

 lib/rhashtable.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
Signature · 1 line
2.3.5
author maintainer
#1 lore raw

PATCH v2 1/2 rhashtable: Schedule async resize when sync realloc fails

When rhashtable_insert_rehash() fails with ENOMEM, this indicates that we can't allocate the necessary memory in the current context but the limits as set by the user would still allow to grow.

Thus attempt an async resize in the background where we can allocate using GFP_KERNEL which is more likely to succeed. The insertion itself will still fail to indicate pressure.

This fixes a bug where the table would never continue growing once the utilization is above 100%.

Fixes ccd57b1bd324 ("rhashtable: Add immediate rehash during insertion")
Signed-off-by Thomas Graf <[email protected]>
---
 lib/rhashtable.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
lib/rhashtable.c +6 -1
index 4898442..f648cfd 100644
@@ -410,8 +410,13 @@ int rhashtable_insert_rehash(struct rhashtable *ht)
 		return -EBUSY;
 
 	new_tbl = bucket_table_alloc(ht, size, GFP_ATOMIC);
-	if (new_tbl == NULL)
+	if (new_tbl == NULL) {
+		/* Schedule async resize/rehash to try allocation
+		 * non-atomic context.
+		 */
+		schedule_work(&ht->run_work);
 		return -ENOMEM;
+	}
 
 	err = rhashtable_rehash_attach(ht, tbl, new_tbl);
 	if (err) {
Signature · 1 line
2.3.5
maintainer Acked-by on v2 1/2
#2 lore raw

Re: [PATCH net 1/2] rhashtable: Schedule async resize when sync realloc fails

Quoted 8 lines · Thomas Graf

When rhashtable_insert_rehash() fails with ENOMEM, this indicates that we can't allocate the necessary memory in the current context but the limits as set by the user would still allow to grow.

Thus attempt an async resize in the background where we can allocate using GFP_KERNEL which is more likely to succeed. The insertion itself will still fail to indicate pressure.

Quoted 5 lines

This fixes a bug where the table would never continue growing once the utilization is above 100%.

Fixes ccd57b1bd324 ("rhashtable: Add immediate rehash during insertion")
Signed-off-by Thomas Graf <[email protected]>
Acked-by Herbert Xu <[email protected]>
Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
author maintainer
#3 lore raw

PATCH v2 2/2 rhashtable: Do not schedule more than one rehash if we can't grow further

In reply to Thomas Graf

The current code currently only stops inserting rehashes into the chain when no resizes are currently scheduled. As long as resizes are scheduled and while inserting above the utilization watermark, more and more rehashes will be scheduled.

This lead to a perfect DoS storm with thousands of rehashes scheduled which lead to thousands of spinlocks to be taken sequentially.

Instead, only allow either a series of resizes or a single rehash. Drop any further rehashes and return -EBUSY.

Fixes ccd57b1bd324 ("rhashtable: Add immediate rehash during insertion")
Signed-off-by Thomas Graf <[email protected]>
Acked-by Herbert Xu <[email protected]>
---
 lib/rhashtable.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
lib/rhashtable.c +2 -2
index f648cfd..b28df40 100644
@@ -405,8 +405,8 @@ int rhashtable_insert_rehash(struct rhashtable *ht)
 
 	if (rht_grow_above_75(ht, tbl))
 		size *= 2;
-	/* More than two rehashes (not resizes) detected. */
-	else if (WARN_ON(old_tbl != tbl && old_tbl->size == size))
+	/* Do not schedule more than one rehash */
+	else if (old_tbl != tbl)
 		return -EBUSY;
 
 	new_tbl = bucket_table_alloc(ht, size, GFP_ATOMIC);
Signature · 1 line
2.3.5