Re: [Patch v2] lib: test get_count_order/long in test_bitops.c

[Wei Yang <richard.weiyang@gmail.com>]

On Fri, Jun 05, 2020 at 05:16:29PM -0700, Andrew Morton wrote:
>On Fri, 5 Jun 2020 23:06:10 +0000 Wei Yang <richard.weiyang@gmail.com> wrote:
>
>> On Thu, Jun 04, 2020 at 02:51:40PM +0200, Geert Uytterhoeven wrote:
>> >Hi Wei,
>> >
>> >On Thu, Jun 4, 2020 at 2:28 PM Wei Yang <richard.weiyang@gmail.com> wrote:
>> [...]
>> >>
>> >> You mean
>> >>
>> >>        {0x0000000000000003,  2},
>> >>        {0x0000000000000004,  2},
>> >>        {0x0000000000001fff, 13},
>> >>        {0x0000000000002000, 13},
>> >>        {0x0000000050000000, 31},
>> >>        {0x0000000080000000, 31},
>> >>        {0x0000000080003000, 32},
>> >
>> >Yes, those values.  And those should work with get_count_order_long()
>> >on both 32-bit and 64-bit.
>
>Geert meant "no, the values in order_comb[]" :)
>
>We have a table of numbers in order_comb[] so we may as well feed them
>into get_count_order_long() (as well as get_count_order()) just for a
>bit more testing.
>
>So how about the below?  order_comb_long[] just can't be used on 32-bit
>machines because their longs are 32-bit.  If we had a
>get_count_order_u64() then we could use it.
>
>I haven't runtime tested this - could you please do so?
>
>
>static unsigned int order_comb[][2] = {
>	{0x00000003,  2},
>	{0x00000004,  2},
>	{0x00001fff, 13},
>	{0x00002000, 13},
>	{0x50000000, 31},
>	{0x80000000, 31},
>	{0x80003000, 32},
>};
>
>#ifdef CONFIG_64BIT
>static unsigned long order_comb_long[][2] = {
>	{0x0000000300000000, 34},
>	{0x0000000400000000, 34},
>	{0x00001fff00000000, 45},
>	{0x0000200000000000, 45},
>	{0x5000000000000000, 63},
>	{0x8000000000000000, 63},
>	{0x8000300000000000, 64},
>};
>#endif
>
>static int __init test_bitops_startup(void)
>{
>	int i;
>
>	pr_warn("Loaded test module\n");
>	set_bit(BITOPS_4, g_bitmap);
>	set_bit(BITOPS_7, g_bitmap);
>	set_bit(BITOPS_11, g_bitmap);
>	set_bit(BITOPS_31, g_bitmap);
>	set_bit(BITOPS_88, g_bitmap);
>
>	for (i = 0; i < ARRAY_SIZE(order_comb); i++) {
>		if (order_comb[i][1] != get_count_order(order_comb[i][0]))
>			pr_warn("get_count_order wrong for %x\n",
>				       order_comb[i][0]);
>	}
>
>	for (i = 0; i < ARRAY_SIZE(order_comb); i++) {
>		if (order_comb[i][1] != get_count_order_long(order_comb[i][0]))
>			pr_warn("get_count_order_long wrong for %x\n",
>				       order_comb[i][0]);
>	}
>
>#ifdef CONFIG_64BIT
>	for (i = 0; i < ARRAY_SIZE(order_comb); i++) {
>		if (order_comb_long[i][1] !=
>			       get_count_order_long(order_comb_long[i][0]))
>			pr_warn("get_count_order_long wrong for %lx\n",
>				       order_comb_long[i][0]);
>	}
>#endif
>	return 0;
>}
>
>
>From: Andrew Morton <akpm@linux-foundation.org>
>Subject: lib-test-get_count_order-long-in-test_bitopsc-fix
>
>Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>Cc: Christian Brauner <christian.brauner@ubuntu.com>
>Cc: Wei Yang <richard.weiyang@gmail.com>
>Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>---
>
> lib/test_bitops.c |   23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
>--- a/lib/test_bitops.c~lib-test-get_count_order-long-in-test_bitopsc-fix
>+++ a/lib/test_bitops.c
>@@ -28,7 +28,7 @@ enum bitops_fun {
> 
> static DECLARE_BITMAP(g_bitmap, BITOPS_LENGTH);
> 
>-unsigned int order_comb[][2] = {
>+static unsigned int order_comb[][2] = {
> 	{0x00000003,  2},
> 	{0x00000004,  2},
> 	{0x00001fff, 13},
>@@ -38,7 +38,8 @@ unsigned int order_comb[][2] = {
> 	{0x80003000, 32},
> };
> 
>-unsigned long order_comb_long[][2] = {
>+#ifdef CONFIG_64BIT
>+static unsigned long order_comb_long[][2] = {
> 	{0x0000000300000000, 34},
> 	{0x0000000400000000, 34},
> 	{0x00001fff00000000, 45},
>@@ -47,6 +48,7 @@ unsigned long order_comb_long[][2] = {
> 	{0x8000000000000000, 63},
> 	{0x8000300000000000, 64},
> };
>+#endif
> 
> static int __init test_bitops_startup(void)
> {
>@@ -62,14 +64,23 @@ static int __init test_bitops_startup(vo
> 	for (i = 0; i < ARRAY_SIZE(order_comb); i++) {
> 		if (order_comb[i][1] != get_count_order(order_comb[i][0]))
> 			pr_warn("get_count_order wrong for %x\n",
>-				       order_comb[i][0]); }
>+				       order_comb[i][0]);
>+	}
> 
>-	for (i = 0; i < ARRAY_SIZE(order_comb_long); i++) {
>+	for (i = 0; i < ARRAY_SIZE(order_comb); i++) {
>+		if (order_comb[i][1] != get_count_order_long(order_comb[i][0]))
>+			pr_warn("get_count_order_long wrong for %x\n",
>+				       order_comb[i][0]);
>+	}
>+
>+#ifdef CONFIG_64BIT
>+	for (i = 0; i < ARRAY_SIZE(order_comb); i++) {
                                   ^
I am afraid this one should be order_comb_long?

The test on 64bit machine pass. Since I don't have a 32bit machine by hand, 
Geert, would you mind have a try on 32bit machine? 

> 		if (order_comb_long[i][1] !=
> 			       get_count_order_long(order_comb_long[i][0]))
> 			pr_warn("get_count_order_long wrong for %lx\n",
>-				       order_comb_long[i][0]); }
>-
>+				       order_comb_long[i][0]);
>+	}
>+#endif
> 	return 0;
> }
> 
>_

-- 
Wei Yang
Help you, Help me