[RFC] Remove most all #define pr_fmt(fmt) lines

From: Joe Perches
Date: Tue Mar 27 2012 - 13:23:51 EST

Next message: Steven Rostedt: "[PATCH][GIT PULL][v3.4] tracing: Fix ent_size in trace output"
Previous message: Ted Ts'o: "Re: Correct way to include siginfo.h?"
Next in thread: Clemens Ladisch: "Re: [RFC] Remove most all #define pr_fmt(fmt) lines"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

For the next RC window I'd like to convert the
current pr_fmt #define in include/linux/printk.h
so that non-debug uses of pr_<level> always get a
standardized KBUILD_MODNAME prefix when pr_fmt is
otherwise undefined.

Basically, add #define pr_fmt_std and pr_fmt_dbg
and convert the existing pr_fmt uses in printk.h
to these new #defines.

This will allow deletion of all of the 600 or so
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
sprinkled around the kernel tree.

This will also allow separate and possibly clearer
uses of dynamic_debugging or adding __func__ only
to pr_debug uses.

Below this rfc patch is more commentary and another
suggested patch to a Makefile to improve prefixing.

include/linux/printk.h | 114 ++++++++++++++++++++++++++++++------------------
1 files changed, 72 insertions(+), 42 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 0525927..d9c05f4 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -158,48 +158,78 @@ static inline void setup_log_buf(int early)

extern void dump_stack(void) __cold;

+/* Define separate possible prefixes for pr_debug and other pr_<level> uses
+ *
+ * pr_fmt_dbg for pr_debug and pr_devel (default: blank)
+ * pr_fmt_std for everything else (default: KBUILD_MODNAME ": ")
+ *
+ * if pr_fmt is defined before this inclusion, it is
+ * used for both pr_fmt_dbg and pr_fmt_std
+ *
+ * nb: Most debugging can now be done with dynamic debug.
+ * dynamic debug can optionally prefix KBUILD_MODNAME and/or __func__
+ * see Documentation/dynamic-debug-howto.txt
+ */
+
#ifndef pr_fmt
+
+#ifndef pr_fmt_std
+#define pr_fmt_std(fmt) KBUILD_MODNAME ": " fmt
+#endif
+#ifndef pr_fmt_dbg
+#define pr_fmt_dbg(fmt) fmt
+#endif
#define pr_fmt(fmt) fmt
+
+#else
+
+#ifndef pr_fmt_std
+#define pr_fmt_std(fmt...) pr_fmt(fmt)
+#endif
+#ifndef pr_fmt_dbg
+#define pr_fmt_dbg(fmt...) pr_fmt(fmt)
+#endif
+
#endif

