Move the logging bits from kernel.h into logging.h so that there is a bit more logical separation of the generic from the logging specific parts. Add pr_<level>_once macros to logging.h. Signed-off-by: Joe Perches <joe@perches.com> --- include/linux/kernel.h | 246 +----------------------------------- include/linux/logging.h | 322 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 324 insertions(+), 244 deletions(-) create mode 100644 include/linux/logging.h diff --git a/include/linux/kernel.h b/include/linux/kernel.h index b526947..a8be3e4 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -17,13 +17,11 @@ #include <linux/bitops.h> #include <linux/log2.h> #include <linux/typecheck.h> +#include <linux/logging.h> #include <linux/dynamic_debug.h> #include <asm/byteorder.h> #include <asm/bug.h> -extern const char linux_banner[]; -extern const char linux_proc_banner[]; - #define USHRT_MAX ((u16)(~0U)) #define SHRT_MAX ((s16)(USHRT_MAX>>1)) #define SHRT_MIN ((s16)(-SHRT_MAX - 1)) @@ -42,7 +40,7 @@ extern const char linux_proc_banner[]; #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) #define __ALIGN_MASK(x, mask) __ALIGN_KERNEL_MASK((x), (mask)) #define PTR_ALIGN(p, a) ((typeof(p))ALIGN((unsigned long)(p), (a))) -#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0) +#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0) #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) @@ -110,31 +108,6 @@ extern const char linux_proc_banner[]; */ #define lower_32_bits(n) ((u32)(n)) -#define KERN_EMERG "<0>" /* system is unusable */ -#define KERN_ALERT "<1>" /* action must be taken immediately */ -#define KERN_CRIT "<2>" /* critical conditions */ -#define KERN_ERR "<3>" /* error conditions */ -#define KERN_WARNING "<4>" /* warning conditions */ -#define KERN_NOTICE "<5>" /* normal but significant condition */ -#define KERN_INFO "<6>" /* ...
I detest patches like this.
The code movement makes it really hard to see what changed. So if you
do code movement, please do _only_ code movement, and send a separate
patch that actually changes things.
Linus
--
Move the logging bits from kernel.h into logging.h so that there is a bit more logical separation of the generic from the logging specific parts. Signed-off-by: Joe Perches <joe@perches.com> --- diff from V1 - Don't add pr_<level>_once macros include/linux/kernel.h | 244 +-------------------------------------- include/linux/logging.h | 295 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 296 insertions(+), 243 deletions(-) create mode 100644 include/linux/logging.h diff --git a/include/linux/kernel.h b/include/linux/kernel.h index b526947..b7a1595 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -17,13 +17,11 @@ #include <linux/bitops.h> #include <linux/log2.h> #include <linux/typecheck.h> +#include <linux/logging.h> #include <linux/dynamic_debug.h> #include <asm/byteorder.h> #include <asm/bug.h> -extern const char linux_banner[]; -extern const char linux_proc_banner[]; - #define USHRT_MAX ((u16)(~0U)) #define SHRT_MAX ((s16)(USHRT_MAX>>1)) #define SHRT_MIN ((s16)(-SHRT_MAX - 1)) @@ -110,31 +108,6 @@ extern const char linux_proc_banner[]; */ #define lower_32_bits(n) ((u32)(n)) -#define KERN_EMERG "<0>" /* system is unusable */ -#define KERN_ALERT "<1>" /* action must be taken immediately */ -#define KERN_CRIT "<2>" /* critical conditions */ -#define KERN_ERR "<3>" /* error conditions */ -#define KERN_WARNING "<4>" /* warning conditions */ -#define KERN_NOTICE "<5>" /* normal but significant condition */ -#define KERN_INFO "<6>" /* informational */ -#define KERN_DEBUG "<7>" /* debug-level messages */ - -/* Use the default kernel loglevel */ -#define KERN_DEFAULT "<d>" -/* - * Annotation for a "continued" line of log printout (only done after a - * line that had no enclosing \n). Only to be used by core/arch code - * during early bootup (a continued line is not SMP-safe otherwise). - */ -#define KERN_CONT "<c>" - -extern int console_printk[]; - -#define console_loglevel ...
So where do the extra 53 lines come from now?
This still is clearly not just code movement, and it's impossible for
me to see what actually changed. Something must have.
Linus
--
Differences in logging.h to original kernel.h were done for cleanliness and checkpatch. o comment added for purpose of struct va_format o function no_print() broken into multiple lines o #ifdef CONFIG_PRINTK vprintk/printk block and #else block moved together o printk_ratelimit and related now have another CONFIG_PRINTK and #else block o pr_debug and pr_debug_ratelimit statement If you want a nearly identical line count for logging.h, followed by a cleanup only pass of logging.h, I'll do that. Is that what you want? I didn't recompile and verify this second patch as all I did was remove the unused for now pr_<level>_once macros I added. It takes my slowish setup quite a while to do full compiles for allyesconfig/allmodconfig/ allnoconfig/defconfig/CONFIG_EMBEDDED/CONFIG_PRINTK=n variants though so it'd be another day or so before it'd be verified. --
Yet another reason why I detest mindless use of checkpatch. And as a maintainer, I balance the value of code movement patches in particular against how it breaks other people's patches, and how unless I do *very* careful review, people can sneak in changes that could either (a) break code accidentally, or even (b) introduce security holes deliberately. (i.e., if I was in charge of the cyber warfare division of some large nation state, a really obvious way of trying to insert vulnerabilities into code is to send "cleanup" patches that move code around, and introduce some subtle buffer overrun or other security change.) As a result, I spend extra time reviewing patches that move code around, and I **really** dislike patches that try to do any kind of cleanup (checkpatch.pl or otherwise) at the same time as they move code around. - Ted --
If it makes you happier, I didn't use it here nor do I generally use checkpatch, (just ask Andrew Morton), I just So fine, I'll do a minimal code movement only patch followed by a style cleansing patch. Anyone else have an opinion on using logging.h vs printk.h as an include name? --
Move the logging bits from kernel.h into printk.h so that there is a bit more logical separation of the generic from the printk logging specific parts. Signed-off-by: Joe Perches <joe@perches.com> --- v3 differences: Rename logging.h to printk.h Direct movement of lines from kernel.h to printk.h One trailing space deleted from a comment to keep git am quiet Compiled x86 defconfig, allnoconfig, allmodconfig and CONFIG_PRINTK=n only include/linux/kernel.h | 244 +----------------------------------------------- include/linux/printk.h | 247 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 248 insertions(+), 243 deletions(-) diff --git a/include/linux/kernel.h b/include/linux/kernel.h index b526947..b6de9a6 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -17,13 +17,11 @@ #include <linux/bitops.h> #include <linux/log2.h> #include <linux/typecheck.h> +#include <linux/printk.h> #include <linux/dynamic_debug.h> #include <asm/byteorder.h> #include <asm/bug.h> -extern const char linux_banner[]; -extern const char linux_proc_banner[]; - #define USHRT_MAX ((u16)(~0U)) #define SHRT_MAX ((s16)(USHRT_MAX>>1)) #define SHRT_MIN ((s16)(-SHRT_MAX - 1)) @@ -110,31 +108,6 @@ extern const char linux_proc_banner[]; */ #define lower_32_bits(n) ((u32)(n)) -#define KERN_EMERG "<0>" /* system is unusable */ -#define KERN_ALERT "<1>" /* action must be taken immediately */ -#define KERN_CRIT "<2>" /* critical conditions */ -#define KERN_ERR "<3>" /* error conditions */ -#define KERN_WARNING "<4>" /* warning conditions */ -#define KERN_NOTICE "<5>" /* normal but significant condition */ -#define KERN_INFO "<6>" /* informational */ -#define KERN_DEBUG "<7>" /* debug-level messages */ - -/* Use the default kernel loglevel */ -#define KERN_DEFAULT "<d>" -/* - * Annotation for a "continued" line of log printout (only done after a - * line that had no enclosing \n). Only to be used by core/arch code - * during early ...
Why linux_banner is moved? Why linux_proc_banner is moved? Why header is not named printk.h? Why random stuff like log_buf_kexec_setup() and hexdump is moved? --
hexdump functions are logging message related
as is log_buf_kexec_setup.
hexdump should/will eventually have an
#ifdef CONFIG_PRINTK
external function prototypes
#else
static inline void
hex_dump_to_buffer(const void *buf, size_t len,
int rowsize, int groupsize,
char *linebuf, size_t linebuflen, bool ascii) {}
static inline void
print_hex_dump(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
const void *buf, size_t len, bool ascii) {}
static inline void
print_hex_dump_bytes(const char *prefix_str, int prefix_type,
const void *buf, size_t len) {}
#endif
so the static strings of the call sites can be eliminated
when !CONFIG_PRINTK.
--
Checking again, linux_proc_banner is only used in one seq_printf and is not used in a printk. $ git grep -w linux_proc_banner * fs/proc/version.c: seq_printf(m, linux_proc_banner, include/linux/logging.h:extern const char linux_proc_banner[]; init/version.c:const char linux_proc_banner[] = I supposed it could be removed from kernel.h or logging.h/printk.h and just declared extern in fs/proc/version.c or moved to init.h or maybe even version.h. init.h already has other extern variables that are declared in init/main.c so that might make some sense. --
Move the logging bits from kernel.h into printk.h so that
there is a bit more logical separation of the generic
kernel.h contents from the printk/logging specific parts.
Signed-off-by: Joe Perches <joe@perches.com>
---
V4: Update for dmesg_restrict
V3: Rename logging.h to printk.h
Direct movement of lines from kernel.h to printk.h
One trailing space deleted from a comment to keep git am quiet
Compiled x86 defconfig, allnoconfig, allmodconfig and CONFIG_PRINTK=n only
include/linux/kernel.h | 245 +-----------------------------------------------
include/linux/printk.h | 248 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 249 insertions(+), 244 deletions(-)
create mode 100644 include/linux/printk.h
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index fc3da9e..b6de9a6 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -17,13 +17,11 @@
#include <linux/bitops.h>
#include <linux/log2.h>
#include <linux/typecheck.h>
+#include <linux/printk.h>
#include <linux/dynamic_debug.h>
#include <asm/byteorder.h>
#include <asm/bug.h>
-extern const char linux_banner[];
-extern const char linux_proc_banner[];
-
#define USHRT_MAX ((u16)(~0U))
#define SHRT_MAX ((s16)(USHRT_MAX>>1))
#define SHRT_MIN ((s16)(-SHRT_MAX - 1))
@@ -110,31 +108,6 @@ extern const char linux_proc_banner[];
*/
#define lower_32_bits(n) ((u32)(n))
-#define KERN_EMERG "<0>" /* system is unusable */
-#define KERN_ALERT "<1>" /* action must be taken immediately */
-#define KERN_CRIT "<2>" /* critical conditions */
-#define KERN_ERR "<3>" /* error conditions */
-#define KERN_WARNING "<4>" /* warning conditions */
-#define KERN_NOTICE "<5>" /* normal but significant condition */
-#define KERN_INFO "<6>" /* informational */
-#define KERN_DEBUG "<7>" /* debug-level messages */
-
-/* Use the default kernel loglevel */
-#define KERN_DEFAULT "<d>"
-/*
- * Annotation for a "continued" line of log printout (only done after a
- * ...