Patch "paride: make 'verbose' parameter an 'int' again" has been added to the 3.14-stable tree

Patch "paride: make 'verbose' parameter an 'int' again" has been added to the 3.14-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    paride: make 'verbose' parameter an 'int' again

to the 3.14-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     paride-make-verbose-parameter-an-int-again.patch
and it can be found in the queue-3.14 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From dec63a4dec2d6d01346fd5d96062e67c0636852b Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 15 Mar 2016 14:53:29 -0700
Subject: paride: make 'verbose' parameter an 'int' again

From: Arnd Bergmann <arnd@arndb.de>

commit dec63a4dec2d6d01346fd5d96062e67c0636852b upstream.

gcc-6.0 found an ancient bug in the paride driver, which had a
"module_param(verbose, bool, 0);" since before 2.6.12, but actually uses
it to accept '0', '1' or '2' as arguments:

  drivers/block/paride/pd.c: In function 'pd_init_dev_parms':
  drivers/block/paride/pd.c:298:29: warning: comparison of constant '1' with boolean expression is always false [-Wbool-compare]
   #define DBMSG(msg) ((verbose>1)?(msg):NULL)

In 2012, Rusty did a cleanup patch that also changed the type of the
variable to 'bool', which introduced what is now a gcc warning.

This changes the type back to 'int' and adapts the module_param() line
instead, so it should work as documented in case anyone ever cares about
running the ancient driver with debugging.

Fixes: 90ab5ee94171 ("module_param: make bool parameters really bool (drivers & misc)")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Rusty Russell <rusty@rustcorp.com.au>
Cc: Tim Waugh <tim@cyberelk.net>
Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Cc: Jens Axboe <axboe@fb.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/block/paride/pd.c |    4 ++--
 drivers/block/paride/pt.c |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

--- a/drivers/block/paride/pd.c
+++ b/drivers/block/paride/pd.c
@@ -126,7 +126,7 @@
 */
 #include <linux/types.h>
 
-static bool verbose = 0;
+static int verbose = 0;
 static int major = PD_MAJOR;
 static char *name = PD_NAME;
 static int cluster = 64;
