From b7f5e5c7f8cedf6b69c9702d448cdf78ffc45c7b Mon Sep 17 00:00:00 2001 From: Daniel Borkmann Date: Fri, 20 Feb 2015 21:14:21 +0100 Subject: rhashtable: don't allocate ht structure on stack in test_rht_init With object runtime debugging enabled, the rhashtable test suite will rightfully throw a warning "ODEBUG: object is on stack, but not annotated" from rhashtable_init(). This is because run_work is (correctly) being initialized via INIT_WORK(), and not annotated by INIT_WORK_ONSTACK(). Meaning, rhashtable_init() is okay as is, we just need to move ht e.g., into global scope. It never triggered anything, since test_rhashtable is rather a controlled environment and effectively runs to completion, so that stack memory is not vanishing underneath us, we shouldn't confuse any testers with it though. Fixes: 7e1e77636e36 ("lib: Resizable, Scalable, Concurrent Hash Table") Signed-off-by: Daniel Borkmann Acked-by: Thomas Graf Signed-off-by: David S. Miller --- lib/test_rhashtable.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib/test_rhashtable.c') diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c index 1dfeba73fc74..f6ce291b68e7 100644 --- a/lib/test_rhashtable.c +++ b/lib/test_rhashtable.c @@ -191,9 +191,10 @@ error: return err; } +static struct rhashtable ht; + static int __init test_rht_init(void) { - struct rhashtable ht; struct rhashtable_params params = { .nelem_hint = TEST_HT_SIZE, .head_offset = offsetof(struct test_obj, node), -- cgit v1.2.3 From 6dd0c1655be26345960a6bae574c7dc4648611d3 Mon Sep 17 00:00:00 2001 From: Daniel Borkmann Date: Fri, 20 Feb 2015 00:53:39 +0100 Subject: rhashtable: allow to unload test module There's no good reason why to disallow unloading of the rhashtable test case module. Commit 9d6dbe1bbaf8 moved the code from a boot test into a stand-alone module, but only converted the subsys_initcall() handler into a module_init() function without a related exit handler, and thus preventing the test module from unloading. Fixes: 9d6dbe1bbaf8 ("rhashtable: Make selftest modular") Signed-off-by: Daniel Borkmann Acked-by: Thomas Graf Signed-off-by: David S. Miller --- lib/test_rhashtable.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'lib/test_rhashtable.c') diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c index f6ce291b68e7..58b995323c44 100644 --- a/lib/test_rhashtable.c +++ b/lib/test_rhashtable.c @@ -223,6 +223,11 @@ static int __init test_rht_init(void) return err; } +static void __exit test_rht_exit(void) +{ +} + module_init(test_rht_init); +module_exit(test_rht_exit); MODULE_LICENSE("GPL v2"); -- cgit v1.2.3 From 8331de75cb13fc907ceba78e698c42150e61dda9 Mon Sep 17 00:00:00 2001 From: Daniel Borkmann Date: Wed, 25 Feb 2015 16:31:53 +0100 Subject: rhashtable: unconditionally grow when max_shift is not specified While commit c0c09bfdc415 ("rhashtable: avoid unnecessary wakeup for worker queue") rightfully moved part of the decision making of whether we should expand or shrink from the expand/shrink functions themselves into insert/delete functions in order to avoid unnecessary worker wake-ups, it however introduced a regression by doing so. Before that change, if no max_shift was specified (= 0) on rhashtable initialization, rhashtable_expand() would just grow unconditionally and lets the available memory be the limiting factor. After that change, if no max_shift was specified, there would be _no_ expansion step at all. Given that netlink and tipc have a max_shift specified, it was not visible there, but Josh Hunt reported that if nft that starts out with a default element hint of 3 if not otherwise provided, would slow i.e. inserts down trememdously as it cannot grow larger to relax table occupancy. Given that the test case verifies shrinks/expands manually, we also must remove pointer to the helper functions to explicitly avoid parallel resizing on insertions/deletions. test_bucket_stats() and test_rht_lookup() could also be wrapped around rhashtable mutex to explicitly synchronize a walk from resizing, but I think that defeats the actual test case which intended to have explicit test steps, i.e. 1) inserts, 2) expands, 3) shrinks, 4) deletions, with object verification after each stage. Reported-by: Josh Hunt Fixes: c0c09bfdc415 ("rhashtable: avoid unnecessary wakeup for worker queue") Signed-off-by: Daniel Borkmann Cc: Ying Xue Cc: Josh Hunt Acked-by: Thomas Graf Signed-off-by: David S. Miller --- lib/test_rhashtable.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'lib/test_rhashtable.c') diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c index 58b995323c44..f9e9d734446a 100644 --- a/lib/test_rhashtable.c +++ b/lib/test_rhashtable.c @@ -202,8 +202,6 @@ static int __init test_rht_init(void) .key_len = sizeof(int), .hashfn = jhash, .nulls_base = (3U << RHT_BASE_SHIFT), - .grow_decision = rht_grow_above_75, - .shrink_decision = rht_shrink_below_30, }; int err; -- cgit v1.2.3 From 4c4b52d9b2df45e8216d3e30b5452e4a364d2cac Mon Sep 17 00:00:00 2001 From: Daniel Borkmann Date: Wed, 25 Feb 2015 16:31:54 +0100 Subject: rhashtable: remove indirection for grow/shrink decision functions Currently, all real users of rhashtable default their grow and shrink decision functions to rht_grow_above_75() and rht_shrink_below_30(), so that there's currently no need to have this explicitly selectable. It can/should be generic and private inside rhashtable until a real use case pops up. Since we can make this private, we'll save us this additional indirection layer and can improve insertion/deletion time as well. Reference: http://patchwork.ozlabs.org/patch/443040/ Suggested-by: David S. Miller Signed-off-by: Daniel Borkmann Acked-by: Thomas Graf Signed-off-by: David S. Miller --- lib/test_rhashtable.c | 1 + 1 file changed, 1 insertion(+) (limited to 'lib/test_rhashtable.c') diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c index f9e9d734446a..67c7593d1dd6 100644 --- a/lib/test_rhashtable.c +++ b/lib/test_rhashtable.c @@ -201,6 +201,7 @@ static int __init test_rht_init(void) .key_offset = offsetof(struct test_obj, value), .key_len = sizeof(int), .hashfn = jhash, + .max_shift = 1, /* we expand/shrink manually here */ .nulls_base = (3U << RHT_BASE_SHIFT), }; int err; -- cgit v1.2.3