printk_ratelimited() not compiling

printk_ratelimited() not compiling

[john stultz]

Hey Joe, 
	So I thought printk_ratelimited would be perfect for an issue I'm
having, but when I tried using it, replacing a printk I had, I got the
following:

fs/namei.c:1075: error: variable ‘_rs’ has initializer but incomplete type
fs/namei.c:1075: error: unknown field ‘interval’ specified in initializer
fs/namei.c:1075: warning: excess elements in struct initializer
fs/namei.c:1075: warning: (near initialization for ‘_rs’)
fs/namei.c:1075: error: unknown field ‘burst’ specified in initializer
fs/namei.c:1075: warning: excess elements in struct initializer
fs/namei.c:1075: warning: (near initialization for ‘_rs’)
fs/namei.c:1075: error: storage size of ‘_rs’ isn’t known
fs/namei.c:1075: warning: unused variable ‘_rs’


Thinking the issue was ratelimit.h wasn't included I tried that, but
got:

 file included from include/linux/spinlock_types.h:18,
                 from include/linux/ratelimit.h:5,
                 from include/linux/kernel.h:21,
                 from /home/jstultz/projects/linux/linux-2.6-git/arch/x86/include/asm/percpu.h:45,
                 from /home/jstultz/projects/linux/linux-2.6-git/arch/x86/include/asm/current.h:5,
                 from /home/jstultz/projects/linux/linux-2.6-git/arch/x86/include/asm/processor.h:15,
                 from include/linux/prefetch.h:14,
                 from include/linux/list.h:6,
                 from include/linux/module.h:9,
                 from include/linux/crypto.h:21,
                 from arch/x86/kernel/asm-offsets_64.c:8,
                 from arch/x86/kernel/asm-offsets.c:4:
include/linux/lockdep.h:52: error: field ‘hash_entry’ has incomplete type
include/linux/lockdep.h:57: error: field ‘lock_entry’ has incomplete type
include/linux/lockdep.h:74: error: field ‘locks_after’ has incomplete type
include/linux/lockdep.h:74: error: field ‘locks_before’ has incomplete type
include/linux/lockdep.h:148: error: field ‘entry’ has incomplete type
include/linux/lockdep.h:167: error: field ‘entry’ has incomplete type
make[1]: *** [arch/x86/kernel/asm-offsets.s] Error 1
make: *** [prepare0] Error 2

Yuck.

Am I missing something obvious, or did something get broken after this went in?

thanks
-john

Re: printk_ratelimited() not compiling

[Joe Perches]

On Thu, 2010-02-18 at 14:10 -0800, john stultz wrote:
> I thought printk_ratelimited would be perfect for an issue I'm
> having, but when I tried using it, replacing a printk I had, I got the
> following:

[]

> fs/namei.c:1075: error: variable ‘_rs’ has initializer but incomplete type

[]

> Am I missing something obvious, or did something get broken after this went in?

Dunno.
Maybe you typed printk_ratelimit not printk_ratelimited?
Can you send the code you're using?

