Re: [PATCH 1/1] driver-core: Shut up dev_dbg_reatelimited() withoutDEBUG
From: Antti Palosaari
Date: Tue Aug 21 2012 - 04:16:10 EST
Hello Hiroshi,
On 08/21/2012 10:02 AM, Hiroshi Doyu wrote:
Antti Palosaari <crope@xxxxxx> wrote @ Mon, 20 Aug 2012 23:29:34 +0200:
On 08/20/2012 02:14 PM, Hiroshi Doyu wrote:
Hi Antti,
Antti Palosaari <crope@xxxxxx> wrote @ Sat, 18 Aug 2012 02:11:56 +0200:
On 08/17/2012 09:04 AM, Hiroshi Doyu wrote:
dev_dbg_reatelimited() without DEBUG printed "217078 callbacks
suppressed". This shouldn't print anything without DEBUG.
Signed-off-by: Hiroshi Doyu <hdoyu@xxxxxxxxxx>
Reported-by: Antti Palosaari <crope@xxxxxx>
---
include/linux/device.h | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index eb945e1..d4dc26e 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -962,9 +962,13 @@ 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(DEBUG)
#define dev_dbg_ratelimited(dev, fmt, ...) \
dev_level_ratelimited(dev_dbg, dev, fmt, ##__VA_ARGS__)
-
+#else
+#define dev_dbg_ratelimited(dev, fmt, ...) \
+ no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#endif
/*
* Stupid hackaround for existing uses of non-printk uses dev_info
*
NACK. I don't think that's correct behavior. After that patch it kills
all output of dev_dbg_ratelimited(). If I use dynamic debugs and order
debugs, I expect to see debugs as earlier.
You are right. I attached the update patch, just moving *_ratelimited
functions after dev_dbg() definitions.
With DEBUG defined/undefined in your "test.ko", it works fine. With
CONFIG_DYNAMIC_DEBUG, it works with "+p", but with "-p", still
"..callbacks suppressed" is printed.
I am using dynamic debugs and behavior is now just same as it was when
reported that bug. OK, likely for static debug it is now correct.
The following patch can also refrain "..callbacks suppressed" with
"-p". I think that it's ok for all cases.
From b4c6aa9160f03b61ed17975c73db36c983a48927 Mon Sep 17 00:00:00 2001
From: Hiroshi Doyu <hdoyu@xxxxxxxxxx>
Date: Mon, 20 Aug 2012 13:49:19 +0300
Subject: [v3 1/1] driver-core: Shut up dev_dbg_reatelimited() without DEBUG
dev_dbg_reatelimited() without DEBUG printed "217078 callbacks
suppressed". This shouldn't print anything without DEBUG.
With CONFIG_DYNAMIC_DEBUG, the print should be configured as expected.
Signed-off-by: Hiroshi Doyu <hdoyu@xxxxxxxxxx>
Reported-by: Antti Palosaari <crope@xxxxxx>
---
include/linux/device.h | 62 +++++++++++++++++++++++++++++------------------
1 files changed, 38 insertions(+), 24 deletions(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index 9648331..bb6ffcb 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -932,6 +932,32 @@ int _dev_info(const struct device *dev, const char *fmt, ...)
#endif
+/*
+ * Stupid hackaround for existing uses of non-printk uses dev_info
+ *
+ * Note that the definition of dev_info below is actually _dev_info
+ * and a macro is used to avoid redefining dev_info
+ */
+
+#define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
+
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define dev_dbg(dev, format, ...) \
+do { \
+ dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
+} while (0)
+#elif defined(DEBUG)
+#define dev_dbg(dev, format, arg...) \
+ dev_printk(KERN_DEBUG, dev, format, ##arg)
+#else
+#define dev_dbg(dev, format, arg...) \
+({ \
+ if (0) \
+ dev_printk(KERN_DEBUG, dev, format, ##arg); \
+ 0; \
+})
+#endif
+
#define dev_level_ratelimited(dev_level, dev, fmt, ...) \
do { \
static DEFINE_RATELIMIT_STATE(_rs, \
@@ -955,33 +981,21 @@ 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)
#define dev_dbg_ratelimited(dev, fmt, ...) \
- dev_level_ratelimited(dev_dbg, dev, fmt, ##__VA_ARGS__)
-
-/*
- * Stupid hackaround for existing uses of non-printk uses dev_info
- *
- * Note that the definition of dev_info below is actually _dev_info
- * and a macro is used to avoid redefining dev_info
- */
-
-#define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
-
-#if defined(CONFIG_DYNAMIC_DEBUG)
-#define dev_dbg(dev, format, ...) \
-do { \
- dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
+do { \
+ static DEFINE_RATELIMIT_STATE(_rs, \
+ DEFAULT_RATELIMIT_INTERVAL, \
+ DEFAULT_RATELIMIT_BURST); \
+ DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
+ if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT) && \
+ __ratelimit(&_rs)) \
+ __dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
+ ##__VA_ARGS__); \
} while (0)
-#elif defined(DEBUG)
-#define dev_dbg(dev, format, arg...) \
- dev_printk(KERN_DEBUG, dev, format, ##arg)
#else
-#define dev_dbg(dev, format, arg...) \
-({ \
- if (0) \
- dev_printk(KERN_DEBUG, dev, format, ##arg); \
- 0; \
-})
+#define dev_dbg_ratelimited(dev, fmt, ...) \
+ no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
#endif
#ifdef VERBOSE_DEBUG
That seems to work correctly now. I tested it using dynamic debugs. It
was Hin-Tak who originally reported that bug for me after I added few
ratelimited debugs for DVB stack. Thank you!
Reported-by: Hin-Tak Leung <htl10@xxxxxxxxxxxxxxxxxxxxx>
Tested-by: Antti Palosaari <crope@xxxxxx>
regards
Antti
--
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/