Re: [PATCH 1/7] dynamic debug v2 - infrastructure

[Greg KH <greg@kroah.com>]

On Tue, Jul 15, 2008 at 05:31:08PM -0400, Jason Baron wrote:
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 1a06026..fb03dbc 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -592,7 +592,11 @@ extern const char *dev_driver_string(struct device *dev);
>  #define dev_info(dev, format, arg...)		\
>  	dev_printk(KERN_INFO , dev , format , ## arg)
>  
> -#ifdef DEBUG
> +#if 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

In looking at your follow-on patches, where you add this to subsystems,
it seems that you have to add a lot of #include <linux/dynamic_printk.h>
lines.

And in looking at this modification to device.h, I think lots of the
kernel should break, but you are getting it "for free" by including the
.h file within kernel.h.

Why not also include it here in device.h?

Also, with this change, code that is already using dev_dbg() today is
instantly converted over to this logic now, right?

Anyway, this looks great, no objections from me at all, very good work.

thanks,

greg k-h