When DEBUG is defined, dev_dbg_ratelimited uses dynamic debug data
structures even when CONFIG_DYNAMIC_DEBUG is not defined.
It leads to build break.
For example, when I try to use dev_dbg_ratelimited in USB code and
CONFIG_USB_DEBUG is enabled, but CONFIG_DYNAMIC_DEBUG is not, I get:
CC [M] drivers/usb/host/xhci-ring.o
drivers/usb/host/xhci-ring.c: In function ‘xhci_queue_intr_tx’:
drivers/usb/host/xhci-ring.c:3059:3: error: implicit declaration of function ‘DEFINE_DYNAMIC_DEBUG_METADATA’ [-Werror=implicit-function-declaration]
drivers/usb/host/xhci-ring.c:3059:3: error: ‘descriptor’ undeclared (first use in this function)
drivers/usb/host/xhci-ring.c:3059:3: note: each undeclared identifier is reported only once for each function it appears in
drivers/usb/host/xhci-ring.c:3059:3: error: implicit declaration of function ‘__dynamic_pr_debug’ [-Werror=implicit-function-declaration]
drivers/usb/host/xhci-ring.c: In function ‘xhci_queue_isoc_tx_prepare’:
drivers/usb/host/xhci-ring.c:3847:3: error: ‘descriptor’ undeclared (first use in this function)
cc1: some warnings being treated as errors
make[2]: *** [drivers/usb/host/xhci-ring.o] Error 1
make[1]: *** [drivers/usb/host] Error 2
make: *** [drivers/usb/] Error 2
This patch separates definition for CONFIG_DYNAMIC_DEBUG and DEBUG cases.
Signed-off-by: Dmitry Kasatkin <[email protected]>
---
include/linux/device.h | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index 22b546a..d336beb 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1099,17 +1099,28 @@ do { \
dev_level_ratelimited(dev_notice, dev, fmt, ##__VA_ARGS__)
#define dev_info_ratelimited(dev, fmt, ...) \
dev_level_ratelimited(dev_info, dev, fmt, ##__VA_ARGS__)
-#if defined(CONFIG_DYNAMIC_DEBUG) || defined(DEBUG)
+#if defined(CONFIG_DYNAMIC_DEBUG)
#define dev_dbg_ratelimited(dev, fmt, ...) \
do { \
static DEFINE_RATELIMIT_STATE(_rs, \
DEFAULT_RATELIMIT_INTERVAL, \
DEFAULT_RATELIMIT_BURST); \
DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
+ /* descriptor check is first to prevent flooding with \
+ "callbacks suppressed" */ \
if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT) && \
__ratelimit(&_rs)) \
- __dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
- ##__VA_ARGS__); \
+ __dynamic_dev_dbg(&descriptor, dev, fmt, \
+ ##__VA_ARGS__); \
+} while (0)
+#elif defined(DEBUG)
+#define dev_dbg_ratelimited(dev, fmt, ...) \
+do { \
+ static DEFINE_RATELIMIT_STATE(_rs, \
+ DEFAULT_RATELIMIT_INTERVAL, \
+ DEFAULT_RATELIMIT_BURST); \
+ if (__ratelimit(&_rs)) \
+ dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
} while (0)
#else
#define dev_dbg_ratelimited(dev, fmt, ...) \
--
1.8.1.2
When debug is not enabled and dev_dbg() will expand to nothing,
log might be flooded with "callbacks suppressed". If it was not
done on purpose, better to use dev_dbg_ratelimited() instead.
Signed-off-by: Dmitry Kasatkin <[email protected]>
---
drivers/usb/host/xhci-ring.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 5b08cd8..3cbf1c0 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3060,14 +3060,10 @@ int xhci_queue_intr_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
* to set the polling interval (once the API is added).
*/
if (xhci_interval != ep_interval) {
- if (printk_ratelimit())
- dev_dbg(&urb->dev->dev, "Driver uses different interval"
- " (%d microframe%s) than xHCI "
- "(%d microframe%s)\n",
- ep_interval,
- ep_interval == 1 ? "" : "s",
- xhci_interval,
- xhci_interval == 1 ? "" : "s");
+ dev_dbg_ratelimited(&urb->dev->dev,
+ "Driver uses different interval (%d microframe%s) than xHCI (%d microframe%s)\n",
+ ep_interval, ep_interval == 1 ? "" : "s",
+ xhci_interval, xhci_interval == 1 ? "" : "s");
urb->interval = xhci_interval;
/* Convert back to frames for LS/FS devices */
if (urb->dev->speed == USB_SPEED_LOW ||
@@ -3849,14 +3845,10 @@ int xhci_queue_isoc_tx_prepare(struct xhci_hcd *xhci, gfp_t mem_flags,
* to set the polling interval (once the API is added).
*/
if (xhci_interval != ep_interval) {
- if (printk_ratelimit())
- dev_dbg(&urb->dev->dev, "Driver uses different interval"
- " (%d microframe%s) than xHCI "
- "(%d microframe%s)\n",
- ep_interval,
- ep_interval == 1 ? "" : "s",
- xhci_interval,
- xhci_interval == 1 ? "" : "s");
+ dev_dbg_ratelimited(&urb->dev->dev,
+ "Driver uses different interval (%d microframe%s) than xHCI (%d microframe%s)\n",
+ ep_interval, ep_interval == 1 ? "" : "s",
+ xhci_interval, xhci_interval == 1 ? "" : "s");
urb->interval = xhci_interval;
/* Convert back to frames for LS/FS devices */
if (urb->dev->speed == USB_SPEED_LOW ||
--
1.8.1.2
Used vim "=" as Sarah suggested.
- Dmitry
On Tue, Aug 27, 2013 at 5:47 PM, Dmitry Kasatkin <[email protected]> wrote:
> When debug is not enabled and dev_dbg() will expand to nothing,
> log might be flooded with "callbacks suppressed". If it was not
> done on purpose, better to use dev_dbg_ratelimited() instead.
>
> Signed-off-by: Dmitry Kasatkin <[email protected]>
> ---
> drivers/usb/host/xhci-ring.c | 24 ++++++++----------------
> 1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 5b08cd8..3cbf1c0 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -3060,14 +3060,10 @@ int xhci_queue_intr_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> * to set the polling interval (once the API is added).
> */
> if (xhci_interval != ep_interval) {
> - if (printk_ratelimit())
> - dev_dbg(&urb->dev->dev, "Driver uses different interval"
> - " (%d microframe%s) than xHCI "
> - "(%d microframe%s)\n",
> - ep_interval,
> - ep_interval == 1 ? "" : "s",
> - xhci_interval,
> - xhci_interval == 1 ? "" : "s");
> + dev_dbg_ratelimited(&urb->dev->dev,
> + "Driver uses different interval (%d microframe%s) than xHCI (%d microframe%s)\n",
> + ep_interval, ep_interval == 1 ? "" : "s",
> + xhci_interval, xhci_interval == 1 ? "" : "s");
> urb->interval = xhci_interval;
> /* Convert back to frames for LS/FS devices */
> if (urb->dev->speed == USB_SPEED_LOW ||
> @@ -3849,14 +3845,10 @@ int xhci_queue_isoc_tx_prepare(struct xhci_hcd *xhci, gfp_t mem_flags,
> * to set the polling interval (once the API is added).
> */
> if (xhci_interval != ep_interval) {
> - if (printk_ratelimit())
> - dev_dbg(&urb->dev->dev, "Driver uses different interval"
> - " (%d microframe%s) than xHCI "
> - "(%d microframe%s)\n",
> - ep_interval,
> - ep_interval == 1 ? "" : "s",
> - xhci_interval,
> - xhci_interval == 1 ? "" : "s");
> + dev_dbg_ratelimited(&urb->dev->dev,
> + "Driver uses different interval (%d microframe%s) than xHCI (%d microframe%s)\n",
> + ep_interval, ep_interval == 1 ? "" : "s",
> + xhci_interval, xhci_interval == 1 ? "" : "s");
> urb->interval = xhci_interval;
> /* Convert back to frames for LS/FS devices */
> if (urb->dev->speed == USB_SPEED_LOW ||
> --
> 1.8.1.2
>
--
Thanks,
Dmitry
On Tue, 2013-08-27 at 17:47 +0300, Dmitry Kasatkin wrote:
> When DEBUG is defined, dev_dbg_ratelimited uses dynamic debug data
> structures even when CONFIG_DYNAMIC_DEBUG is not defined.
> It leads to build break.
> For example, when I try to use dev_dbg_ratelimited in USB code and
> CONFIG_USB_DEBUG is enabled, but CONFIG_DYNAMIC_DEBUG is not, I get:
Jason?
Seems mostly sensible to me but I think the first check
needs to be
#if defined(CONFIG_DYNAMIC_DEBUG) && defined(DEBUG)
Also, it's curious why it was ever using __dynamic_pr_debug
instead of __dynamic_dev_dbg
Maybe for completeness there should be a
dev_vdbg_ratelimited too.
Additional bit below...
> CC [M] drivers/usb/host/xhci-ring.o
> drivers/usb/host/xhci-ring.c: In function ‘xhci_queue_intr_tx’:
> drivers/usb/host/xhci-ring.c:3059:3: error: implicit declaration of function ‘DEFINE_DYNAMIC_DEBUG_METADATA’ [-Werror=implicit-function-declaration]
> drivers/usb/host/xhci-ring.c:3059:3: error: ‘descriptor’ undeclared (first use in this function)
> drivers/usb/host/xhci-ring.c:3059:3: note: each undeclared identifier is reported only once for each function it appears in
> drivers/usb/host/xhci-ring.c:3059:3: error: implicit declaration of function ‘__dynamic_pr_debug’ [-Werror=implicit-function-declaration]
> drivers/usb/host/xhci-ring.c: In function ‘xhci_queue_isoc_tx_prepare’:
> drivers/usb/host/xhci-ring.c:3847:3: error: ‘descriptor’ undeclared (first use in this function)
> cc1: some warnings being treated as errors
> make[2]: *** [drivers/usb/host/xhci-ring.o] Error 1
> make[1]: *** [drivers/usb/host] Error 2
> make: *** [drivers/usb/] Error 2
>
> This patch separates definition for CONFIG_DYNAMIC_DEBUG and DEBUG cases.
>
> Signed-off-by: Dmitry Kasatkin <[email protected]>
> ---
> include/linux/device.h | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 22b546a..d336beb 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -1099,17 +1099,28 @@ do { \
> dev_level_ratelimited(dev_notice, dev, fmt, ##__VA_ARGS__)
> #define dev_info_ratelimited(dev, fmt, ...) \
> dev_level_ratelimited(dev_info, dev, fmt, ##__VA_ARGS__)
> -#if defined(CONFIG_DYNAMIC_DEBUG) || defined(DEBUG)
> +#if defined(CONFIG_DYNAMIC_DEBUG)
I think this needs to be
#if defined(CONFIG_DYNAMIC_DEBUG) && defined(DEBUG)
> #define dev_dbg_ratelimited(dev, fmt, ...) \
> do { \
> static DEFINE_RATELIMIT_STATE(_rs, \
> DEFAULT_RATELIMIT_INTERVAL, \
> DEFAULT_RATELIMIT_BURST); \
> DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
> + /* descriptor check is first to prevent flooding with \
> + "callbacks suppressed" */ \
> if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT) && \
> __ratelimit(&_rs)) \
> - __dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
> - ##__VA_ARGS__); \
> + __dynamic_dev_dbg(&descriptor, dev, fmt, \
> + ##__VA_ARGS__); \
> +} while (0)
> +#elif defined(DEBUG)
> +#define dev_dbg_ratelimited(dev, fmt, ...) \
> +do { \
> + static DEFINE_RATELIMIT_STATE(_rs, \
> + DEFAULT_RATELIMIT_INTERVAL, \
> + DEFAULT_RATELIMIT_BURST); \
> + if (__ratelimit(&_rs)) \
> + dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
> } while (0)
> #else
> #define dev_dbg_ratelimited(dev, fmt, ...) \
On 08/27/2013 12:20 PM, Joe Perches wrote:
> On Tue, 2013-08-27 at 17:47 +0300, Dmitry Kasatkin wrote:
>> When DEBUG is defined, dev_dbg_ratelimited uses dynamic debug data
>> structures even when CONFIG_DYNAMIC_DEBUG is not defined.
>> It leads to build break.
>> For example, when I try to use dev_dbg_ratelimited in USB code and
>> CONFIG_USB_DEBUG is enabled, but CONFIG_DYNAMIC_DEBUG is not, I get:
> Jason?
>
> Seems mostly sensible to me but I think the first check
> needs to be
>
> #if defined(CONFIG_DYNAMIC_DEBUG) && defined(DEBUG)
Why? All the other call-sites, do it the way Dmitry has done it.
> Also, it's curious why it was ever using __dynamic_pr_debug
> instead of __dynamic_dev_dbg
Not sure.
> Maybe for completeness there should be a
> dev_vdbg_ratelimited too.
I could also see the rate limiting being pushed further down, such that
dynamic debug could control how and whether or not its enabled.
Thanks,
-Jason
> Additional bit below...
>
>> CC [M] drivers/usb/host/xhci-ring.o
>> drivers/usb/host/xhci-ring.c: In function ‘xhci_queue_intr_tx’:
>> drivers/usb/host/xhci-ring.c:3059:3: error: implicit declaration of function ‘DEFINE_DYNAMIC_DEBUG_METADATA’ [-Werror=implicit-function-declaration]
>> drivers/usb/host/xhci-ring.c:3059:3: error: ‘descriptor’ undeclared (first use in this function)
>> drivers/usb/host/xhci-ring.c:3059:3: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/usb/host/xhci-ring.c:3059:3: error: implicit declaration of function ‘__dynamic_pr_debug’ [-Werror=implicit-function-declaration]
>> drivers/usb/host/xhci-ring.c: In function ‘xhci_queue_isoc_tx_prepare’:
>> drivers/usb/host/xhci-ring.c:3847:3: error: ‘descriptor’ undeclared (first use in this function)
>> cc1: some warnings being treated as errors
>> make[2]: *** [drivers/usb/host/xhci-ring.o] Error 1
>> make[1]: *** [drivers/usb/host] Error 2
>> make: *** [drivers/usb/] Error 2
>>
>> This patch separates definition for CONFIG_DYNAMIC_DEBUG and DEBUG cases.
>>
>> Signed-off-by: Dmitry Kasatkin <[email protected]>
>> ---
>> include/linux/device.h | 17 ++++++++++++++---
>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index 22b546a..d336beb 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -1099,17 +1099,28 @@ do { \
>> dev_level_ratelimited(dev_notice, dev, fmt, ##__VA_ARGS__)
>> #define dev_info_ratelimited(dev, fmt, ...) \
>> dev_level_ratelimited(dev_info, dev, fmt, ##__VA_ARGS__)
>> -#if defined(CONFIG_DYNAMIC_DEBUG) || defined(DEBUG)
>> +#if defined(CONFIG_DYNAMIC_DEBUG)
> I think this needs to be
>
> #if defined(CONFIG_DYNAMIC_DEBUG) && defined(DEBUG)
>
>> #define dev_dbg_ratelimited(dev, fmt, ...) \
>> do { \
>> static DEFINE_RATELIMIT_STATE(_rs, \
>> DEFAULT_RATELIMIT_INTERVAL, \
>> DEFAULT_RATELIMIT_BURST); \
>> DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
>> + /* descriptor check is first to prevent flooding with \
>> + "callbacks suppressed" */ \
>> if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT) && \
>> __ratelimit(&_rs)) \
>> - __dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
>> - ##__VA_ARGS__); \
>> + __dynamic_dev_dbg(&descriptor, dev, fmt, \
>> + ##__VA_ARGS__); \
>> +} while (0)
>> +#elif defined(DEBUG)
>> +#define dev_dbg_ratelimited(dev, fmt, ...) \
>> +do { \
>> + static DEFINE_RATELIMIT_STATE(_rs, \
>> + DEFAULT_RATELIMIT_INTERVAL, \
>> + DEFAULT_RATELIMIT_BURST); \
>> + if (__ratelimit(&_rs)) \
>> + dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
>> } while (0)
>> #else
>> #define dev_dbg_ratelimited(dev, fmt, ...) \
>
>
Hi Dmitry,
This patch looks fine, and applies ok. I'll send it off to Greg with a
couple more bug fixes shortly.
Sarah Sharp
On Tue, Aug 27, 2013 at 05:52:33PM +0300, Dmitry Kasatkin wrote:
> Used vim "=" as Sarah suggested.
>
> - Dmitry
>
> On Tue, Aug 27, 2013 at 5:47 PM, Dmitry Kasatkin <[email protected]> wrote:
> > When debug is not enabled and dev_dbg() will expand to nothing,
> > log might be flooded with "callbacks suppressed". If it was not
> > done on purpose, better to use dev_dbg_ratelimited() instead.
> >
> > Signed-off-by: Dmitry Kasatkin <[email protected]>
> > ---
> > drivers/usb/host/xhci-ring.c | 24 ++++++++----------------
> > 1 file changed, 8 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> > index 5b08cd8..3cbf1c0 100644
> > --- a/drivers/usb/host/xhci-ring.c
> > +++ b/drivers/usb/host/xhci-ring.c
> > @@ -3060,14 +3060,10 @@ int xhci_queue_intr_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> > * to set the polling interval (once the API is added).
> > */
> > if (xhci_interval != ep_interval) {
> > - if (printk_ratelimit())
> > - dev_dbg(&urb->dev->dev, "Driver uses different interval"
> > - " (%d microframe%s) than xHCI "
> > - "(%d microframe%s)\n",
> > - ep_interval,
> > - ep_interval == 1 ? "" : "s",
> > - xhci_interval,
> > - xhci_interval == 1 ? "" : "s");
> > + dev_dbg_ratelimited(&urb->dev->dev,
> > + "Driver uses different interval (%d microframe%s) than xHCI (%d microframe%s)\n",
> > + ep_interval, ep_interval == 1 ? "" : "s",
> > + xhci_interval, xhci_interval == 1 ? "" : "s");
> > urb->interval = xhci_interval;
> > /* Convert back to frames for LS/FS devices */
> > if (urb->dev->speed == USB_SPEED_LOW ||
> > @@ -3849,14 +3845,10 @@ int xhci_queue_isoc_tx_prepare(struct xhci_hcd *xhci, gfp_t mem_flags,
> > * to set the polling interval (once the API is added).
> > */
> > if (xhci_interval != ep_interval) {
> > - if (printk_ratelimit())
> > - dev_dbg(&urb->dev->dev, "Driver uses different interval"
> > - " (%d microframe%s) than xHCI "
> > - "(%d microframe%s)\n",
> > - ep_interval,
> > - ep_interval == 1 ? "" : "s",
> > - xhci_interval,
> > - xhci_interval == 1 ? "" : "s");
> > + dev_dbg_ratelimited(&urb->dev->dev,
> > + "Driver uses different interval (%d microframe%s) than xHCI (%d microframe%s)\n",
> > + ep_interval, ep_interval == 1 ? "" : "s",
> > + xhci_interval, xhci_interval == 1 ? "" : "s");
> > urb->interval = xhci_interval;
> > /* Convert back to frames for LS/FS devices */
> > if (urb->dev->speed == USB_SPEED_LOW ||
> > --
> > 1.8.1.2
> >
>
>
>
> --
> Thanks,
> Dmitry
On Tue, 2013-08-27 at 13:32 -0400, Jason Baron wrote:
> On 08/27/2013 12:20 PM, Joe Perches wrote:
> > On Tue, 2013-08-27 at 17:47 +0300, Dmitry Kasatkin wrote:
> >> When DEBUG is defined, dev_dbg_ratelimited uses dynamic debug data
> >> structures even when CONFIG_DYNAMIC_DEBUG is not defined.
> >> It leads to build break.
> >> For example, when I try to use dev_dbg_ratelimited in USB code and
> >> CONFIG_USB_DEBUG is enabled, but CONFIG_DYNAMIC_DEBUG is not, I get:
> > Jason?
> >
> > Seems mostly sensible to me but I think the first check
> > needs to be
> >
> > #if defined(CONFIG_DYNAMIC_DEBUG) && defined(DEBUG)
>
> Why? All the other call-sites, do it the way Dmitry has done it.
Fine. Originally I thought it useful to not
store the ratelimit state, but this way those
messages can be enabled via the dynamic_debug
control.
On Tue, Aug 27, 2013 at 9:16 PM, Joe Perches <[email protected]> wrote:
> On Tue, 2013-08-27 at 13:32 -0400, Jason Baron wrote:
>> On 08/27/2013 12:20 PM, Joe Perches wrote:
>> > On Tue, 2013-08-27 at 17:47 +0300, Dmitry Kasatkin wrote:
>> >> When DEBUG is defined, dev_dbg_ratelimited uses dynamic debug data
>> >> structures even when CONFIG_DYNAMIC_DEBUG is not defined.
>> >> It leads to build break.
>> >> For example, when I try to use dev_dbg_ratelimited in USB code and
>> >> CONFIG_USB_DEBUG is enabled, but CONFIG_DYNAMIC_DEBUG is not, I get:
>> > Jason?
>> >
>> > Seems mostly sensible to me but I think the first check
>> > needs to be
>> >
>> > #if defined(CONFIG_DYNAMIC_DEBUG) && defined(DEBUG)
>>
>> Why? All the other call-sites, do it the way Dmitry has done it.
>
I followed the pattern..
> Fine. Originally I thought it useful to not
> store the ratelimit state, but this way those
> messages can be enabled via the dynamic_debug
> control.
>
Indeed.
Thanks.
- Dmitry
--
Thanks,
Dmitry
Applying: dev-core: fix build break when DEBUG is enabled
WARNING: Avoid unnecessary line continuations
#18: FILE: include/linux/device.h:1110:
+ "callbacks suppressed" */ \
WARNING: Prefer dev_dbg(... to dev_printk(KERN_DEBUG, ...
#33: FILE: include/linux/device.h:1123:
+ dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
total: 0 errors, 2 warnings, 31 lines checked
I'm just going to fix the first warning myself by moving your comment
above the macro. Please don't add comments inside macros, and please
run your patches through checkpatch.pl.
Sarah Sharp
On Tue, Aug 27, 2013 at 05:47:34PM +0300, Dmitry Kasatkin wrote:
> When DEBUG is defined, dev_dbg_ratelimited uses dynamic debug data
> structures even when CONFIG_DYNAMIC_DEBUG is not defined.
> It leads to build break.
> For example, when I try to use dev_dbg_ratelimited in USB code and
> CONFIG_USB_DEBUG is enabled, but CONFIG_DYNAMIC_DEBUG is not, I get:
>
> CC [M] drivers/usb/host/xhci-ring.o
> drivers/usb/host/xhci-ring.c: In function ‘xhci_queue_intr_tx’:
> drivers/usb/host/xhci-ring.c:3059:3: error: implicit declaration of function ‘DEFINE_DYNAMIC_DEBUG_METADATA’ [-Werror=implicit-function-declaration]
> drivers/usb/host/xhci-ring.c:3059:3: error: ‘descriptor’ undeclared (first use in this function)
> drivers/usb/host/xhci-ring.c:3059:3: note: each undeclared identifier is reported only once for each function it appears in
> drivers/usb/host/xhci-ring.c:3059:3: error: implicit declaration of function ‘__dynamic_pr_debug’ [-Werror=implicit-function-declaration]
> drivers/usb/host/xhci-ring.c: In function ‘xhci_queue_isoc_tx_prepare’:
> drivers/usb/host/xhci-ring.c:3847:3: error: ‘descriptor’ undeclared (first use in this function)
> cc1: some warnings being treated as errors
> make[2]: *** [drivers/usb/host/xhci-ring.o] Error 1
> make[1]: *** [drivers/usb/host] Error 2
> make: *** [drivers/usb/] Error 2
>
> This patch separates definition for CONFIG_DYNAMIC_DEBUG and DEBUG cases.
>
> Signed-off-by: Dmitry Kasatkin <[email protected]>
> ---
> include/linux/device.h | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 22b546a..d336beb 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -1099,17 +1099,28 @@ do { \
> dev_level_ratelimited(dev_notice, dev, fmt, ##__VA_ARGS__)
> #define dev_info_ratelimited(dev, fmt, ...) \
> dev_level_ratelimited(dev_info, dev, fmt, ##__VA_ARGS__)
> -#if defined(CONFIG_DYNAMIC_DEBUG) || defined(DEBUG)
> +#if defined(CONFIG_DYNAMIC_DEBUG)
> #define dev_dbg_ratelimited(dev, fmt, ...) \
> do { \
> static DEFINE_RATELIMIT_STATE(_rs, \
> DEFAULT_RATELIMIT_INTERVAL, \
> DEFAULT_RATELIMIT_BURST); \
> DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
> + /* descriptor check is first to prevent flooding with \
> + "callbacks suppressed" */ \
> if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT) && \
> __ratelimit(&_rs)) \
> - __dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
> - ##__VA_ARGS__); \
> + __dynamic_dev_dbg(&descriptor, dev, fmt, \
> + ##__VA_ARGS__); \
> +} while (0)
> +#elif defined(DEBUG)
> +#define dev_dbg_ratelimited(dev, fmt, ...) \
> +do { \
> + static DEFINE_RATELIMIT_STATE(_rs, \
> + DEFAULT_RATELIMIT_INTERVAL, \
> + DEFAULT_RATELIMIT_BURST); \
> + if (__ratelimit(&_rs)) \
> + dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
> } while (0)
> #else
> #define dev_dbg_ratelimited(dev, fmt, ...) \
> --
> 1.8.1.2
>
On 28/08/13 20:43, Sarah Sharp wrote:
> Applying: dev-core: fix build break when DEBUG is enabled
> WARNING: Avoid unnecessary line continuations
> #18: FILE: include/linux/device.h:1110:
> + "callbacks suppressed" */ \
>
> WARNING: Prefer dev_dbg(... to dev_printk(KERN_DEBUG, ...
> #33: FILE: include/linux/device.h:1123:
> + dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
>
> total: 0 errors, 2 warnings, 31 lines checked
>
> I'm just going to fix the first warning myself by moving your comment
> above the macro. Please don't add comments inside macros, and please
> run your patches through checkpatch.pl.
I run checkpatch always.
The first warning is in my understanding is not related to comments
inside macro,
but just that avoid using "\".
I would not remove comment, because it is for explaining the macro line...
Second warning is natural in this case because macro itself defines
dev_dbg() functionality.
Internally it must use something else than itself...
- Dmitry
> Sarah Sharp
>
> On Tue, Aug 27, 2013 at 05:47:34PM +0300, Dmitry Kasatkin wrote:
>> When DEBUG is defined, dev_dbg_ratelimited uses dynamic debug data
>> structures even when CONFIG_DYNAMIC_DEBUG is not defined.
>> It leads to build break.
>> For example, when I try to use dev_dbg_ratelimited in USB code and
>> CONFIG_USB_DEBUG is enabled, but CONFIG_DYNAMIC_DEBUG is not, I get:
>>
>> CC [M] drivers/usb/host/xhci-ring.o
>> drivers/usb/host/xhci-ring.c: In function ‘xhci_queue_intr_tx’:
>> drivers/usb/host/xhci-ring.c:3059:3: error: implicit declaration of function ‘DEFINE_DYNAMIC_DEBUG_METADATA’ [-Werror=implicit-function-declaration]
>> drivers/usb/host/xhci-ring.c:3059:3: error: ‘descriptor’ undeclared (first use in this function)
>> drivers/usb/host/xhci-ring.c:3059:3: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/usb/host/xhci-ring.c:3059:3: error: implicit declaration of function ‘__dynamic_pr_debug’ [-Werror=implicit-function-declaration]
>> drivers/usb/host/xhci-ring.c: In function ‘xhci_queue_isoc_tx_prepare’:
>> drivers/usb/host/xhci-ring.c:3847:3: error: ‘descriptor’ undeclared (first use in this function)
>> cc1: some warnings being treated as errors
>> make[2]: *** [drivers/usb/host/xhci-ring.o] Error 1
>> make[1]: *** [drivers/usb/host] Error 2
>> make: *** [drivers/usb/] Error 2
>>
>> This patch separates definition for CONFIG_DYNAMIC_DEBUG and DEBUG cases.
>>
>> Signed-off-by: Dmitry Kasatkin <[email protected]>
>> ---
>> include/linux/device.h | 17 ++++++++++++++---
>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index 22b546a..d336beb 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -1099,17 +1099,28 @@ do { \
>> dev_level_ratelimited(dev_notice, dev, fmt, ##__VA_ARGS__)
>> #define dev_info_ratelimited(dev, fmt, ...) \
>> dev_level_ratelimited(dev_info, dev, fmt, ##__VA_ARGS__)
>> -#if defined(CONFIG_DYNAMIC_DEBUG) || defined(DEBUG)
>> +#if defined(CONFIG_DYNAMIC_DEBUG)
>> #define dev_dbg_ratelimited(dev, fmt, ...) \
>> do { \
>> static DEFINE_RATELIMIT_STATE(_rs, \
>> DEFAULT_RATELIMIT_INTERVAL, \
>> DEFAULT_RATELIMIT_BURST); \
>> DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
>> + /* descriptor check is first to prevent flooding with \
>> + "callbacks suppressed" */ \
>> if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT) && \
>> __ratelimit(&_rs)) \
>> - __dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
>> - ##__VA_ARGS__); \
>> + __dynamic_dev_dbg(&descriptor, dev, fmt, \
>> + ##__VA_ARGS__); \
>> +} while (0)
>> +#elif defined(DEBUG)
>> +#define dev_dbg_ratelimited(dev, fmt, ...) \
>> +do { \
>> + static DEFINE_RATELIMIT_STATE(_rs, \
>> + DEFAULT_RATELIMIT_INTERVAL, \
>> + DEFAULT_RATELIMIT_BURST); \
>> + if (__ratelimit(&_rs)) \
>> + dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
>> } while (0)
>> #else
>> #define dev_dbg_ratelimited(dev, fmt, ...) \
>> --
>> 1.8.1.2
>>
Any objections to queuing this patch through Greg's usb-next tree? I've
already sent the 2/2 patch, which will cause build breakage without this
patch.
Changes since v2:
- Fixed a checkpatch warning caused by a comment in a macro.
Sarah Sharp
>8----------------------------------------------------------------8<
From: Dmitry Kasatkin <[email protected]>
When DEBUG is defined, dev_dbg_ratelimited uses dynamic debug data
structures even when CONFIG_DYNAMIC_DEBUG is not defined.
It leads to build break.
For example, when I try to use dev_dbg_ratelimited in USB code and
CONFIG_USB_DEBUG is enabled, but CONFIG_DYNAMIC_DEBUG is not, I get:
CC [M] drivers/usb/host/xhci-ring.o
drivers/usb/host/xhci-ring.c: In function ‘xhci_queue_intr_tx’:
drivers/usb/host/xhci-ring.c:3059:3: error: implicit declaration of function ‘DEFINE_DYNAMIC_DEBUG_METADATA’ [-Werror=implicit-function-declaration]
drivers/usb/host/xhci-ring.c:3059:3: error: ‘descriptor’ undeclared (first use in this function)
drivers/usb/host/xhci-ring.c:3059:3: note: each undeclared identifier is reported only once for each function it appears in
drivers/usb/host/xhci-ring.c:3059:3: error: implicit declaration of function ‘__dynamic_pr_debug’ [-Werror=implicit-function-declaration]
drivers/usb/host/xhci-ring.c: In function ‘xhci_queue_isoc_tx_prepare’:
drivers/usb/host/xhci-ring.c:3847:3: error: ‘descriptor’ undeclared (first use in this function)
cc1: some warnings being treated as errors
make[2]: *** [drivers/usb/host/xhci-ring.o] Error 1
make[1]: *** [drivers/usb/host] Error 2
make: *** [drivers/usb/] Error 2
This patch separates definition for CONFIG_DYNAMIC_DEBUG and DEBUG cases.
[Note, Sarah moved the comment above the macro to avoid checkpatch
warnings.]
Signed-off-by: Dmitry Kasatkin <[email protected]>
Signed-off-by: Sarah Sharp <[email protected]>
---
include/linux/device.h | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index 22b546a..7d960d5 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1099,7 +1099,8 @@ do { \
dev_level_ratelimited(dev_notice, dev, fmt, ##__VA_ARGS__)
#define dev_info_ratelimited(dev, fmt, ...) \
dev_level_ratelimited(dev_info, dev, fmt, ##__VA_ARGS__)
-#if defined(CONFIG_DYNAMIC_DEBUG) || defined(DEBUG)
+#if defined(CONFIG_DYNAMIC_DEBUG)
+/* descriptor check is first to prevent flooding with "callbacks suppressed" */
#define dev_dbg_ratelimited(dev, fmt, ...) \
do { \
static DEFINE_RATELIMIT_STATE(_rs, \
@@ -1108,8 +1109,17 @@ do { \
DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT) && \
__ratelimit(&_rs)) \
- __dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
- ##__VA_ARGS__); \
+ __dynamic_dev_dbg(&descriptor, dev, fmt, \
+ ##__VA_ARGS__); \
+} while (0)
+#elif defined(DEBUG)
+#define dev_dbg_ratelimited(dev, fmt, ...) \
+do { \
+ static DEFINE_RATELIMIT_STATE(_rs, \
+ DEFAULT_RATELIMIT_INTERVAL, \
+ DEFAULT_RATELIMIT_BURST); \
+ if (__ratelimit(&_rs)) \
+ dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
} while (0)
#else
#define dev_dbg_ratelimited(dev, fmt, ...) \
--
1.8.3.3
On Wed, Aug 28, 2013 at 10:50:31AM -0700, Sarah Sharp wrote:
> Any objections to queuing this patch through Greg's usb-next tree? I've
> already sent the 2/2 patch, which will cause build breakage without this
> patch.
>
> Changes since v2:
> - Fixed a checkpatch warning caused by a comment in a macro.
>
> Sarah Sharp
>
> >8----------------------------------------------------------------8<
>
> From: Dmitry Kasatkin <[email protected]>
>
> When DEBUG is defined, dev_dbg_ratelimited uses dynamic debug data
> structures even when CONFIG_DYNAMIC_DEBUG is not defined.
> It leads to build break.
> For example, when I try to use dev_dbg_ratelimited in USB code and
> CONFIG_USB_DEBUG is enabled, but CONFIG_DYNAMIC_DEBUG is not, I get:
>
> CC [M] drivers/usb/host/xhci-ring.o
> drivers/usb/host/xhci-ring.c: In function ‘xhci_queue_intr_tx’:
> drivers/usb/host/xhci-ring.c:3059:3: error: implicit declaration of function ‘DEFINE_DYNAMIC_DEBUG_METADATA’ [-Werror=implicit-function-declaration]
> drivers/usb/host/xhci-ring.c:3059:3: error: ‘descriptor’ undeclared (first use in this function)
> drivers/usb/host/xhci-ring.c:3059:3: note: each undeclared identifier is reported only once for each function it appears in
> drivers/usb/host/xhci-ring.c:3059:3: error: implicit declaration of function ‘__dynamic_pr_debug’ [-Werror=implicit-function-declaration]
> drivers/usb/host/xhci-ring.c: In function ‘xhci_queue_isoc_tx_prepare’:
> drivers/usb/host/xhci-ring.c:3847:3: error: ‘descriptor’ undeclared (first use in this function)
> cc1: some warnings being treated as errors
> make[2]: *** [drivers/usb/host/xhci-ring.o] Error 1
> make[1]: *** [drivers/usb/host] Error 2
> make: *** [drivers/usb/] Error 2
>
> This patch separates definition for CONFIG_DYNAMIC_DEBUG and DEBUG cases.
>
> [Note, Sarah moved the comment above the macro to avoid checkpatch
> warnings.]
>
> Signed-off-by: Dmitry Kasatkin <[email protected]>
> Signed-off-by: Sarah Sharp <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>