All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rhashtable: Always allocate at least one bucket lock
@ 2015-12-15 22:45 William Hua
  2015-12-16  5:53 ` Herbert Xu
  0 siblings, 1 reply; 5+ messages in thread
From: William Hua @ 2015-12-15 22:45 UTC (permalink / raw)
  To: netdev, Thomas Graf; +Cc: William Hua

No bucket locks are allocated when an rhashtable is initialized with
fewer than two elements. In this special case, we should allocate at
least one to prevent a segfault.

Signed-off-by: William Hua <william.hua@canonical.com>
---
 lib/rhashtable.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index a54ff89..1b1b5b2 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -76,6 +76,9 @@ static int alloc_bucket_locks(struct rhashtable *ht, struct bucket_table *tbl,
 	/* Never allocate more than 0.5 locks per bucket */
 	size = min_t(unsigned int, size, tbl->size >> 1);
 
+	/* Always allocate at least one lock */
+	size = max_t(unsigned int, size, 1);
+
 	if (sizeof(spinlock_t) != 0) {
 #ifdef CONFIG_NUMA
 		if (size * sizeof(spinlock_t) > PAGE_SIZE &&
-- 
2.6.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] rhashtable: Always allocate at least one bucket lock
  2015-12-15 22:45 [PATCH] rhashtable: Always allocate at least one bucket lock William Hua
@ 2015-12-16  5:53 ` Herbert Xu
  2015-12-16  7:33   ` William Hua
  0 siblings, 1 reply; 5+ messages in thread
From: Herbert Xu @ 2015-12-16  5:53 UTC (permalink / raw)
  To: William Hua; +Cc: netdev, tgraf, william.hua

William Hua <william.hua@canonical.com> wrote:
> No bucket locks are allocated when an rhashtable is initialized with
> fewer than two elements. In this special case, we should allocate at
> least one to prevent a segfault.
> 
> Signed-off-by: William Hua <william.hua@canonical.com>

Huh? The minimum hash table size is 4.  How are you getting an
rhashtable that's smaller than the required minimum?
-- 
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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] rhashtable: Always allocate at least one bucket lock
  2015-12-16  5:53 ` Herbert Xu
@ 2015-12-16  7:33   ` William Hua
  2015-12-16 10:13     ` rhashtable: Enforce minimum size on initial hash table Herbert Xu
  0 siblings, 1 reply; 5+ messages in thread
From: William Hua @ 2015-12-16  7:33 UTC (permalink / raw)
  To: Herbert Xu; +Cc: netdev, tgraf

On 12/16/2015 12:53 AM, Herbert Xu wrote:
> William Hua <william.hua@canonical.com> wrote:
>> No bucket locks are allocated when an rhashtable is initialized with
>> fewer than two elements. In this special case, we should allocate at
>> least one to prevent a segfault.
>>
>> Signed-off-by: William Hua <william.hua@canonical.com>
> 
> Huh? The minimum hash table size is 4.  How are you getting an
> rhashtable that's smaller than the required minimum?

I wasn't aware there was an enforced minimum size. I simply set the 
nelem_hint in the rhastable_params struct to 1, expecting it to grow as
needed. This caused a segfault afterwards when trying to insert an
element.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* rhashtable: Enforce minimum size on initial hash table
  2015-12-16  7:33   ` William Hua
@ 2015-12-16 10:13     ` Herbert Xu
  2015-12-16 15:45       ` David Miller
  0 siblings, 1 reply; 5+ messages in thread
From: Herbert Xu @ 2015-12-16 10:13 UTC (permalink / raw)
  To: William Hua; +Cc: netdev, tgraf

William Hua <william.hua@canonical.com> wrote:
>
> I wasn't aware there was an enforced minimum size. I simply set the 
> nelem_hint in the rhastable_params struct to 1, expecting it to grow as
> needed. This caused a segfault afterwards when trying to insert an
> element.

OK we're doing the size computation before we enforce the limit
on min_size.

---8<---
We need to do the initial hash table size computation after we
have obtained the correct min_size/max_size parameters.  Otherwise
we may end up with a hash table whose size is outside the allowed
envelope.

Fixes: a998f712f77e ("rhashtable: Round up/down min/max_size to...")
Reported-by: William Hua <william.hua@canonical.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 1c624db..9a40963 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -739,9 +737,6 @@ int rhashtable_init(struct rhashtable *ht,
 	if (params->nulls_base && params->nulls_base < (1U << RHT_BASE_SHIFT))
 		return -EINVAL;
 
-	if (params->nelem_hint)
-		size = rounded_hashtable_size(params);
-
 	memset(ht, 0, sizeof(*ht));
 	mutex_init(&ht->mutex);
 	spin_lock_init(&ht->lock);
@@ -761,6 +756,9 @@ int rhashtable_init(struct rhashtable *ht,
 
 	ht->p.min_size = max(ht->p.min_size, HASH_MIN_SIZE);
 
+	if (params->nelem_hint)
+		size = rounded_hashtable_size(&ht->p);
+
 	/* The maximum (not average) chain length grows with the
 	 * size of the hash table, at a rate of (log N)/(log log N).
 	 * The value of 16 is selected so that even if the hash
-- 
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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: rhashtable: Enforce minimum size on initial hash table
  2015-12-16 10:13     ` rhashtable: Enforce minimum size on initial hash table Herbert Xu
@ 2015-12-16 15:45       ` David Miller
  0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2015-12-16 15:45 UTC (permalink / raw)
  To: herbert; +Cc: william.hua, netdev, tgraf

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Wed, 16 Dec 2015 18:13:14 +0800

> William Hua <william.hua@canonical.com> wrote:
>>
>> I wasn't aware there was an enforced minimum size. I simply set the 
>> nelem_hint in the rhastable_params struct to 1, expecting it to grow as
>> needed. This caused a segfault afterwards when trying to insert an
>> element.
> 
> OK we're doing the size computation before we enforce the limit
> on min_size.
> 
> ---8<---
> We need to do the initial hash table size computation after we
> have obtained the correct min_size/max_size parameters.  Otherwise
> we may end up with a hash table whose size is outside the allowed
> envelope.
> 
> Fixes: a998f712f77e ("rhashtable: Round up/down min/max_size to...")
> Reported-by: William Hua <william.hua@canonical.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied and queued up for -stable, thanks!

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-12-16 15:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-15 22:45 [PATCH] rhashtable: Always allocate at least one bucket lock William Hua
2015-12-16  5:53 ` Herbert Xu
2015-12-16  7:33   ` William Hua
2015-12-16 10:13     ` rhashtable: Enforce minimum size on initial hash table Herbert Xu
2015-12-16 15:45       ` David Miller

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.