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

From dec63a4dec2d6d01346fd5d96062e67c0636852b Mon Sep 17 00:00:00 2001

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 <[email protected]> know about it.

Quoted 1 line

Quoted 1 line

From dec63a4dec2d6d01346fd5d96062e67c0636852b Mon Sep 17 00:00:00 2001

From: Arnd Bergmann <[email protected]>
Date: Tue, 15 Mar 2016 14:53:29 -0700
Subject: paride: make 'verbose' parameter an 'int' again

From: Arnd Bergmann <[email protected]>

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)")

Quoted 35 lines · [email protected] 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 <[email protected]> know about it.

Quoted 1 line

From dec63a4dec2d6d01346fd5d96062e67c0636852b Mon Sep 17 00:00:00 2001

From: Arnd Bergmann <[email protected]>
Date: Tue, 15 Mar 2016 14:53:29 -0700
Subject: paride: make 'verbose' parameter an 'int' again

From: Arnd Bergmann <[email protected]>

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.

Quoted 5 lines

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?

Quoted 7 lines · 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.

Arnd

---
commit 3352539ac7e770a69b003f4a0952f817695b9204

Author: Andrew Morton <[email protected]>
Date:   Fri Jan 7 22:21:50 2005 -0800

[PATCH] MODULE_PARM conversions

Rusty Russell <[email protected]>

MODULE_PARM conversions for x86 `allyesconfig'.

Signed-off-by Andrew Morton <[email protected]>

Signed-off-by Linus Torvalds <[email protected]>

drivers/block/paride/pd.c +9 -9

index 5f6534d..c4c8f0d 100644
@@ -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"