[PATCH 0/4] printk cleanups

[PATCH 0/4] printk cleanups

[Namhyung Kim]

Hello,

This patchset tries to remove sparse warnings on printk routines.
All patches in this series are simple, one-liner and don't waste
your time much. Please take a look and consider applying it. :-)

Thanks.

---
Namhyung Kim (4):
  printk: Fixup declaration of kmsg_reasons
  printk: Add lock context annotation
  printk: Change type of 'boot_delay' to int *
  printk: Declare printk_ratelimit_state in ratelimit.h

 include/linux/ratelimit.h |    4 ++++
 kernel/printk.c           |    5 +++--
 kernel/sysctl.c           |    2 --
 3 files changed, 7 insertions(+), 4 deletions(-)

[PATCH 1/4] printk: Fixup declaration of kmsg_reasons

[Namhyung Kim]

Move redundant 'const' after '*' to make pointer itself const

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 kernel/printk.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index 8fe465a..a491251 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -1511,7 +1511,7 @@ int kmsg_dump_unregister(struct kmsg_dumper *dumper)
 }
 EXPORT_SYMBOL_GPL(kmsg_dump_unregister);
 
-static const char const *kmsg_reasons[] = {
+static const char * const kmsg_reasons[] = {
 	[KMSG_DUMP_OOPS]	= "oops",
 	[KMSG_DUMP_PANIC]	= "panic",
 	[KMSG_DUMP_KEXEC]	= "kexec",
-- 
1.7.0.4

[PATCH 2/4] printk: Add lock context annotation

[Namhyung Kim]

acquire_console_semaphore_for_printk() releases logbuf_lock but
was missing proper annotation. Add it.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 kernel/printk.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index a491251..a7aa858 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -647,6 +647,7 @@ static inline int can_use_console(unsigned int cpu)
  * released but interrupts still disabled.
  */
 static int acquire_console_semaphore_for_printk(unsigned int cpu)
+	__releases(&logbuf_lock)
 {
 	int retval = 0;
 
-- 
1.7.0.4

[PATCH 3/4] printk: Change type of 'boot_delay' to int *

[Namhyung Kim]

get_option() takes its 2nd arg as int * so passing boot_delay to it
caused following warnings from sparse:

 kernel/printk.c:223:27: warning: incorrect type in argument 2 (different signedness)
 kernel/printk.c:223:27:    expected int *pint
 kernel/printk.c:223:27:    got unsigned int static [toplevel] *<noident>

Since boot_delay can't grow more than 10,000 changing it to 'int *'
will not produce any problem.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 kernel/printk.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index a7aa858..e5a5c13 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -210,7 +210,7 @@ __setup("log_buf_len=", log_buf_len_setup);
 
 #ifdef CONFIG_BOOT_PRINTK_DELAY
 
-static unsigned int boot_delay; /* msecs delay after each printk during bootup */
+static int boot_delay; /* msecs delay after each printk during bootup */
 static unsigned long long loops_per_msec;	/* based on boot_delay */
 
 static int __init boot_delay_setup(char *str)
-- 
1.7.0.4

[PATCH 4/4] printk: Declare printk_ratelimit_state in ratelimit.h

[Namhyung Kim]

Adding declaration of printk_ratelimit_state in ratelimit.h removes
potential build breakage and following sparse warning:

 kernel/printk.c:1426:1: warning: symbol 'printk_ratelimit_state' was not declared. Should it be static?

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 include/linux/ratelimit.h |    4 ++++
 kernel/sysctl.c           |    2 --
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/ratelimit.h b/include/linux/ratelimit.h
index 8f69d09..55e1bbc 100644
--- a/include/linux/ratelimit.h
+++ b/include/linux/ratelimit.h
@@ -36,6 +36,10 @@ static inline void ratelimit_state_init(struct ratelimit_state *rs,
 	rs->begin = 0;
 }
 
+#ifdef CONFIG_PRINTK
+extern struct ratelimit_state printk_ratelimit_state;
+#endif
+
 extern int ___ratelimit(struct ratelimit_state *rs, const char *func);
 #define __ratelimit(state) ___ratelimit(state, __func__)
 
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index f88552c..720ab2b 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -161,8 +161,6 @@ extern int no_unaligned_warning;
 extern int unaligned_dump_stack;
 #endif
 
-extern struct ratelimit_state printk_ratelimit_state;
-
 #ifdef CONFIG_PROC_SYSCTL
 static int proc_do_cad_pid(struct ctl_table *table, int write,
 		  void __user *buffer, size_t *lenp, loff_t *ppos);
-- 
1.7.0.4

Re: [PATCH 4/4] printk: Declare printk_ratelimit_state in ratelimit.h

[Andrew Morton]

On Sun, 19 Sep 2010 00:53:02 +0900
Namhyung Kim <namhyung@gmail.com> wrote:

> Adding declaration of printk_ratelimit_state in ratelimit.h removes
> potential build breakage and following sparse warning:
> 
>  kernel/printk.c:1426:1: warning: symbol 'printk_ratelimit_state' was not declared. Should it be static?
> 
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
> ---
>  include/linux/ratelimit.h |    4 ++++
>  kernel/sysctl.c           |    2 --
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/ratelimit.h b/include/linux/ratelimit.h
> index 8f69d09..55e1bbc 100644
> --- a/include/linux/ratelimit.h
> +++ b/include/linux/ratelimit.h
> @@ -36,6 +36,10 @@ static inline void ratelimit_state_init(struct ratelimit_state *rs,
>  	rs->begin = 0;
>  }
>  
> +#ifdef CONFIG_PRINTK
> +extern struct ratelimit_state printk_ratelimit_state;
> +#endif
> +

We don't actually need the ifdefs there.

If we remove them then it adds a risk that someone will discover their
error at link-timer rather than at compile-time.  On the other hand,
removing the ifdefs deuglifies the code for everyone, for ever. 
Personally I prefer to not have the ifdefs.

--- a/include/linux/ratelimit.h~printk-declare-printk_ratelimit_state-in-ratelimith-fix
+++ a/include/linux/ratelimit.h
@@ -36,9 +36,7 @@ static inline void ratelimit_state_init(
 	rs->begin = 0;
 }
 
-#ifdef CONFIG_PRINTK
 extern struct ratelimit_state printk_ratelimit_state;
-#endif
 
 extern int ___ratelimit(struct ratelimit_state *rs, const char *func);
 #define __ratelimit(state) ___ratelimit(state, __func__)
diff -puN kernel/sysctl.c~printk-declare-printk_ratelimit_state-in-ratelimith-fix kernel/sysctl.c
_