-#define pr_emerg(fmt, ...) \
- printk(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_alert(fmt, ...) \
- printk(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_crit(fmt, ...) \
- printk(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_err(fmt, ...) \
- printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_warning(fmt, ...) \
- printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_emerg(fmt, ...) \
+ printk(KERN_EMERG pr_fmt_std(fmt), ##__VA_ARGS__)
+#define pr_alert(fmt, ...) \
+ printk(KERN_ALERT pr_fmt_std(fmt), ##__VA_ARGS__)
+#define pr_crit(fmt, ...) \
+ printk(KERN_CRIT pr_fmt_std(fmt), ##__VA_ARGS__)
+#define pr_err(fmt, ...) \
+ printk(KERN_ERR pr_fmt_std(fmt), ##__VA_ARGS__)
+#define pr_warning(fmt, ...) \
+ printk(KERN_WARNING pr_fmt_std(fmt), ##__VA_ARGS__)
#define pr_warn pr_warning
-#define pr_notice(fmt, ...) \
- printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_info(fmt, ...) \
- printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_cont(fmt, ...) \
+#define pr_notice(fmt, ...) \
+ printk(KERN_NOTICE pr_fmt_std(fmt), ##__VA_ARGS__)
+#define pr_info(fmt, ...) \
+ printk(KERN_INFO pr_fmt_std(fmt), ##__VA_ARGS__)
+#define pr_cont(fmt, ...) \
printk(KERN_CONT fmt, ##__VA_ARGS__)

/* pr_devel() should produce zero code unless DEBUG is defined */
#ifdef DEBUG
-#define pr_devel(fmt, ...) \
- printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_devel(fmt, ...) \
+ printk(KERN_DEBUG pr_fmt_dbg(fmt), ##__VA_ARGS__)
#else
-#define pr_devel(fmt, ...) \
- no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_devel(fmt, ...) \
+ no_printk(KERN_DEBUG pr_fmt_dbg(fmt), ##__VA_ARGS__)
#endif

/* If you are writing a driver, please use dev_dbg instead */
#if defined(CONFIG_DYNAMIC_DEBUG)
/* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
-#define pr_debug(fmt, ...) \
+#define pr_debug(fmt, ...) \
dynamic_pr_debug(fmt, ##__VA_ARGS__)
#elif defined(DEBUG)
-#define pr_debug(fmt, ...) \
- printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_debug(fmt, ...) \
+ printk(KERN_DEBUG pr_fmt_dbg(fmt), ##__VA_ARGS__)
#else
-#define pr_debug(fmt, ...) \
- no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_debug(fmt, ...) \
+ no_printk(KERN_DEBUG pr_fmt_dbg(fmt), ##__VA_ARGS__)
#endif

/*
@@ -222,28 +252,28 @@ extern void dump_stack(void) __cold;
#endif

#define pr_emerg_once(fmt, ...) \
- printk_once(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_EMERG pr_fmt_std(fmt), ##__VA_ARGS__)
#define pr_alert_once(fmt, ...) \
- printk_once(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_ALERT pr_fmt_std(fmt), ##__VA_ARGS__)
#define pr_crit_once(fmt, ...) \
- printk_once(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_CRIT pr_fmt_std(fmt), ##__VA_ARGS__)
#define pr_err_once(fmt, ...) \
- printk_once(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_ERR pr_fmt_std(fmt), ##__VA_ARGS__)
#define pr_warn_once(fmt, ...) \
- printk_once(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_WARNING pr_fmt_std(fmt), ##__VA_ARGS__)
#define pr_notice_once(fmt, ...) \
- printk_once(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_NOTICE pr_fmt_std(fmt), ##__VA_ARGS__)
#define pr_info_once(fmt, ...) \
- printk_once(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_INFO pr_fmt_std(fmt), ##__VA_ARGS__)
#define pr_cont_once(fmt, ...) \
printk_once(KERN_CONT pr_fmt(fmt), ##__VA_ARGS__)
/* If you are writing a driver, please use dev_dbg instead */
#if defined(DEBUG)
#define pr_debug_once(fmt, ...) \
- printk_once(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+ printk_once(KERN_DEBUG pr_fmt_dbg(fmt), ##__VA_ARGS__)
#else
#define pr_debug_once(fmt, ...) \
- no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+ no_printk(KERN_DEBUG pr_fmt_dbg(fmt), ##__VA_ARGS__)
#endif

/*
@@ -266,27 +296,27 @@ extern void dump_stack(void) __cold;
#endif

#define pr_emerg_ratelimited(fmt, ...) \
- printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
+ printk_ratelimited(KERN_EMERG pr_fmt_std(fmt), ##__VA_ARGS__)
#define pr_alert_ratelimited(fmt, ...) \
- printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
+ printk_ratelimited(KERN_ALERT pr_fmt_std(fmt), ##__VA_ARGS__)
#define pr_crit_ratelimited(fmt, ...) \
- printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
+ printk_ratelimited(KERN_CRIT pr_fmt_std(fmt), ##__VA_ARGS__)
#define pr_err_ratelimited(fmt, ...) \
- printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
+ printk_ratelimited(KERN_ERR pr_fmt_std(fmt), ##__VA_ARGS__)
#define pr_warn_ratelimited(fmt, ...) \
- printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
+ printk_ratelimited(KERN_WARNING pr_fmt_std(fmt), ##__VA_ARGS__)
#define pr_notice_ratelimited(fmt, ...) \
- printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
+ printk_ratelimited(KERN_NOTICE pr_fmt_std(fmt), ##__VA_ARGS__)
#define pr_info_ratelimited(fmt, ...) \
- printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
+ printk_ratelimited(KERN_INFO pr_fmt_std(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__)
+ printk_ratelimited(KERN_DEBUG pr_fmt_dbg(fmt), ##__VA_ARGS__)
#else
#define pr_debug_ratelimited(fmt, ...) \
- no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+ no_printk(KERN_DEBUG pr_fmt_dbg(fmt), ##__VA_ARGS__)
#endif

enum {

-------------------------

This does have some side-effects to current uses of
pr_<level> without a pre-existing pr_fmt.

All non-debug pr_<level> uses will have a prefix.

pr_debug can be prefixed by dynamic_debug or have
__func__ added independently of other pr_<level> uses.

For instance, drivers/regulator/ uses pr_err and pr_crit
for dmesg output, but the messages are not prefixed
by subsystem at all.

With this change, objects like drivers/regulator/core.o
are prefixed with "core: ". This is somewhat senseless
and may prompt Makefile changes to make the prefixes for
some objects more sensible.

For example, modifying the Makefile to bundle objects
together with a specific name can change the prefixes.
Here the prefix becomes "regulator: "

diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 94b5274..93fff05 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -2,8 +2,10 @@
# Makefile for regulator drivers.
#

+regulator-y := core.o dummy.o fixed-helper.o
+regulator-objs := $(regulator-y)

-obj-$(CONFIG_REGULATOR) += core.o dummy.o fixed-helper.o
+obj-$(CONFIG_REGULATOR) += regulator.o
obj-$(CONFIG_OF) += of_regulator.o
obj-$(CONFIG_REGULATOR_FIXED_VOLTAGE) += fixed.o
obj-$(CONFIG_REGULATOR_VIRTUAL_CONSUMER) += virtual.o

------------------

Any objections or other suggestions/improvements?

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

Next message: Steven Rostedt: "[PATCH][GIT PULL][v3.4] tracing: Fix ent_size in trace output"
Previous message: Ted Ts'o: "Re: Correct way to include siginfo.h?"
Next in thread: Clemens Ladisch: "Re: [RFC] Remove most all #define pr_fmt(fmt) lines"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]