Skip to contentSkip to conversation

dev-core: fix build break when DEBUG is enabled

PATCH In review Dmitry Kasatkin 6 participants 12 messages Aug 27, 2013 – Aug 28, 2013

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

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
index 22b546a..d336beb 100644
@@ -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
#1 lore raw

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
index 5b08cd8..3cbf1c0 100644
@@ -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
#2 lore raw

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 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 -17
index 5b08cd8..3cbf1c0 100644
@@ -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
#3 lore raw

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 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 -17
index 5b08cd8..3cbf1c0 100644
@@ -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
#4 lore raw
In reply to Dmitry Kasatkin
Quoted 5 lines · Dmitry Kasatkin

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...

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 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 +0 -0
index 22b546a..d336beb 100644

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, ...)			\
#5 lore raw
Quoted 6 lines · Joe Perches
Quoted 5 lines · Dmitry Kasatkin

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:

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_debug instead of __dynamic_dev_dbg

Not sure.

Quoted 2 lines

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

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 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 +0 -0
index 22b546a..d336beb 100644

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, ...)			\
#6 lore raw
Quoted 7 lines · Jason Baron
Quoted 6 lines · Joe Perches
Quoted 5 lines · Dmitry Kasatkin

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:

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.

#7 lore raw
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_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:

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 the dynamic_debug
control.

Indeed.

Thanks.

- Dmitry

Signature · 2 lines
Thanks,
Dmitry
#8 lore raw

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
index 22b546a..7d960d5 100644
@@ -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
maintainer Acked-by on v3 1/1
#9 lore raw
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_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

Quoted 7 lines

This patch separates definition for CONFIG_DYNAMIC_DEBUG and DEBUG cases.

[Note, Sarah moved the comment above the macro to avoid checkpatch warnings.]

#10 lore raw
In reply to Dmitry Kasatkin

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_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
index 22b546a..d336beb 100644
@@ -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
#11 lore raw
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_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
index 22b546a..d336beb 100644
@@ -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