diff mbox series

[Resend] rhashtable: make test actually random

Message ID 5894765.lOV4Wx5bFT@eto.sf-tec.de
State New
Headers
Series [Resend] rhashtable: make test actually random |

Commit Message

Rolf Eike Beer Oct. 21, 2022, 1:47 p.m. UTC 
  The "random rhlist add/delete operations" actually wasn't very random, as all
cases tested the same bit. Since the later parts of this loop depend on the
first case execute this unconditionally, and then test on different bits for the
remaining tests. While at it only request as much random bits as are actually
used.

Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de>
---
 lib/test_rhashtable.c | 58 ++++++++++++++++---------------------------
 1 file changed, 22 insertions(+), 36 deletions(-)

[Resend with correct from address]

Comments

Jason A. Donenfeld Oct. 22, 2022, 1:52 a.m. UTC | #1 
Hi,

On Fri, Oct 21, 2022 at 9:47 AM Rolf Eike Beer <eike-kernel@sf-tec.de> wrote:
>
> The "random rhlist add/delete operations" actually wasn't very random, as all
> cases tested the same bit. Since the later parts of this loop depend on the
> first case execute this unconditionally, and then test on different bits for the
> remaining tests. While at it only request as much random bits as are actually
> used.

Seems reasonable to me. If it's okay with Thomas, who you CC'd, I'd
like to take this through my random tree, as that'll prevent it from
conflicting with a series I have out currently:
https://lore.kernel.org/lkml/20221022014403.3881893-1-Jason@zx2c4.com/

Jason
Herbert Xu Oct. 22, 2022, 4:14 a.m. UTC | #2 
On Fri, Oct 21, 2022 at 03:47:03PM +0200, Rolf Eike Beer wrote:
> The "random rhlist add/delete operations" actually wasn't very random, as all
> cases tested the same bit. Since the later parts of this loop depend on the
> first case execute this unconditionally, and then test on different bits for the
> remaining tests. While at it only request as much random bits as are actually
> used.
> 
> Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de>
> ---
>  lib/test_rhashtable.c | 58 ++++++++++++++++---------------------------
>  1 file changed, 22 insertions(+), 36 deletions(-)

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Herbert Xu Oct. 22, 2022, 4:15 a.m. UTC | #3 
On Fri, Oct 21, 2022 at 09:52:56PM -0400, Jason A. Donenfeld wrote:
>
> Seems reasonable to me. If it's okay with Thomas, who you CC'd, I'd
> like to take this through my random tree, as that'll prevent it from
> conflicting with a series I have out currently:
> https://lore.kernel.org/lkml/20221022014403.3881893-1-Jason@zx2c4.com/

Sure, this code rarely changes so it should be fine to go through
your tree.

Thanks,
patchwork-bot+netdevbpf@kernel.org Oct. 26, 2022, 12:50 p.m. UTC | #4 
Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Fri, 21 Oct 2022 15:47:03 +0200 you wrote:
> The "random rhlist add/delete operations" actually wasn't very random, as all
> cases tested the same bit. Since the later parts of this loop depend on the
> first case execute this unconditionally, and then test on different bits for the
> remaining tests. While at it only request as much random bits as are actually
> used.
> 
> Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de>
> 
> [...]

Here is the summary with links:
  - [Resend] rhashtable: make test actually random
    https://git.kernel.org/netdev/net/c/c5f0a1728874

You are awesome, thank you!
diff mbox series

Patch

diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c
index b358a74ed7ed..f2ba5787055a 100644
--- a/lib/test_rhashtable.c
+++ b/lib/test_rhashtable.c
@@ -369,18 +369,10 @@  static int __init test_rhltable(unsigned int entries)
 	pr_info("test %d random rhlist add/delete operations\n", entries);
 	for (j = 0; j < entries; j++) {
 		u32 i = prandom_u32_max(entries);
-		u32 prand = get_random_u32();
+		u32 prand = prandom_u32_max(4);
 
 		cond_resched();
 
-		if (prand == 0)
-			prand = get_random_u32();
-
-		if (prand & 1) {
-			prand >>= 1;
-			continue;
-		}
-
 		err = rhltable_remove(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
 		if (test_bit(i, obj_in_table)) {
 			clear_bit(i, obj_in_table);
@@ -393,35 +385,29 @@  static int __init test_rhltable(unsigned int entries)
 		}
 
 		if (prand & 1) {
-			prand >>= 1;
-			continue;
-		}
-
-		err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
-		if (err == 0) {
-			if (WARN(test_and_set_bit(i, obj_in_table), "succeeded to insert same object %d", i))
-				continue;
-		} else {
-			if (WARN(!test_bit(i, obj_in_table), "failed to insert object %d", i))
-				continue;
-		}
-
-		if (prand & 1) {
-			prand >>= 1;
-			continue;
+			err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
+			if (err == 0) {
+				if (WARN(test_and_set_bit(i, obj_in_table), "succeeded to insert same object %d", i))
+					continue;
+			} else {
+				if (WARN(!test_bit(i, obj_in_table), "failed to insert object %d", i))
+					continue;
+			}
 		}
 
-		i = prandom_u32_max(entries);
-		if (test_bit(i, obj_in_table)) {
-			err = rhltable_remove(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
-			WARN(err, "cannot remove element at slot %d", i);
-			if (err == 0)
-				clear_bit(i, obj_in_table);
-		} else {
-			err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
-			WARN(err, "failed to insert object %d", i);
-			if (err == 0)
-				set_bit(i, obj_in_table);
+		if (prand & 2) {
+			i = prandom_u32_max(entries);
+			if (test_bit(i, obj_in_table)) {
+				err = rhltable_remove(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
+				WARN(err, "cannot remove element at slot %d", i);
+				if (err == 0)
+					clear_bit(i, obj_in_table);
+			} else {
+				err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
+				WARN(err, "failed to insert object %d", i);
+				if (err == 0)
+					set_bit(i, obj_in_table);
+			}
 		}
 	}