tiny: reverse logic for DISABLE_DEV_COREDUMP

PATCH In review Aristeu Rozanski 4 participants 11 messages Oct 16, 2014 – Oct 30, 2014 Driver core

lore

Share & tools

lore

Aristeu Rozanski

author

2014-10-16 13:37 UTC #0 lore raw

PATCH 1/1 tiny: reverse logic for DISABLE_DEV_COREDUMP

(This time Cc'ing Johannes)

It's desirable for allnconfig and tinyconfig targets to result in the least amount of code possible. DISABLE_DEV_COREDUMP exists as a way to switch off DEV_COREDUMP regardless if any drivers select WANT_DEV_COREDUMP.

This patch renames the option to ENABLE_DEV_COREDUMP and setting it to 'n' (as in allnconfig or tinyconfig) will effectively disable device coredump.

Cc Greg Kroah-Hartman <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Johannes Berg <[email protected]>

Reviewed-by Josh Triplett <[email protected]>

Signed-off-by Aristeu Rozanski <[email protected]>

drivers/base/Kconfig +11 -8

index 134f763..99d3072 100644
@@ -171,20 +171,23 @@ config WANT_DEV_COREDUMP
 	  Drivers should "select" this option if they desire to use the
 	  device coredump mechanism.
 
-config DISABLE_DEV_COREDUMP
-	bool "Disable device coredump" if EXPERT
+config ENABLE_DEV_COREDUMP
+	bool "Enable device coredump" if EXPERT
+	default y
 	help
-	  Disable the device coredump mechanism despite drivers wanting to
-	  use it; this allows for more sensitive systems or systems that
-	  don't want to ever access the information to not have the code,
-	  nor keep any data.
+	  This option controls if the device coredump mechanism is available or
+	  not; if disabled, the mechanism will be omitted even if drivers that
+	  can use it are enabled.
+	  Say 'N' for more sensitive systems or systems that don't want
+	  to ever access the information to not have the code, nor keep any
+	  data.
 
-	  If unsure, say N.
+	  If unsure, say Y.
 
 config DEV_COREDUMP
 	bool
 	default y if WANT_DEV_COREDUMP
-	depends on !DISABLE_DEV_COREDUMP
+	depends on ENABLE_DEV_COREDUMP
 
 config DEBUG_DRIVER
 	bool "Driver Core verbose debug messages"

Josh Triplett

2014-10-16 13:44 UTC #1 lore raw

Quoted 1 line · Aristeu Rozanski

(This time Cc'ing Johannes)

This kind of note shouldn't be in the commit-message portion of the commit.

Quoted 9 lines

It's desirable for allnconfig and tinyconfig targets to result in the least amount of code possible. DISABLE_DEV_COREDUMP exists as a way to switch off DEV_COREDUMP regardless if any drivers select WANT_DEV_COREDUMP.
This patch renames the option to ENABLE_DEV_COREDUMP and setting it to 'n' (as in allnconfig or tinyconfig) will effectively disable device coredump.

Quoted 5 lines

Cc Greg Kroah-Hartman <[email protected]>
Cc Josh Triplett <[email protected]>
Cc Johannes Berg <[email protected]>
Reviewed-by Josh Triplett <[email protected]>
Signed-off-by Aristeu Rozanski <[email protected]>

You should have a "---" line here, followed by a diffstat. If you're not using git format-patch to format your patches, you should.

- Josh Triplett

Quoted 36 lines

drivers/base/Kconfig +11 -8

index 134f763..99d3072 100644
@@ -171,20 +171,23 @@ config WANT_DEV_COREDUMP
 	  Drivers should "select" this option if they desire to use the
 	  device coredump mechanism.
 
-config DISABLE_DEV_COREDUMP
-	bool "Disable device coredump" if EXPERT
+config ENABLE_DEV_COREDUMP
+	bool "Enable device coredump" if EXPERT
+	default y
 	help
-	  Disable the device coredump mechanism despite drivers wanting to
-	  use it; this allows for more sensitive systems or systems that
-	  don't want to ever access the information to not have the code,
-	  nor keep any data.
+	  This option controls if the device coredump mechanism is available or
+	  not; if disabled, the mechanism will be omitted even if drivers that
+	  can use it are enabled.
+	  Say 'N' for more sensitive systems or systems that don't want
+	  to ever access the information to not have the code, nor keep any
+	  data.
 
-	  If unsure, say N.
+	  If unsure, say Y.
 
 config DEV_COREDUMP
 	bool
 	default y if WANT_DEV_COREDUMP
-	depends on !DISABLE_DEV_COREDUMP
+	depends on ENABLE_DEV_COREDUMP
 
 config DEBUG_DRIVER
 	bool "Driver Core verbose debug messages"

Aristeu Rozanski

author

2014-10-16 15:49 UTC #2 lore raw

PATCH 1/1 tiny: reverse logic for DISABLE_DEV_COREDUMP

This patch renames the option to ENABLE_DEV_COREDUMP and setting it to 'n' (as in allnconfig or tinyconfig) will effectively disable device coredump.

Cc Greg Kroah-Hartman <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Johannes Berg <[email protected]>

Reviewed-by Josh Triplett <[email protected]>

Signed-off-by Aristeu Rozanski <[email protected]>

---
 drivers/base/Kconfig | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

drivers/base/Kconfig +11 -8

index 134f763..99d3072 100644
@@ -171,20 +171,23 @@ config WANT_DEV_COREDUMP
 	  Drivers should "select" this option if they desire to use the
 	  device coredump mechanism.
 
-config DISABLE_DEV_COREDUMP
-	bool "Disable device coredump" if EXPERT
+config ENABLE_DEV_COREDUMP
+	bool "Enable device coredump" if EXPERT
+	default y
 	help
-	  Disable the device coredump mechanism despite drivers wanting to
-	  use it; this allows for more sensitive systems or systems that
-	  don't want to ever access the information to not have the code,
-	  nor keep any data.
+	  This option controls if the device coredump mechanism is available or
+	  not; if disabled, the mechanism will be omitted even if drivers that
+	  can use it are enabled.
+	  Say 'N' for more sensitive systems or systems that don't want
+	  to ever access the information to not have the code, nor keep any
+	  data.
 
-	  If unsure, say N.
+	  If unsure, say Y.
 
 config DEV_COREDUMP
 	bool
 	default y if WANT_DEV_COREDUMP
-	depends on !DISABLE_DEV_COREDUMP
+	depends on ENABLE_DEV_COREDUMP
 
 config DEBUG_DRIVER
 	bool "Driver Core verbose debug messages"

Signature · 1 line

1.8.3.1

Johannes Berg

Reviewed-by Acked-by on v1 1/1

2014-10-20 09:42 UTC #3 lore raw

Quoted 5 lines · Aristeu Rozanski

It's desirable for allnconfig and tinyconfig targets to result in the least amount of code possible. DISABLE_DEV_COREDUMP exists as a way to switch off DEV_COREDUMP regardless if any drivers select WANT_DEV_COREDUMP.

Quoted 7 lines

This patch renames the option to ENABLE_DEV_COREDUMP and setting it to 'n' (as in allnconfig or tinyconfig) will effectively disable device coredump.
Cc Greg Kroah-Hartman <[email protected]>
Cc Josh Triplett <[email protected]>
Cc Johannes Berg <[email protected]>

Reviewed-by Johannes Berg <[email protected]>

Acked-by Johannes Berg <[email protected]>

Yeah, looks sensible, and should work.

johannes

Josh Triplett

2014-10-20 11:55 UTC #4 lore raw

Quoted 10 lines · Johannes Berg

Quoted 9 lines · Aristeu Rozanski
It's desirable for allnconfig and tinyconfig targets to result in the least amount of code possible. DISABLE_DEV_COREDUMP exists as a way to switch off DEV_COREDUMP regardless if any drivers select WANT_DEV_COREDUMP.
This patch renames the option to ENABLE_DEV_COREDUMP and setting it to 'n' (as in allnconfig or tinyconfig) will effectively disable device coredump.

Quoted 8 lines

Quoted 3 lines
Cc Greg Kroah-Hartman <[email protected]>
Cc Josh Triplett <[email protected]>
Cc Johannes Berg <[email protected]>
Reviewed-by Johannes Berg <[email protected]>
Acked-by Johannes Berg <[email protected]>
Yeah, looks sensible, and should work.

Thanks.

I've added this to the tiny/reverse-dev-coredump branch of my tinification tree at
https://git.kernel.org/cgit/linux/kernel/git/josh/linux.git/ , with Johannes' review, and I'll push it upstream during the 3.19 merge window.

- Josh Triplett

Greg Kroah-Hartman

maintainer

2014-10-20 22:19 UTC #5 lore raw

Quoted 20 lines · Josh Triplett

Quoted 10 lines · Johannes Berg
Quoted 9 lines · Aristeu Rozanski
It's desirable for allnconfig and tinyconfig targets to result in the least amount of code possible. DISABLE_DEV_COREDUMP exists as a way to switch off DEV_COREDUMP regardless if any drivers select WANT_DEV_COREDUMP.
This patch renames the option to ENABLE_DEV_COREDUMP and setting it to 'n' (as in allnconfig or tinyconfig) will effectively disable device coredump.
Quoted 8 lines
Quoted 3 lines
Cc Greg Kroah-Hartman <[email protected]>
Cc Josh Triplett <[email protected]>
Cc Johannes Berg <[email protected]>
Reviewed-by Johannes Berg <[email protected]>
Acked-by Johannes Berg <[email protected]>
Yeah, looks sensible, and should work.

Quoted 7 lines

Thanks.
I've added this to the tiny/reverse-dev-coredump branch of my tinification tree at
https://git.kernel.org/cgit/linux/kernel/git/josh/linux.git/ , with Johannes' review, and I'll push it upstream during the 3.19 merge window.

I'll take it through the same tree this original came in and will get it into 3.18 so we don't confuse anyone with an odd config option.

thanks,

greg k-h

[email protected]

2014-10-20 23:45 UTC #6 lore raw

Quoted 23 lines · Greg Kroah-Hartman

Quoted 22 lines · Josh Triplett
Quoted 10 lines · Johannes Berg
Quoted 9 lines · Aristeu Rozanski
It's desirable for allnconfig and tinyconfig targets to result in the least amount of code possible. DISABLE_DEV_COREDUMP exists as a way to switch off DEV_COREDUMP regardless if any drivers select WANT_DEV_COREDUMP.
This patch renames the option to ENABLE_DEV_COREDUMP and setting it to 'n' (as in allnconfig or tinyconfig) will effectively disable device coredump.
Quoted 8 lines
Quoted 3 lines
Cc Greg Kroah-Hartman <[email protected]>
Cc Josh Triplett <[email protected]>
Cc Johannes Berg <[email protected]>
Reviewed-by Johannes Berg <[email protected]>
Acked-by Johannes Berg <[email protected]>
Yeah, looks sensible, and should work.
Thanks.

Quoted 8 lines

Quoted 5 lines
I've added this to the tiny/reverse-dev-coredump branch of my tinification tree at
https://git.kernel.org/cgit/linux/kernel/git/josh/linux.git/ , with Johannes' review, and I'll push it upstream during the 3.19 merge window.
I'll take it through the same tree this original came in and will get it into 3.18 so we don't confuse anyone with an odd config option.

Excellent, dropped from tiny then.

- Josh Triplett

Johannes Berg

2014-10-30 08:01 UTC #7 lore raw

In reply to Greg Kroah-Hartman

Quoted 8 lines · Greg Kroah-Hartman

Quoted 5 lines
I've added this to the tiny/reverse-dev-coredump branch of my tinification tree at
https://git.kernel.org/cgit/linux/kernel/git/josh/linux.git/ , with Johannes' review, and I'll push it upstream during the 3.19 merge window.
I'll take it through the same tree this original came in and will get it into 3.18 so we don't confuse anyone with an odd config option.

I don't know how far this commit made it, but looking at it again now I think the new option should probably be called "Allow device coredump" rather than "Enable device coredump" since it doesn't actually enable it if no drivers want it.

Should I send another patch on top?

johannes

Johannes Berg

2014-10-30 09:00 UTC #8 lore raw

PATCH 1/1 tiny: rename ENABLE_DEV_COREDUMP to ALLOW_DEV_COREDUMP

From: Johannes Berg <[email protected]>

The ENABLE_DEV_COREDUMP option is misleading as it implies that it gets the framework enabled, this isn't true it just allows it to get enabled if a driver needs it.

Rename it to ALLOW_DEV_COREDUMP to better capture its semantics.

Signed-off-by Johannes Berg <[email protected]>

---
 drivers/base/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/base/Kconfig +3 -3

index 99d30729d11e..e9f96afe920d 100644
@@ -171,8 +171,8 @@ config WANT_DEV_COREDUMP
 	  Drivers should "select" this option if they desire to use the
 	  device coredump mechanism.
 
-config ENABLE_DEV_COREDUMP
-	bool "Enable device coredump" if EXPERT
+config ALLOW_DEV_COREDUMP
+	bool "Allow device coredump" if EXPERT
 	default y
 	help
 	  This option controls if the device coredump mechanism is available or

@@ -187,7 +187,7 @@ config ENABLE_DEV_COREDUMP
 config DEV_COREDUMP
 	bool
 	default y if WANT_DEV_COREDUMP
-	depends on ENABLE_DEV_COREDUMP
+	depends on ALLOW_DEV_COREDUMP
 
 config DEBUG_DRIVER
 	bool "Driver Core verbose debug messages"

Signature · 1 line

2.1.0

Aristeu Rozanski

author Acked-by on v1 1/1

2014-10-30 12:55 UTC #9 lore raw

Re: [PATCH 3.18] tiny: rename ENABLE_DEV_COREDUMP to ALLOW_DEV_COREDUMP

Quoted 7 lines · Johannes Berg

From: Johannes Berg <[email protected]>
The ENABLE_DEV_COREDUMP option is misleading as it implies that it gets the framework enabled, this isn't true it just allows it to get enabled if a driver needs it.
Rename it to ALLOW_DEV_COREDUMP to better capture its semantics.

Sounds much better!

Acked-by Aristeu Rozanski <[email protected]>

Quoted 34 lines

Signed-off-by Johannes Berg <[email protected]>

---
 drivers/base/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/base/Kconfig +3 -3

index 99d30729d11e..e9f96afe920d 100644
@@ -171,8 +171,8 @@ config WANT_DEV_COREDUMP
 	  Drivers should "select" this option if they desire to use the
 	  device coredump mechanism.
 
-config ENABLE_DEV_COREDUMP
-	bool "Enable device coredump" if EXPERT
+config ALLOW_DEV_COREDUMP
+	bool "Allow device coredump" if EXPERT
 	default y
 	help
 	  This option controls if the device coredump mechanism is available or

@@ -187,7 +187,7 @@ config ENABLE_DEV_COREDUMP
 config DEV_COREDUMP
 	bool
 	default y if WANT_DEV_COREDUMP
-	depends on ENABLE_DEV_COREDUMP
+	depends on ALLOW_DEV_COREDUMP
 
 config DEBUG_DRIVER
 	bool "Driver Core verbose debug messages"

Signature · 1 line

2.1.0

Signature · 1 line

Aristeu

Josh Triplett

Reviewed-by on v1 1/1

2014-10-30 13:32 UTC #10 lore raw

Re: [PATCH 3.18] tiny: rename ENABLE_DEV_COREDUMP to ALLOW_DEV_COREDUMP

In reply to Johannes Berg

Quoted 9 lines · Johannes Berg

From: Johannes Berg <[email protected]>
The ENABLE_DEV_COREDUMP option is misleading as it implies that it gets the framework enabled, this isn't true it just allows it to get enabled if a driver needs it.
Rename it to ALLOW_DEV_COREDUMP to better capture its semantics.
Signed-off-by Johannes Berg <[email protected]>

Seems reasonable. The original "DISABLE" was really "FORCE_DISABLE" (even if used), and the inverse of "FORCE_DISABLE" is not "ENABLE", it's "ALLOW" (if used).

Reviewed-by Josh Triplett <[email protected]>

Quoted 31 lines

 drivers/base/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/base/Kconfig +3 -3

index 99d30729d11e..e9f96afe920d 100644
@@ -171,8 +171,8 @@ config WANT_DEV_COREDUMP
 	  Drivers should "select" this option if they desire to use the
 	  device coredump mechanism.
 
-config ENABLE_DEV_COREDUMP
-	bool "Enable device coredump" if EXPERT
+config ALLOW_DEV_COREDUMP
+	bool "Allow device coredump" if EXPERT
 	default y
 	help
 	  This option controls if the device coredump mechanism is available or

@@ -187,7 +187,7 @@ config ENABLE_DEV_COREDUMP
 config DEV_COREDUMP
 	bool
 	default y if WANT_DEV_COREDUMP
-	depends on ENABLE_DEV_COREDUMP
+	depends on ALLOW_DEV_COREDUMP
 
 config DEBUG_DRIVER
 	bool "Driver Core verbose debug messages"

Signature · 1 line

2.1.0