linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] lib/test_bitops: Do the full test during module init
@ 2020-07-06 11:29 Geert Uytterhoeven
  2020-07-06 11:34 ` Andy Shevchenko
  0 siblings, 1 reply; 3+ messages in thread
From: Geert Uytterhoeven @ 2020-07-06 11:29 UTC (permalink / raw)
  To: Wei Yang, Jesse Brandeburg, Andrew Morton
  Cc: Andy Shevchenko, linux-kernel, 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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] lib/test_bitops: Do the full test during module init
  2020-07-06 11:29 [PATCH v2] lib/test_bitops: Do the full test during module init Geert Uytterhoeven
@ 2020-07-06 11:34 ` Andy Shevchenko
  2020-07-06 11:41   ` Geert Uytterhoeven
  0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2020-07-06 11:34 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Wei Yang, Jesse Brandeburg, Andrew Morton, linux-kernel

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



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] lib/test_bitops: Do the full test during module init
  2020-07-06 11:34 ` Andy Shevchenko
@ 2020-07-06 11:41   ` Geert Uytterhoeven
  0 siblings, 0 replies; 3+ messages in thread
From: Geert Uytterhoeven @ 2020-07-06 11:41 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Wei Yang, Jesse Brandeburg, Andrew Morton, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-07-06 11:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-06 11:29 [PATCH v2] lib/test_bitops: Do the full test during module init Geert Uytterhoeven
2020-07-06 11:34 ` Andy Shevchenko
2020-07-06 11:41   ` Geert Uytterhoeven

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox