| Message ID | 20150131102150.GA29498@gondor.apana.org.au |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On 01/31/15 at 09:21pm, Herbert Xu wrote: > Currently hash_rnd is a parameter that users can set. However, > no existing users set this parameter. It is also something that > people are unlikely to want to set directly since it's just a > random number. > > In preparation for allowing the reseeding/rehashing of rhashtable, > this patch moves hash_rnd into bucket_table so that it's now an > internal state rather than a parameter. > > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> For net-next Acked-by: Thomas Graf <tgraf@suug.ch> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Herbert Xu <herbert@gondor.apana.org.au> Date: Sat, 31 Jan 2015 21:21:50 +1100 > Currently hash_rnd is a parameter that users can set. However, > no existing users set this parameter. It is also something that > people are unlikely to want to set directly since it's just a > random number. > > In preparation for allowing the reseeding/rehashing of rhashtable, > this patch moves hash_rnd into bucket_table so that it's now an > internal state rather than a parameter. > > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> Also applied to net-next, thanks a lot. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: David Miller <davem@davemloft.net> Date: Mon, 02 Feb 2015 19:19:56 -0800 (PST) > From: Herbert Xu <herbert@gondor.apana.org.au> > Date: Sat, 31 Jan 2015 21:21:50 +1100 > >> Currently hash_rnd is a parameter that users can set. However, >> no existing users set this parameter. It is also something that >> people are unlikely to want to set directly since it's just a >> random number. >> >> In preparation for allowing the reseeding/rehashing of rhashtable, >> this patch moves hash_rnd into bucket_table so that it's now an >> internal state rather than a parameter. >> >> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> > > Also applied to net-next, thanks a lot. Actually this and the netfilter change break the build. I'm reverting all of these rhashtable changes, please fix this stuff up. Firstly, the hash_rnd change results in this: lib/rhashtable.c: In function ‘obj_raw_hashfn’: lib/rhashtable.c:84:9: warning: passing argument 1 of ‘lockdep_rht_mutex_is_held’ discards ‘const’ qualifier from pointer target type [enabled by default] lib/rhashtable.c:57:5: note: expected ‘struct rhashtable *’ but argument is of type ‘const struct rhashtable *’ Next, the netfilter change results in: ERROR: "rhashtable_walk_start" [net/netfilter/nft_hash.ko] undefined!
On Mon, Feb 02, 2015 at 07:26:34PM -0800, David Miller wrote: > > Firstly, the hash_rnd change results in this: > > lib/rhashtable.c: In function ‘obj_raw_hashfn’: > lib/rhashtable.c:84:9: warning: passing argument 1 of ‘lockdep_rht_mutex_is_held’ discards ‘const’ qualifier from pointer target type [enabled by default] > lib/rhashtable.c:57:5: note: expected ‘struct rhashtable *’ but argument is of type ‘const struct rhashtable *’ Pleas drop the hash_rnd patch since that's only needed by my rehash work which is not yet complete. Although I thought it built correctly on its own since I've always had it in my test tree but obviously I missed something. > Next, the netfilter change results in: > > ERROR: "rhashtable_walk_start" [net/netfilter/nft_hash.ko] undefined! Oops, I missed an export on that symbol. I'll fix it up and repost. Thanks,
On Wed, Feb 04, 2015 at 07:17:50AM +1100, Herbert Xu wrote: > > > ERROR: "rhashtable_walk_start" [net/netfilter/nft_hash.ko] undefined! > > Oops, I missed an export on that symbol. I'll fix it up and > repost. OK here is the repost without the hash_rnd patch. The first patch fixes a potential crash with nft_hash destroying the table during a shrinking process. While the next patch adds rhashtable iterators to replace current manual walks used by netlink and netfilter. The final two patches make use of these iterators in netlink and netfilter. Cheers,
From: Herbert Xu <herbert@gondor.apana.org.au> Date: Wed, 4 Feb 2015 07:32:13 +1100 > On Wed, Feb 04, 2015 at 07:17:50AM +1100, Herbert Xu wrote: >> >> > ERROR: "rhashtable_walk_start" [net/netfilter/nft_hash.ko] undefined! >> >> Oops, I missed an export on that symbol. I'll fix it up and >> repost. > > OK here is the repost without the hash_rnd patch. > > The first patch fixes a potential crash with nft_hash destroying > the table during a shrinking process. While the next patch adds > rhashtable iterators to replace current manual walks used by > netlink and netfilter. The final two patches make use of these > iterators in netlink and netfilter. Series applied to net-next. Please address the sparse locking warnings reported against patch #3, it's probably just some missing acquire/release annotations. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h index a2562ed..6d7e840 100644 --- a/include/linux/rhashtable.h +++ b/include/linux/rhashtable.h @@ -55,6 +55,7 @@ struct rhash_head { struct bucket_table { size_t size; unsigned int locks_mask; + u32 hash_rnd; spinlock_t *locks; struct rhash_head __rcu *buckets[]; }; @@ -70,7 +71,6 @@ struct rhashtable; * @key_len: Length of key * @key_offset: Offset of key in struct to be hashed * @head_offset: Offset of rhash_head in struct to be hashed - * @hash_rnd: Seed to use while hashing * @max_shift: Maximum number of shifts while expanding * @min_shift: Minimum number of shifts while shrinking * @nulls_base: Base value to generate nulls marker @@ -89,7 +89,6 @@ struct rhashtable_params { size_t key_len; size_t key_offset; size_t head_offset; - u32 hash_rnd; size_t max_shift; size_t min_shift; u32 nulls_base; diff --git a/lib/rhashtable.c b/lib/rhashtable.c index 84a78e3..71c6aa1 100644 --- a/lib/rhashtable.c +++ b/lib/rhashtable.c @@ -81,13 +81,14 @@ static u32 rht_bucket_index(const struct bucket_table *tbl, u32 hash) static u32 obj_raw_hashfn(const struct rhashtable *ht, const void *ptr) { + struct bucket_table *tbl = rht_dereference_rcu(ht->tbl, ht); u32 hash; if (unlikely(!ht->p.key_len)) - hash = ht->p.obj_hashfn(ptr, ht->p.hash_rnd); + hash = ht->p.obj_hashfn(ptr, tbl->hash_rnd); else hash = ht->p.hashfn(ptr + ht->p.key_offset, ht->p.key_len, - ht->p.hash_rnd); + tbl->hash_rnd); return hash >> HASH_RESERVED_SPACE; } @@ -97,7 +98,7 @@ static u32 key_hashfn(struct rhashtable *ht, const void *key, u32 len) struct bucket_table *tbl = rht_dereference_rcu(ht->tbl, ht); u32 hash; - hash = ht->p.hashfn(key, len, ht->p.hash_rnd); + hash = ht->p.hashfn(key, len, tbl->hash_rnd); hash >>= HASH_RESERVED_SPACE; return rht_bucket_index(tbl, hash); @@ -894,14 +895,13 @@ int rhashtable_init(struct rhashtable *ht, struct rhashtable_params *params) if (tbl == NULL) return -ENOMEM; + get_random_bytes(&tbl->hash_rnd, sizeof(tbl->hash_rnd)); + atomic_set(&ht->nelems, 0); atomic_set(&ht->shift, ilog2(tbl->size)); RCU_INIT_POINTER(ht->tbl, tbl); RCU_INIT_POINTER(ht->future_tbl, tbl); - if (!ht->p.hash_rnd) - get_random_bytes(&ht->p.hash_rnd, sizeof(ht->p.hash_rnd)); - if (ht->p.grow_decision || ht->p.shrink_decision) INIT_WORK(&ht->run_work, rht_deferred_worker);
Currently hash_rnd is a parameter that users can set. However, no existing users set this parameter. It is also something that people are unlikely to want to set directly since it's just a random number. In preparation for allowing the reseeding/rehashing of rhashtable, this patch moves hash_rnd into bucket_table so that it's now an internal state rather than a parameter. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>