[PATCH v2] lib/test_bitops: Do the full test during module init

[PATCH v2] lib/test_bitops: Do the full test during module init

[Geert Uytterhoeven]

Currently, the bitops test consists of two parts: one part is executed
during module load, the second part during module unload. This is
cumbersome for the user, as he has to perform two steps to execute all
tests, and is different from most (all?) other tests.

Merge the two parts, so both are executed during module load.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
  - Keep a dummy module_exit() function to support module unloading.
---
 lib/test_bitops.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/lib/test_bitops.c b/lib/test_bitops.c
index ced25e3a779baf96..471141ddd6913724 100644
--- a/lib/test_bitops.c
+++ b/lib/test_bitops.c
@@ -52,9 +52,9 @@ static unsigned long order_comb_long[][2] = {
 
 static int __init test_bitops_startup(void)
 {
-	int i;
+	int i, bit_set;
 
-	pr_warn("Loaded test module\n");
+	pr_info("Starting bitops test\n");
 	set_bit(BITOPS_4, g_bitmap);
 	set_bit(BITOPS_7, g_bitmap);
 	set_bit(BITOPS_11, g_bitmap);
@@ -81,12 +81,8 @@ static int __init test_bitops_startup(void)
 				       order_comb_long[i][0]);
 	}
 #endif
-	return 0;
-}
 
-static void __exit test_bitops_unstartup(void)
-{
-	int bit_set;
+	barrier();
 
 	clear_bit(BITOPS_4, g_bitmap);
 	clear_bit(BITOPS_7, g_bitmap);
@@ -98,7 +94,13 @@ static void __exit test_bitops_unstartup(void)
 	if (bit_set != BITOPS_LAST)
 		pr_err("ERROR: FOUND SET BIT %d\n", bit_set);
 
-	pr_warn("Unloaded test module\n");
+	pr_info("Completed bitops test\n");
+
+	return 0;
+}
+
+static void __exit test_bitops_unstartup(void)
+{
 }
 
 module_init(test_bitops_startup);
-- 
2.17.1

Re: [PATCH v2] lib/test_bitops: Do the full test during module init

[Andy Shevchenko]

On Mon, Jul 06, 2020 at 01:29:00PM +0200, Geert Uytterhoeven wrote:
> Currently, the bitops test consists of two parts: one part is executed
> during module load, the second part during module unload. This is
> cumbersome for the user, as he has to perform two steps to execute all
> tests, and is different from most (all?) other tests.
> 
> Merge the two parts, so both are executed during module load.

I think it's right way to go, sorry, I didn't notice this during module
submission.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

One question though, is compiler barrier enough to prevent potential ordering issues?

> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> v2:
>   - Keep a dummy module_exit() function to support module unloading.
> ---
>  lib/test_bitops.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/test_bitops.c b/lib/test_bitops.c
> index ced25e3a779baf96..471141ddd6913724 100644
> --- a/lib/test_bitops.c
> +++ b/lib/test_bitops.c
> @@ -52,9 +52,9 @@ static unsigned long order_comb_long[][2] = {
>  
>  static int __init test_bitops_startup(void)
>  {
> -	int i;
> +	int i, bit_set;
>  
> -	pr_warn("Loaded test module\n");
> +	pr_info("Starting bitops test\n");
>  	set_bit(BITOPS_4, g_bitmap);
>  	set_bit(BITOPS_7, g_bitmap);
>  	set_bit(BITOPS_11, g_bitmap);
> @@ -81,12 +81,8 @@ static int __init test_bitops_startup(void)
>  				       order_comb_long[i][0]);
>  	}
>  #endif
> -	return 0;
> -}
>  
> -static void __exit test_bitops_unstartup(void)
> -{
> -	int bit_set;
> +	barrier();
>  
>  	clear_bit(BITOPS_4, g_bitmap);
>  	clear_bit(BITOPS_7, g_bitmap);
> @@ -98,7 +94,13 @@ static void __exit test_bitops_unstartup(void)
>  	if (bit_set != BITOPS_LAST)
>  		pr_err("ERROR: FOUND SET BIT %d\n", bit_set);
>  
> -	pr_warn("Unloaded test module\n");
> +	pr_info("Completed bitops test\n");
> +
> +	return 0;
> +}
> +
> +static void __exit test_bitops_unstartup(void)
> +{
>  }
>  
>  module_init(test_bitops_startup);
> -- 
> 2.17.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] lib/test_bitops: Do the full test during module init

[Geert Uytterhoeven]

Hi Andy,

On Mon, Jul 6, 2020 at 1:34 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Mon, Jul 06, 2020 at 01:29:00PM +0200, Geert Uytterhoeven wrote:
> > Currently, the bitops test consists of two parts: one part is executed
> > during module load, the second part during module unload. This is
> > cumbersome for the user, as he has to perform two steps to execute all
> > tests, and is different from most (all?) other tests.
> >
> > Merge the two parts, so both are executed during module load.
>
> I think it's right way to go, sorry, I didn't notice this during module
> submission.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thanks!

> One question though, is compiler barrier enough to prevent potential ordering issues?

I think so, that's why I used barrier().
You may still be subject to CPU instruction reordering, though :-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds