[PATCH] DEBUG_KOBJECT vs. DYNAMIC_PRINTK_DEBUG

[PATCH] DEBUG_KOBJECT vs. DYNAMIC_PRINTK_DEBUG

[Cornelia Huck]

DEBUG_KOBJECT has no effect when DYNAMIC_PRINTK_DEBUG is set
(and you can get the messages via that feature), so let's make
it depend on !DYNAMIC_PRINTK_DEBUG.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 lib/Kconfig.debug |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

--- linux-2.6.orig/lib/Kconfig.debug
+++ linux-2.6/lib/Kconfig.debug
@@ -450,7 +450,7 @@ config STACKTRACE
 
 config DEBUG_KOBJECT
 	bool "kobject debugging"
-	depends on DEBUG_KERNEL
+	depends on DEBUG_KERNEL && !DYNAMIC_PRINTK_DEBUG
 	help
 	  If you say Y here, some extra kobject debugging messages will be sent
 	  to the syslog. 

Re: [PATCH] DEBUG_KOBJECT vs. DYNAMIC_PRINTK_DEBUG

[Jason Baron]

On Tue, Dec 02, 2008 at 02:52:51PM +0100, Cornelia Huck wrote:
> DEBUG_KOBJECT has no effect when DYNAMIC_PRINTK_DEBUG is set
> (and you can get the messages via that feature), so let's make
> it depend on !DYNAMIC_PRINTK_DEBUG.
> 

indeed. you raise the more general question of what do if both 'DEBUG'
and 'CONFIG_DYNAMIC_PRINTK_DEBUG' are set for a file? I think that in
general the 'DEBUG' should take precedence, as you point out. However, I
think we should fix this by reshuffling the logic in
include/linux/kernel.h by doing:

if (DEBUG)
	#define pr_debug printk
elseif (CONFIG_DYNAMIC_PRINTK_DEBUG)
	#define pr_debug dynamic_pr_debug()
else
	#define pr_debug if (0) blah:
endif

make sense? what do you think?

thanks,

-Jason

Re: [PATCH] DEBUG_KOBJECT vs. DYNAMIC_PRINTK_DEBUG

[Cornelia Huck]

On Wed, 3 Dec 2008 15:12:03 -0500,
Jason Baron <jbaron@redhat.com> wrote:

> indeed. you raise the more general question of what do if both 'DEBUG'
> and 'CONFIG_DYNAMIC_PRINTK_DEBUG' are set for a file? I think that in
> general the 'DEBUG' should take precedence, as you point out. However, I
> think we should fix this by reshuffling the logic in
> include/linux/kernel.h by doing:
> 
> if (DEBUG)
> 	#define pr_debug printk
> elseif (CONFIG_DYNAMIC_PRINTK_DEBUG)
> 	#define pr_debug dynamic_pr_debug()
> else
> 	#define pr_debug if (0) blah:
> endif
> 
> make sense? what do you think?

Yes, that makes sense, I agree. I'll follow up with a patch.

[PATCH] Make DEBUG take precedence over DYNAMIC_PRINTK_DEBUG.

[Cornelia Huck]

Statically defined DEBUG should take precedence over
dynamically enabled debugging; otherwise adding DEBUG
(like, for example, via CONFIG_DEBUG_KOBJECT) does not
have the expected result of printing pr_debug() messages
unconditionally.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 include/linux/kernel.h |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

--- linux-2.6.orig/include/linux/kernel.h
+++ linux-2.6/include/linux/kernel.h
@@ -338,13 +338,13 @@ static inline char *pack_hex_byte(char *
         printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
 
 /* If you are writing a driver, please use dev_dbg instead */
-#if defined(CONFIG_DYNAMIC_PRINTK_DEBUG)
+#if defined(DEBUG)
+#define pr_debug(fmt, ...) \
+	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#elif defined(CONFIG_DYNAMIC_PRINTK_DEBUG)
 #define pr_debug(fmt, ...) do { \
 	dynamic_pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
 	} while (0)
-#elif defined(DEBUG)
-#define pr_debug(fmt, ...) \
-	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
 #else
 #define pr_debug(fmt, ...) \
 	({ if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); 0; })

Re: [PATCH] Make DEBUG take precedence over DYNAMIC_PRINTK_DEBUG.

[Jason Baron]

On Thu, Dec 04, 2008 at 01:51:13PM +0100, Cornelia Huck wrote:
> Statically defined DEBUG should take precedence over
> dynamically enabled debugging; otherwise adding DEBUG
> (like, for example, via CONFIG_DEBUG_KOBJECT) does not
> have the expected result of printing pr_debug() messages
> unconditionally.
> 
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  include/linux/kernel.h |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> --- linux-2.6.orig/include/linux/kernel.h
> +++ linux-2.6/include/linux/kernel.h
> @@ -338,13 +338,13 @@ static inline char *pack_hex_byte(char *
>          printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
>  
>  /* If you are writing a driver, please use dev_dbg instead */
> -#if defined(CONFIG_DYNAMIC_PRINTK_DEBUG)
> +#if defined(DEBUG)
> +#define pr_debug(fmt, ...) \
> +	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +#elif defined(CONFIG_DYNAMIC_PRINTK_DEBUG)
>  #define pr_debug(fmt, ...) do { \
>  	dynamic_pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
>  	} while (0)
> -#elif defined(DEBUG)
> -#define pr_debug(fmt, ...) \
> -	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
>  #else
>  #define pr_debug(fmt, ...) \
>  	({ if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); 0; })

