On Mon, Sep 27, 2021 at 6:33 AM Isabella Basso isabellabdoamaral@usp.br wrote:
Split up test_hash_init so that it calls each test more explicitly insofar it is possible without rewriting the entire file. This aims at improving readability.
Split tests performed on string_or as they don't interfere with those performed in hash_or. Also separate pr_info calls about skipped tests as they're not part of the tests themselves, but only warn about (un)defined arch-specific hash functions.
Changes since v1:
- As suggested by David Gow:
- Rename arch-specific test functions.
- Remove spare whitespace changes.
- As suggested by Marco Elver:
- Add struct for carrying test variables.
Nit: Move the changelog to after the "---" (and the correct patch).
Tested-by: David Gow davidgow@google.com Signed-off-by: Isabella Basso isabellabdoamaral@usp.br
This seems good to me, though I admit this is the part I'm probably least knowledgeable about. I'm pretty sure there has to be a more straightforward way to test some of these hash functions, but it's probably better to keep this as-is rather than doing anything too drastic in the middle of the port to KUnit.
The biggest downside here is that we now double the number of calls to fill_buffer() and full_name_hash(), so the test is likely to be a bit slower. It still runs fast enough (at least with the default SIZE of 256) that it's not noticeable to me, though, so I don't think it's a problem.
Apart from Marco's comment about the changelog in the commit message is fixed, this is:
Reviewed-by: David Gow davidgow@google.com
Cheers, -- David
lib/test_hash.c | 66 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 12 deletions(-)
diff --git a/lib/test_hash.c b/lib/test_hash.c index 08fe63776c4f..db9dd18b4e8b 100644 --- a/lib/test_hash.c +++ b/lib/test_hash.c @@ -153,11 +153,39 @@ test_int_hash(unsigned long long h64, u32 hash_or[2][33])
#define SIZE 256 /* Run time is cubic in SIZE */
-static int __init -test_hash_init(void) +static int __init test_string_or(void) { char buf[SIZE+1];
u32 string_or = 0, hash_or[2][33] = { { 0, } };
u32 string_or = 0;int i, j;fill_buf(buf, SIZE, 1);/* Test every possible non-empty substring in the buffer. */for (j = SIZE; j > 0; --j) {buf[j] = '\0';for (i = 0; i <= j; i++) {u32 h0 = full_name_hash(buf+i, buf+i, j-i);string_or |= h0;} /* i */} /* j *//* The OR of all the hash values should cover all the bits */if (~string_or) {pr_err("OR of all string hash results = %#x != %#x",string_or, -1u);return -EINVAL;}return 0;+}
+static int __init test_hash_or(void) +{
char buf[SIZE+1];u32 hash_or[2][33] = { { 0, } }; unsigned tests = 0; unsigned long long h64 = 0; int i, j;@@ -187,7 +215,6 @@ test_hash_init(void) return -EINVAL; }
string_or |= h0; h64 = h64 << 32 | h0; /* For use with hash_64 */ if (!test_int_hash(h64, hash_or)) return -EINVAL;@@ -195,12 +222,6 @@ test_hash_init(void) } /* i */ } /* j */
/* The OR of all the hash values should cover all the bits */if (~string_or) {pr_err("OR of all string hash results = %#x != %#x",string_or, -1u);return -EINVAL;} if (~hash_or[0][0]) { pr_err("OR of all __hash_32 results = %#x != %#x", hash_or[0][0], -1u);@@ -232,6 +253,13 @@ test_hash_init(void) } }
pr_notice("%u tests passed.", tests);return 0;+}
+static void __init notice_skipped_tests(void) +{ /* Issue notices about skipped tests. */ #ifdef HAVE_ARCH__HASH_32 #if HAVE_ARCH__HASH_32 != 1 @@ -247,10 +275,24 @@ test_hash_init(void) #else pr_info("hash_64() has no arch implementation to test."); #endif +}
pr_notice("%u tests passed.", tests);+static int __init +test_hash_init(void) +{
int ret;
return 0;
ret = test_string_or();if (ret < 0)return ret;ret = test_hash_or();if (ret < 0)return ret;notice_skipped_tests();return ret;}
static void __exit test_hash_exit(void)
2.33.0