LinuxLists.cc - [PATCH] DEBUG_KOBJECT vs. DYNAMIC_PRINTK

2008-12-02 13:53:14

Subject: [PATCH] DEBUG_KOBJECT vs. DYNAMIC_PRINTK_DEBUG

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

2008-12-03 20:36:22

by Jason Baron

[permalink] [raw]

Subject: Re: [PATCH] DEBUG_KOBJECT vs. DYNAMIC_PRINTK_DEBUG

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

2008-12-04 12:49:01

by Cornelia Huck

[permalink] [raw]

Subject: Re: [PATCH] DEBUG_KOBJECT vs. DYNAMIC_PRINTK_DEBUG

On Wed, 3 Dec 2008 15:12:03 -0500,
Jason Baron <[email protected]> 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.

2008-12-04 12:52:41

by Cornelia Huck

[permalink] [raw]

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

2008-12-04 14:43:46

by Jason Baron

[permalink] [raw]

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

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 <[email protected]>
> ---
> 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

2008-12-04 15:56:00

by Cornelia Huck

[permalink] [raw]

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

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 <[email protected]>
---
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; })

2008-12-04 16:45:30

by Jason Baron

[permalink] [raw]

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

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 <[email protected]>
> ---
> 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 <[email protected]>

thanks,

-Jason