looks good...if you want to also make the analogous change for
'dev_dbg()' in include/linux/device.h, I'll ack it.

thanks,

-Jason

[PATCH] Make DEBUG take precedence over DYNAMIC_PRINTK_DEBUG (v2)

[Cornelia Huck]

Statically defined DEBUG should take precedence over
dynamically enabled debugging; otherwise adding DEBUG
(like, for example, via CONFIG_DEBUG_KOBJECT) does not
have the expected result of printing pr_debug() and dev_dbg()
messages unconditionally.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 include/linux/device.h |    8 ++++----
 include/linux/kernel.h |    8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

--- linux-2.6.orig/include/linux/kernel.h
+++ linux-2.6/include/linux/kernel.h
@@ -338,13 +338,13 @@ static inline char *pack_hex_byte(char *
         printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
 
 /* If you are writing a driver, please use dev_dbg instead */
-#if defined(CONFIG_DYNAMIC_PRINTK_DEBUG)
+#if defined(DEBUG)
+#define pr_debug(fmt, ...) \
+	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#elif defined(CONFIG_DYNAMIC_PRINTK_DEBUG)
 #define pr_debug(fmt, ...) do { \
 	dynamic_pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
 	} while (0)
-#elif defined(DEBUG)
-#define pr_debug(fmt, ...) \
-	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
 #else
 #define pr_debug(fmt, ...) \
 	({ if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); 0; })
--- linux-2.6.orig/include/linux/device.h
+++ linux-2.6/include/linux/device.h
@@ -553,13 +553,13 @@ extern const char *dev_driver_string(con
 #define dev_info(dev, format, arg...)		\
 	dev_printk(KERN_INFO , dev , format , ## arg)
 
-#if defined(CONFIG_DYNAMIC_PRINTK_DEBUG)
+#if defined(DEBUG)
+#define dev_dbg(dev, format, arg...)		\
+	dev_printk(KERN_DEBUG , dev , format , ## arg)
+#elif defined(CONFIG_DYNAMIC_PRINTK_DEBUG)
 #define dev_dbg(dev, format, ...) do { \
 	dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
 	} while (0)
-#elif defined(DEBUG)
-#define dev_dbg(dev, format, arg...)		\
-	dev_printk(KERN_DEBUG , dev , format , ## arg)
 #else
 #define dev_dbg(dev, format, arg...)		\
 	({ if (0) dev_printk(KERN_DEBUG, dev, format, ##arg); 0; })

Re: [PATCH] Make DEBUG take precedence over DYNAMIC_PRINTK_DEBUG (v2)

[Jason Baron]

On Thu, Dec 04, 2008 at 04:55:47PM +0100, Cornelia Huck wrote:
> Statically defined DEBUG should take precedence over
> dynamically enabled debugging; otherwise adding DEBUG
> (like, for example, via CONFIG_DEBUG_KOBJECT) does not
> have the expected result of printing pr_debug() and dev_dbg()
> messages unconditionally.
> 
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  include/linux/device.h |    8 ++++----
>  include/linux/kernel.h |    8 ++++----
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> --- linux-2.6.orig/include/linux/kernel.h
> +++ linux-2.6/include/linux/kernel.h
> @@ -338,13 +338,13 @@ static inline char *pack_hex_byte(char *
>          printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
>  
>  /* If you are writing a driver, please use dev_dbg instead */
> -#if defined(CONFIG_DYNAMIC_PRINTK_DEBUG)
> +#if defined(DEBUG)
> +#define pr_debug(fmt, ...) \
> +	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +#elif defined(CONFIG_DYNAMIC_PRINTK_DEBUG)
>  #define pr_debug(fmt, ...) do { \
>  	dynamic_pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
>  	} while (0)
> -#elif defined(DEBUG)
> -#define pr_debug(fmt, ...) \
> -	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
>  #else
>  #define pr_debug(fmt, ...) \
>  	({ if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); 0; })
> --- linux-2.6.orig/include/linux/device.h
> +++ linux-2.6/include/linux/device.h
> @@ -553,13 +553,13 @@ extern const char *dev_driver_string(con
>  #define dev_info(dev, format, arg...)		\
>  	dev_printk(KERN_INFO , dev , format , ## arg)
>  
> -#if defined(CONFIG_DYNAMIC_PRINTK_DEBUG)
> +#if defined(DEBUG)
> +#define dev_dbg(dev, format, arg...)		\
> +	dev_printk(KERN_DEBUG , dev , format , ## arg)
> +#elif defined(CONFIG_DYNAMIC_PRINTK_DEBUG)
>  #define dev_dbg(dev, format, ...) do { \
>  	dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
>  	} while (0)
> -#elif defined(DEBUG)
> -#define dev_dbg(dev, format, arg...)		\
> -	dev_printk(KERN_DEBUG , dev , format , ## arg)
>  #else
>  #define dev_dbg(dev, format, arg...)		\
>  	({ if (0) dev_printk(KERN_DEBUG, dev, format, ##arg); 0; })

looks good.

Acked-by: Jason Baron <jbaron@redhat.com>

thanks,

-Jason