I just tried it in drivers/net/tg3.c and it compiled fine.

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 7f82b02..cf9b0d8 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -1092,7 +1092,7 @@ static int tg3_mdio_init(struct tg3 *tp)
 
 	i = mdiobus_register(tp->mdio_bus);
 	if (i) {
-		printk(KERN_WARNING "%s: mdiobus_reg failed (0x%x)\n",
+		printk_ratelimited(KERN_WARNING "%s: mdiobus_reg failed (0x%x)\n
 			tp->dev->name, i);
 		mdiobus_free(tp->mdio_bus);
 		return i;

Re: printk_ratelimited() not compiling

[john stultz]

On Thu, 2010-02-18 at 14:10 -0800, john stultz wrote:
> Hey Joe, 
> 	So I thought printk_ratelimited would be perfect for an issue I'm
> having, but when I tried using it, replacing a printk I had, I got the
> following:
> 
> fs/namei.c:1075: error: variable ‘_rs’ has initializer but incomplete type
> fs/namei.c:1075: error: unknown field ‘interval’ specified in initializer
> fs/namei.c:1075: warning: excess elements in struct initializer
> fs/namei.c:1075: warning: (near initialization for ‘_rs’)
> fs/namei.c:1075: error: unknown field ‘burst’ specified in initializer
> fs/namei.c:1075: warning: excess elements in struct initializer
> fs/namei.c:1075: warning: (near initialization for ‘_rs’)
> fs/namei.c:1075: error: storage size of ‘_rs’ isn’t known
> fs/namei.c:1075: warning: unused variable ‘_rs’
> 
> 
> Thinking the issue was ratelimit.h wasn't included I tried that, but
> got:
> 
>  file included from include/linux/spinlock_types.h:18,
>                  from include/linux/ratelimit.h:5,
>                  from include/linux/kernel.h:21,
>                  from /home/jstultz/projects/linux/linux-2.6-git/arch/x86/include/asm/percpu.h:45,
>                  from /home/jstultz/projects/linux/linux-2.6-git/arch/x86/include/asm/current.h:5,
>                  from /home/jstultz/projects/linux/linux-2.6-git/arch/x86/include/asm/processor.h:15,
>                  from include/linux/prefetch.h:14,
>                  from include/linux/list.h:6,
>                  from include/linux/module.h:9,
>                  from include/linux/crypto.h:21,
>                  from arch/x86/kernel/asm-offsets_64.c:8,
>                  from arch/x86/kernel/asm-offsets.c:4:
> include/linux/lockdep.h:52: error: field ‘hash_entry’ has incomplete type
> include/linux/lockdep.h:57: error: field ‘lock_entry’ has incomplete type
> include/linux/lockdep.h:74: error: field ‘locks_after’ has incomplete type
> include/linux/lockdep.h:74: error: field ‘locks_before’ has incomplete type
> include/linux/lockdep.h:148: error: field ‘entry’ has incomplete type
> include/linux/lockdep.h:167: error: field ‘entry’ has incomplete type
> make[1]: *** [arch/x86/kernel/asm-offsets.s] Error 1
> make: *** [prepare0] Error 2
> 
> Yuck.
> 
> Am I missing something obvious, or did something get broken after this went in?

Ok. Solved it. I needed to #include <linux/ratelimit.h> in the file I
was adding the printk_ratelimited usage in, rather then where
printk_ratelimited is defined.

Maybe would it be better to move the printk_ratelimited definitions into
ratelimit.h so this would be more obvious?

Something like the following?

Signed-off-by: John Stultz <johnstul@us.ibm.com>

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 328bca6..9119b72 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -404,49 +405,6 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
 	({ if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); 0; })
 #endif
 
-/*
- * ratelimited messages with local ratelimit_state,
- * no local ratelimit_state used in the !PRINTK case
- */
-#ifdef CONFIG_PRINTK
-#define printk_ratelimited(fmt, ...)  ({		\
-	static struct ratelimit_state _rs = {		\
-		.interval = DEFAULT_RATELIMIT_INTERVAL, \
-		.burst = DEFAULT_RATELIMIT_BURST,       \
-	};                                              \
-							\
-	if (!__ratelimit(&_rs))                         \
-		printk(fmt, ##__VA_ARGS__);		\
-})
-#else
-/* No effect, but we still get type checking even in the !PRINTK case: */
-#define printk_ratelimited printk
-#endif
-
-#define pr_emerg_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_alert_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_crit_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_err_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_warning_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_notice_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_info_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
-/* no pr_cont_ratelimited, don't do that... */
-/* If you are writing a driver, please use dev_dbg instead */
-#if defined(DEBUG)
-#define pr_debug_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
-#else
-#define pr_debug_ratelimited(fmt, ...) \
-	({ if (0) printk_ratelimited(KERN_DEBUG pr_fmt(fmt), \
-				     ##__VA_ARGS__); 0; })
-#endif
 
 /*
  * General tracing related utility functions - trace_printk(),
diff --git a/include/linux/ratelimit.h b/include/linux/ratelimit.h
index 668cf1b..39cc4df 100644
--- a/include/linux/ratelimit.h
+++ b/include/linux/ratelimit.h
@@ -28,4 +28,48 @@ struct ratelimit_state {
 extern int ___ratelimit(struct ratelimit_state *rs, const char *func);
 #define __ratelimit(state) ___ratelimit(state, __func__)
 
+/*
+ * ratelimited messages with local ratelimit_state,
+ * no local ratelimit_state used in the !PRINTK case
+ */
+#ifdef CONFIG_PRINTK
+#define printk_ratelimited(fmt, ...)  ({		\
+	static struct ratelimit_state _rs = {		\
+		.interval = DEFAULT_RATELIMIT_INTERVAL, \
+		.burst = DEFAULT_RATELIMIT_BURST,       \
+	};                                              \
+							\
+	if (!__ratelimit(&_rs))                         \
+		printk(fmt, ##__VA_ARGS__);		\
+})
+#else
+/* No effect, but we still get type checking even in the !PRINTK case: */
+#define printk_ratelimited printk
+#endif
+
+#define pr_emerg_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_alert_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_crit_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_err_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_warning_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_notice_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_info_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
+/* no pr_cont_ratelimited, don't do that... */
+/* If you are writing a driver, please use dev_dbg instead */
+#if defined(DEBUG)
+#define pr_debug_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#else
+#define pr_debug_ratelimited(fmt, ...) \
+	({ if (0) printk_ratelimited(KERN_DEBUG pr_fmt(fmt), \
+				     ##__VA_ARGS__); 0; })
+#endif
+
 #endif /* _LINUX_RATELIMIT_H */

Re: printk_ratelimited() not compiling

[Joe Perches]

On Thu, 2010-02-18 at 14:36 -0800, john stultz wrote: 
> > Am I missing something obvious, or did something get broken after this went in?
> Ok. Solved it. I needed to #include <linux/ratelimit.h> in the file I
> was adding the printk_ratelimited usage in, rather then where
> printk_ratelimited is defined.
> 
> Maybe would it be better to move the printk_ratelimited definitions into
> ratelimit.h so this would be more obvious?

That's one option.

Probably the only places I tried it had an
#include <linux/net.h> somewhere which does an
#include <linux/ratelimit.h>

Personally, I think it'd be better to put
#include <linux/ratelimit.h> back in kernel.h
Commit 3fff4c42bd0a89869a0eb1e7874cc06ffa4aa0f5 removed it.

Re: printk_ratelimited() not compiling

[john stultz]

On Thu, 2010-02-18 at 15:23 -0800, Joe Perches wrote:
> On Thu, 2010-02-18 at 14:36 -0800, john stultz wrote: 
> > > Am I missing something obvious, or did something get broken after this went in?
> > Ok. Solved it. I needed to #include <linux/ratelimit.h> in the file I
> > was adding the printk_ratelimited usage in, rather then where
> > printk_ratelimited is defined.
> > 
> > Maybe would it be better to move the printk_ratelimited definitions into
> > ratelimit.h so this would be more obvious?
> 
> That's one option.
> 
> Probably the only places I tried it had an
> #include <linux/net.h> somewhere which does an
> #include <linux/ratelimit.h>
> 
> Personally, I think it'd be better to put
> #include <linux/ratelimit.h> back in kernel.h
> Commit 3fff4c42bd0a89869a0eb1e7874cc06ffa4aa0f5 removed it.

Right, that's what I tried first, but it doesn't build. :)

If ratelimit.h has to be included for it to work (which is fine by me),
it seems ratelimit.h would be the ideal place to define it.

CC'ing Peter to see what his thoughts are.

thanks
-john

Re: printk_ratelimited() not compiling

[john stultz]

On Thu, 2010-02-18 at 15:33 -0800, john stultz wrote:
> On Thu, 2010-02-18 at 15:23 -0800, Joe Perches wrote:
> > On Thu, 2010-02-18 at 14:36 -0800, john stultz wrote: 
> > > > Am I missing something obvious, or did something get broken after this went in?
> > > Ok. Solved it. I needed to #include <linux/ratelimit.h> in the file I
> > > was adding the printk_ratelimited usage in, rather then where
> > > printk_ratelimited is defined.
> > > 
> > > Maybe would it be better to move the printk_ratelimited definitions into
> > > ratelimit.h so this would be more obvious?
> > 
> > That's one option.
> > 
> > Probably the only places I tried it had an
> > #include <linux/net.h> somewhere which does an
> > #include <linux/ratelimit.h>
> > 
> > Personally, I think it'd be better to put
> > #include <linux/ratelimit.h> back in kernel.h
> > Commit 3fff4c42bd0a89869a0eb1e7874cc06ffa4aa0f5 removed it.
> 
> Right, that's what I tried first, but it doesn't build. :)
> 
> If ratelimit.h has to be included for it to work (which is fine by me),
> it seems ratelimit.h would be the ideal place to define it.
> 
> CC'ing Peter to see what his thoughts are.

Sorry, that should have been Ingo I cc'ed. Peter didn't make that
change.

thanks
-john

[RFC PATCH] printk_ratelimited: Move definitions to ratelimit.h, use DEFINE_RATELIMIT_STATE

[Joe Perches]

On Thu, 2010-02-18 at 15:49 -0800, john stultz wrote:
> On Thu, 2010-02-18 at 15:33 -0800, john stultz wrote:
> > On Thu, 2010-02-18 at 15:23 -0800, Joe Perches wrote:
> > > On Thu, 2010-02-18 at 14:36 -0800, john stultz wrote: 
> > > > > Am I missing something obvious, or did something get broken after this went in?
> > > > Ok. Solved it. I needed to #include <linux/ratelimit.h> in the file I
> > > > was adding the printk_ratelimited usage in, rather then where
> > > > printk_ratelimited is defined.
> > > > 
> > > > Maybe would it be better to move the printk_ratelimited definitions into
> > > > ratelimit.h so this would be more obvious?
> > > 
> > > That's one option.
> > > 
> > > Probably the only places I tried it had an
> > > #include <linux/net.h> somewhere which does an
> > > #include <linux/ratelimit.h>
> > > 
> > > Personally, I think it'd be better to put
> > > #include <linux/ratelimit.h> back in kernel.h
> > > Commit 3fff4c42bd0a89869a0eb1e7874cc06ffa4aa0f5 removed it.
> > 
> > Right, that's what I tried first, but it doesn't build. :)
> > 
> > If ratelimit.h has to be included for it to work (which is fine by me),
> > it seems ratelimit.h would be the ideal place to define it.
> > 
> > CC'ing Peter to see what his thoughts are.
> 
> Sorry, that should have been Ingo I cc'ed. Peter didn't make that
> change.

One possible problem with this is kernel.h needs to be
#included before ratelimit.h.

Maybe ratelimit.h needs to #include <linux/kernel.h>

------

Move the printk_ratelimited and pr_<level>_ratelimited
macro helpers to ratelimit.h

Use DEFINE_RATELIMIT_STATE

Signed-off-by: Joe Perches <joe@perches.com>
---
 include/linux/kernel.h    |   44 --------------------------------------------
 include/linux/ratelimit.h |   44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 328bca6..db8e089 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -405,50 +405,6 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
 #endif
 
 /*
- * ratelimited messages with local ratelimit_state,
- * no local ratelimit_state used in the !PRINTK case
- */
-#ifdef CONFIG_PRINTK
-#define printk_ratelimited(fmt, ...)  ({		\
-	static struct ratelimit_state _rs = {		\
-		.interval = DEFAULT_RATELIMIT_INTERVAL, \
-		.burst = DEFAULT_RATELIMIT_BURST,       \
-	};                                              \
-							\
-	if (!__ratelimit(&_rs))                         \
-		printk(fmt, ##__VA_ARGS__);		\
-})
-#else
-/* No effect, but we still get type checking even in the !PRINTK case: */
-#define printk_ratelimited printk
-#endif
-
-#define pr_emerg_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_alert_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_crit_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_err_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_warning_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_notice_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_info_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
-/* no pr_cont_ratelimited, don't do that... */
-/* If you are writing a driver, please use dev_dbg instead */
-#if defined(DEBUG)
-#define pr_debug_ratelimited(fmt, ...) \
-	printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
-#else
-#define pr_debug_ratelimited(fmt, ...) \
-	({ if (0) printk_ratelimited(KERN_DEBUG pr_fmt(fmt), \
-				     ##__VA_ARGS__); 0; })
-#endif
-
-/*
  * General tracing related utility functions - trace_printk(),
  * tracing_on/tracing_off and tracing_start()/tracing_stop
  *
diff --git a/include/linux/ratelimit.h b/include/linux/ratelimit.h
index 668cf1b..c892373 100644
--- a/include/linux/ratelimit.h
+++ b/include/linux/ratelimit.h
@@ -28,4 +28,48 @@ struct ratelimit_state {
 extern int ___ratelimit(struct ratelimit_state *rs, const char *func);
 #define __ratelimit(state) ___ratelimit(state, __func__)
 
+/*
+ * ratelimited messages with local ratelimit_state,
+ * no local ratelimit_state used in the !PRINTK case
+ */
+#ifdef CONFIG_PRINTK
+#define printk_ratelimited(fmt, ...)				\
+({								\
+	DEFINE_RATELIMIT_STATE(_rs,				\
+			       DEFAULT_RATELIMIT_INTERVAL,	\
+			       DEFAULT_RATELIMIT_BURST);	\
+								\
+	if (!__ratelimit(&_rs))					\
+		printk(fmt, ##__VA_ARGS__);			\
+})
+#else
+/* No effect, but we still get type checking even in the !PRINTK case: */
+#define printk_ratelimited printk
+#endif
+
+#define pr_emerg_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_alert_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_crit_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_err_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_warning_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_notice_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_info_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
+/* no pr_cont_ratelimited, don't do that... */
+/* If you are writing a driver, please use dev_dbg instead */
+#if defined(DEBUG)
+#define pr_debug_ratelimited(fmt, ...) \
+	printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#else
+#define pr_debug_ratelimited(fmt, ...) \
+	({ if (0) printk_ratelimited(KERN_DEBUG pr_fmt(fmt), \
+				     ##__VA_ARGS__); 0; })
+#endif
+
 #endif /* _LINUX_RATELIMIT_H */