@@ -161,7 +161,7 @@ enum {D_PRT, D_PRO, D_UNI, D_MOD, D_GEO,
 static DEFINE_MUTEX(pd_mutex);
 static DEFINE_SPINLOCK(pd_lock);
 
-module_param(verbose, bool, 0);
+module_param(verbose, int, 0);
 module_param(major, int, 0);
 module_param(name, charp, 0);
 module_param(cluster, int, 0);
--- a/drivers/block/paride/pt.c
+++ b/drivers/block/paride/pt.c
@@ -117,7 +117,7 @@
 
 */
 
-static bool verbose = 0;
+static int verbose = 0;
 static int major = PT_MAJOR;
 static char *name = PT_NAME;
 static int disable = 0;
@@ -152,7 +152,7 @@ static int (*drives[4])[6] = {&drive0, &
 
 #include <asm/uaccess.h>
 
-module_param(verbose, bool, 0);
+module_param(verbose, int, 0);
 module_param(major, int, 0);
 module_param(name, charp, 0);
 module_param_array(drive0, int, NULL, 0);


Patches currently in stable-queue which might be from arnd@arndb.de are

queue-3.14/paride-make-verbose-parameter-an-int-again.patch
queue-3.14/asoc-s3c24xx-use-const-snd_soc_component_driver-pointer.patch

Re: Patch "paride: make 'verbose' parameter an 'int' again" has been added to the 3.14-stable tree

[Rusty Russell]

gregkh@linuxfoundation.org writes:
> This is a note to let you know that I've just added the patch titled
>
>     paride: make 'verbose' parameter an 'int' again
>
> to the 3.14-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>
> The filename of the patch is:
>      paride-make-verbose-parameter-an-int-again.patch
> and it can be found in the queue-3.14 subdirectory.
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.
>
>
>>>From dec63a4dec2d6d01346fd5d96062e67c0636852b Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue, 15 Mar 2016 14:53:29 -0700
> Subject: paride: make 'verbose' parameter an 'int' again
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> commit dec63a4dec2d6d01346fd5d96062e67c0636852b upstream.
>
> gcc-6.0 found an ancient bug in the paride driver, which had a
> "module_param(verbose, bool, 0);" since before 2.6.12, but actually uses
> it to accept '0', '1' or '2' as arguments:
>
>   drivers/block/paride/pd.c: In function 'pd_init_dev_parms':
>   drivers/block/paride/pd.c:298:29: warning: comparison of constant '1' with boolean expression is always false [-Wbool-compare]
>    #define DBMSG(msg) ((verbose>1)?(msg):NULL)
>
> In 2012, Rusty did a cleanup patch that also changed the type of the
> variable to 'bool', which introduced what is now a gcc warning.
>
> This changes the type back to 'int' and adapts the module_param() line
> instead, so it should work as documented in case anyone ever cares about
> running the ancient driver with debugging.
>
> Fixes: 90ab5ee94171 ("module_param: make bool parameters really bool (drivers & misc)")

Hmm, I got CC'd on this, not the original :(

Because it *never* worked.  90ab5ee94171 changed it to a bool, but it
was already module_param(bool) so you could never set it to 2.  ... And
it's been module_param(bool) before 2005.

But this breaks anyone who has been using just "verbose" or "verbose=y".

Fortunately, I'm sure nobody cares about it?

Cheers,
Rusty.

Re: Patch "paride: make 'verbose' parameter an 'int' again" has been added to the 3.14-stable tree

[Arnd Bergmann]

On Tuesday 03 May 2016 07:04:10 Rusty Russell wrote:
> gregkh@linuxfoundation.org writes:
> > This is a note to let you know that I've just added the patch titled
> >
> >     paride: make 'verbose' parameter an 'int' again
> >
> > to the 3.14-stable tree which can be found at:
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> >
> > The filename of the patch is:
> >      paride-make-verbose-parameter-an-int-again.patch
> > and it can be found in the queue-3.14 subdirectory.
> >
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@vger.kernel.org> know about it.
> >
> >
> >>From dec63a4dec2d6d01346fd5d96062e67c0636852b Mon Sep 17 00:00:00 2001
> > From: Arnd Bergmann <arnd@arndb.de>
> > Date: Tue, 15 Mar 2016 14:53:29 -0700
> > Subject: paride: make 'verbose' parameter an 'int' again
> >
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > commit dec63a4dec2d6d01346fd5d96062e67c0636852b upstream.
> >
> > gcc-6.0 found an ancient bug in the paride driver, which had a
> > "module_param(verbose, bool, 0);" since before 2.6.12, but actually uses
> > it to accept '0', '1' or '2' as arguments:
> >
> >   drivers/block/paride/pd.c: In function 'pd_init_dev_parms':
> >   drivers/block/paride/pd.c:298:29: warning: comparison of constant '1' with boolean expression is always false [-Wbool-compare]
> >    #define DBMSG(msg) ((verbose>1)?(msg):NULL)
> >
> > In 2012, Rusty did a cleanup patch that also changed the type of the
> > variable to 'bool', which introduced what is now a gcc warning.
> >
> > This changes the type back to 'int' and adapts the module_param() line
> > instead, so it should work as documented in case anyone ever cares about
> > running the ancient driver with debugging.
> >
> > Fixes: 90ab5ee94171 ("module_param: make bool parameters really bool (drivers & misc)")
> 
> Hmm, I got CC'd on this, not the original 

Sorry about that, I try to Cc people manually when I have a fixes tag, but
I sometimes forget. I should update my script to do that automatically.

> Because it *never* worked.  90ab5ee94171 changed it to a bool, but it
> was already module_param(bool) so you could never set it to 2.  ... And
> it's been module_param(bool) before 2005.
> 
> But this breaks anyone who has been using just "verbose" or "verbose=y".
> 
> Fortunately, I'm sure nobody cares about it?

Right. I think we can safely make that assumption that nobody cares
about the new change in behavior. As I tried to explain in the changelog,
the bug of not being able to set verbose=2 is historic, and I am
not worried about it. Your patch from 2012 makes the driver more
consistent, but it does introduce what has now become a gcc warning.

We probably don't care about that in backports, because nobody uses
the driver and people rarely build older kernels with the latest
gcc release, if that's even possible.

FWIW, I've managed to find how the odd behavior was introduced originally
in linux-2.6.11 just before the start of git, see below for giggles.
I'm also sure that nobody cared about the verbose=2 syntax breaking back
then, it was already obsolete technology even at the turn of the century.

	Arnd

---
commit 3352539ac7e770a69b003f4a0952f817695b9204
Author: Andrew Morton <akpm@osdl.org>
Date:   Fri Jan 7 22:21:50 2005 -0800

    [PATCH] MODULE_PARM conversions
    
    Rusty Russell <rusty@rustcorp.com.au>
    
    MODULE_PARM conversions for x86 `allyesconfig'.
    
    Signed-off-by: Andrew Morton <akpm@osdl.org>
    Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c
index 5f6534d..c4c8f0d 100644
--- a/drivers/block/paride/pd.c
+++ b/drivers/block/paride/pd.c
@@ -178,15 +178,15 @@ void pd_setup(char *str, int *ints)
 
 #endif
 
-MODULE_PARM(verbose, "i");
-MODULE_PARM(major, "i");
-MODULE_PARM(name, "s");
-MODULE_PARM(cluster, "i");
-MODULE_PARM(nice, "i");
-MODULE_PARM(drive0, "1-8i");
-MODULE_PARM(drive1, "1-8i");
-MODULE_PARM(drive2, "1-8i");
-MODULE_PARM(drive3, "1-8i");
+module_param(verbose, bool, 0);
+module_param(major, int, 0);
+module_param(name, charp, 0);
+module_param(cluster, int, 0);
+module_param(nice, int, 0);
+module_param_array(drive0, int, NULL, 0);
+module_param_array(drive1, int, NULL, 0);
+module_param_array(drive2, int, NULL, 0);
+module_param_array(drive3, int, NULL, 0);
 
 #include "paride.h"
 

Re: Patch "paride: make 'verbose' parameter an 'int' again" has been added to the 3.14-stable tree

[Rusty Russell]

Arnd Bergmann <arnd@arndb.de> writes:
> On Tuesday 03 May 2016 07:04:10 Rusty Russell wrote:
>> Because it *never* worked.  90ab5ee94171 changed it to a bool, but it
>> was already module_param(bool) so you could never set it to 2.  ... And
>> it's been module_param(bool) before 2005.
>> 
>> But this breaks anyone who has been using just "verbose" or "verbose=y".
>> 
>> Fortunately, I'm sure nobody cares about it?
>
> Right. I think we can safely make that assumption that nobody cares
> about the new change in behavior. As I tried to explain in the changelog,
> the bug of not being able to set verbose=2 is historic, and I am
> not worried about it. Your patch from 2012 makes the driver more
> consistent, but it does introduce what has now become a gcc warning.
>
> We probably don't care about that in backports, because nobody uses
> the driver and people rarely build older kernels with the latest
> gcc release, if that's even possible.
>
> FWIW, I've managed to find how the odd behavior was introduced originally
> in linux-2.6.11 just before the start of git, see below for giggles.
> I'm also sure that nobody cared about the verbose=2 syntax breaking back
> then, it was already obsolete technology even at the turn of the century.

Yeah, I suspected it was my screwup originally :)

Thanks!
Rusty.

> 	Arnd
>
> ---
> commit 3352539ac7e770a69b003f4a0952f817695b9204
> Author: Andrew Morton <akpm@osdl.org>
> Date:   Fri Jan 7 22:21:50 2005 -0800
>
>     [PATCH] MODULE_PARM conversions
>     
>     Rusty Russell <rusty@rustcorp.com.au>
>     
>     MODULE_PARM conversions for x86 `allyesconfig'.
>     
>     Signed-off-by: Andrew Morton <akpm@osdl.org>
>     Signed-off-by: Linus Torvalds <torvalds@osdl.org>
>
> diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c
> index 5f6534d..c4c8f0d 100644
> --- a/drivers/block/paride/pd.c
> +++ b/drivers/block/paride/pd.c
> @@ -178,15 +178,15 @@ void pd_setup(char *str, int *ints)
>  
>  #endif
>  
> -MODULE_PARM(verbose, "i");
> -MODULE_PARM(major, "i");
> -MODULE_PARM(name, "s");
> -MODULE_PARM(cluster, "i");
> -MODULE_PARM(nice, "i");
> -MODULE_PARM(drive0, "1-8i");
> -MODULE_PARM(drive1, "1-8i");
> -MODULE_PARM(drive2, "1-8i");
> -MODULE_PARM(drive3, "1-8i");
> +module_param(verbose, bool, 0);
> +module_param(major, int, 0);
> +module_param(name, charp, 0);
> +module_param(cluster, int, 0);
> +module_param(nice, int, 0);
> +module_param_array(drive0, int, NULL, 0);
> +module_param_array(drive1, int, NULL, 0);
> +module_param_array(drive2, int, NULL, 0);
> +module_param_array(drive3, int, NULL, 0);
>  
>  #include "paride.h"
>