dev-core: fix build break when DEBUG is enabled
Subsystems (3)
PATCH v2 1/2 dev-core: fix build break when DEBUG is enabled
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.
--- include/linux/device.h | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
include/linux/device.h +14 -3
@@ -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, ...) \
Signature · 1 line
1.8.1.2
PATCH v2 2/2 xhci:prevent "callbacks suppressed" when debug is not enabled
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.
--- drivers/usb/host/xhci-ring.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-)
drivers/usb/host/xhci-ring.c +8 -16
@@ -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 ||
Signature · 1 line
1.8.1.2
Re: [PATCHv2 2/2] xhci:prevent "callbacks suppressed" when debug is not enabled
Used vim "=" as Sarah suggested.
- Dmitry
Quoted 54 lines · Dmitry Kasatkin
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 usedev_dbg_ratelimited()instead.Signed-off-by Dmitry Kasatkin <[email protected]>--- drivers/usb/host/xhci-ring.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-)drivers/usb/host/xhci-ring.c +8
-17@@ -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
Signature · 2 lines
Thanks, Dmitry
Re: [PATCHv2 2/2] xhci:prevent "callbacks suppressed" when debug is not enabled
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
Quoted 65 lines · Dmitry Kasatkin
Used vim "=" as Sarah suggested.
- Dmitry
Quoted 54 lines · Dmitry Kasatkin
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 usedev_dbg_ratelimited()instead.Signed-off-by Dmitry Kasatkin <[email protected]>--- drivers/usb/host/xhci-ring.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-)drivers/usb/host/xhci-ring.c +8
-17@@ -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
Signature · 2 lines
Thanks, Dmitry
Quoted 5 lines · Dmitry Kasatkin
When DEBUG is defined,
dev_dbg_ratelimiteduses dynamic debug data structures even whenCONFIG_DYNAMIC_DEBUGis not defined. It leads to build break.
For example, when I try to usedev_dbg_ratelimitedin USB code andCONFIG_USB_DEBUGis enabled, butCONFIG_DYNAMIC_DEBUGis 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 adev_vdbg_ratelimited too.
Additional bit below...
Quoted 25 lines
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 indrivers/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 2This patch separates definition for
CONFIG_DYNAMIC_DEBUGand DEBUG cases.Signed-off-by Dmitry Kasatkin <[email protected]>--- include/linux/device.h | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)include/linux/device.h +0
-0
include/linux/device.h do { \ · line 1102
Quoted 5 lines
include/linux/device.h +1
-1@@ -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)
Quoted 26 lines
include/linux/device.h +13
-2#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, ...) \
Quoted 6 lines · Joe Perches
Quoted 5 lines · Dmitry Kasatkin
When DEBUG is defined,
dev_dbg_ratelimiteduses dynamic debug data structures even whenCONFIG_DYNAMIC_DEBUGis not defined. It leads to build break.
For example, when I try to usedev_dbg_ratelimitedin USB code andCONFIG_USB_DEBUGis enabled, butCONFIG_DYNAMIC_DEBUGis not, I get:
Quoted 6 lines
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.
Quoted 2 lines
Also, it's curious why it was ever using
__dynamic_pr_debuginstead of__dynamic_dev_dbg
Not sure.
Quoted 2 lines
Maybe for completeness there should be a
dev_vdbg_ratelimitedtoo.
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
Quoted 64 lines
Additional bit below...
Quoted 25 lines
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 indrivers/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 2This patch separates definition for
CONFIG_DYNAMIC_DEBUGand DEBUG cases.Signed-off-by Dmitry Kasatkin <[email protected]>--- include/linux/device.h | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)include/linux/device.h +0
-0include/linux/device.h do { \ · line 1102
Quoted 5 lines
include/linux/device.h +1
-1@@ -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)Quoted 26 lines
include/linux/device.h +13
-2#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, ...) \
Quoted 7 lines · Jason Baron
Quoted 6 lines · Joe Perches
Quoted 5 lines · Dmitry Kasatkin
When DEBUG is defined,
dev_dbg_ratelimiteduses dynamic debug data structures even whenCONFIG_DYNAMIC_DEBUGis not defined. It leads to build break.
For example, when I try to usedev_dbg_ratelimitedin USB code andCONFIG_USB_DEBUGis enabled, butCONFIG_DYNAMIC_DEBUGis not, I get:
Quoted 8 lines
Quoted 6 lines
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.
Quoted 8 lines · Joe Perches
Quoted 7 lines · Jason Baron
Quoted 6 lines · Joe Perches
Quoted 5 lines · Dmitry Kasatkin
When DEBUG is defined,
dev_dbg_ratelimiteduses dynamic debug data structures even whenCONFIG_DYNAMIC_DEBUGis not defined. It leads to build break.
For example, when I try to usedev_dbg_ratelimitedin USB code andCONFIG_USB_DEBUGis enabled, butCONFIG_DYNAMIC_DEBUGis not, I get:
Quoted 9 lines
Quoted 8 lines
Quoted 6 lines
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..
Quoted 5 lines
Fine. Originally I thought it useful to not
store the ratelimit state, but this way those
messages can be enabled via thedynamic_debug
control.
Indeed.
Thanks.
- Dmitry
Signature · 2 lines
Thanks, Dmitry
PATCH v3 1/1 dev-core: fix build break when DEBUG is enabled
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
Quoted 1 line
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.]
--- include/linux/device.h | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
include/linux/device.h +13 -3
@@ -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, ...) \
Signature · 1 line
1.8.3.3
Quoted 32 lines · Sarah Sharp
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
Quoted 1 line
8----------------------------------------------------------------8<
From: Dmitry Kasatkin <[email protected]>
When DEBUG is defined,
dev_dbg_ratelimiteduses dynamic debug data structures even whenCONFIG_DYNAMIC_DEBUGis not defined. It leads to build break.
For example, when I try to usedev_dbg_ratelimitedin USB code andCONFIG_USB_DEBUGis enabled, butCONFIG_DYNAMIC_DEBUGis 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 indrivers/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
Quoted 7 lines
This patch separates definition for
CONFIG_DYNAMIC_DEBUGand 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]>
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
Quoted 65 lines · Dmitry Kasatkin
When DEBUG is defined,
dev_dbg_ratelimiteduses dynamic debug data structures even whenCONFIG_DYNAMIC_DEBUGis not defined. It leads to build break.
For example, when I try to usedev_dbg_ratelimitedin USB code andCONFIG_USB_DEBUGis enabled, butCONFIG_DYNAMIC_DEBUGis 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 indrivers/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 2This patch separates definition for
CONFIG_DYNAMIC_DEBUGand DEBUG cases.Signed-off-by Dmitry Kasatkin <[email protected]>--- include/linux/device.h | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)include/linux/device.h +14
-3@@ -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, ...) \Signature · 1 line
1.8.1.2
Quoted 9 lines · Sarah Sharp
Applying: dev-core: fix build break when DEBUG is enabled
WARNING: Avoid unnecessary line continuations #18: FILE:include/linux/device.h:1110:include/linux/device.h +1
-0+"callbacks suppressed" */ \WARNING: Prefer dev_dbg(... to dev_printk(KERN_DEBUG, ... #33: FILE:
include/linux/device.h:1123:include/linux/device.h +1
-0+ dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
Quoted 5 lines
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
Quoted 68 lines
Sarah Sharp
Quoted 65 lines · Dmitry Kasatkin
When DEBUG is defined,
dev_dbg_ratelimiteduses dynamic debug data structures even whenCONFIG_DYNAMIC_DEBUGis not defined. It leads to build break.
For example, when I try to usedev_dbg_ratelimitedin USB code andCONFIG_USB_DEBUGis enabled, butCONFIG_DYNAMIC_DEBUGis 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 indrivers/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 2This patch separates definition for
CONFIG_DYNAMIC_DEBUGand DEBUG cases.Signed-off-by Dmitry Kasatkin <[email protected]>--- include/linux/device.h | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)include/linux/device.h +14
-3@@ -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, ...) \Signature · 1 line
1.8.1.2