Re: [PATCH v2] printk: move dump stack related code to lib/dump_stack.c

[Petr Mladek]

On Tue 2018-02-13 17:35:05, Sergey Senozhatsky wrote:
> On (02/13/18 15:28), Dave Young wrote:
> > 
> > dump_stack related stuff should belong to lib/dump_stack.c thus move them
> > there. Also conditionally compile lib/dump_stack.c since dump_stack code
> > does not make sense if printk is disabled.
> > 
> > Signed-off-by: Dave Young 
> > Suggested-by: Steven Rostedt 
> > Suggested-by: Sergey Senozhatsky 
> 
> Reviewed-by: Sergey Senozhatsky 

Looks OK to me as well.

Reviewed-by: Petr Mladek 

I have pushed it into printk.git, branch for-4.17.


> The patch also saves us some additional memory on !PRINTK configs.
> For example things like this
> 
>   if (kobj->state_initialized) {
>   /* do not error out as sometimes we can recover */
>   printk(KERN_ERR "kobject (%p): tried to init an initialized "
>  "object, something is seriously wrong.\n", kobj);
>   dump_stack();
>   }
> 
> now we won't have that dump_stack() call, which does not seems to be doing
> anything useful anyway when printk() is not available.

Yup.

Best Regards,
Petr

Re: [PATCH v2] printk: move dump stack related code to lib/dump_stack.c

[Petr Mladek]

On Tue 2018-02-13 17:35:05, Sergey Senozhatsky wrote:
> On (02/13/18 15:28), Dave Young wrote:
> > 
> > dump_stack related stuff should belong to lib/dump_stack.c thus move them
> > there. Also conditionally compile lib/dump_stack.c since dump_stack code
> > does not make sense if printk is disabled.
> > 
> > Signed-off-by: Dave Young 
> > Suggested-by: Steven Rostedt 
> > Suggested-by: Sergey Senozhatsky 
> 
> Reviewed-by: Sergey Senozhatsky 

Looks OK to me as well.

Reviewed-by: Petr Mladek 

I have pushed it into printk.git, branch for-4.17.


> The patch also saves us some additional memory on !PRINTK configs.
> For example things like this
> 
>   if (kobj->state_initialized) {
>   /* do not error out as sometimes we can recover */
>   printk(KERN_ERR "kobject (%p): tried to init an initialized "
>  "object, something is seriously wrong.\n", kobj);
>   dump_stack();
>   }
> 
> now we won't have that dump_stack() call, which does not seems to be doing
> anything useful anyway when printk() is not available.

Yup.

Best Regards,
Petr

Re: [PATCH v2] printk: move dump stack related code to lib/dump_stack.c

[Sergey Senozhatsky]

On (02/13/18 15:28), Dave Young wrote:
> 
> dump_stack related stuff should belong to lib/dump_stack.c thus move them
> there. Also conditionally compile lib/dump_stack.c since dump_stack code
> does not make sense if printk is disabled.
> 
> Signed-off-by: Dave Young 
> Suggested-by: Steven Rostedt 
> Suggested-by: Sergey Senozhatsky 

Reviewed-by: Sergey Senozhatsky 


The patch also saves us some additional memory on !PRINTK configs.
For example things like this

if (kobj->state_initialized) {
/* do not error out as sometimes we can recover */
printk(KERN_ERR "kobject (%p): tried to init an initialized "
   "object, something is seriously wrong.\n", kobj);
dump_stack();
}

now we won't have that dump_stack() call, which does not seems to be doing
anything useful anyway when printk() is not available.

-ss

Re: [PATCH v2] printk: move dump stack related code to lib/dump_stack.c

[Sergey Senozhatsky]

On (02/13/18 15:28), Dave Young wrote:
> 
> dump_stack related stuff should belong to lib/dump_stack.c thus move them
> there. Also conditionally compile lib/dump_stack.c since dump_stack code
> does not make sense if printk is disabled.
> 
> Signed-off-by: Dave Young 
> Suggested-by: Steven Rostedt 
> Suggested-by: Sergey Senozhatsky 

Reviewed-by: Sergey Senozhatsky 


The patch also saves us some additional memory on !PRINTK configs.
For example things like this

if (kobj->state_initialized) {
/* do not error out as sometimes we can recover */
printk(KERN_ERR "kobject (%p): tried to init an initialized "
   "object, something is seriously wrong.\n", kobj);
dump_stack();
}

now we won't have that dump_stack() call, which does not seems to be doing
anything useful anyway when printk() is not available.

-ss

[PATCH v2] printk: move dump stack related code to lib/dump_stack.c

[Dave Young]

dump_stack related stuff should belong to lib/dump_stack.c thus move them
there. Also conditionally compile lib/dump_stack.c since dump_stack code
does not make sense if printk is disabled.

Signed-off-by: Dave Young 
Suggested-by: Steven Rostedt 
Suggested-by: Sergey Senozhatsky 
---
Note: dump stack etc still share printk.h as before, I would keep it as is
since a lot of files need it.
 include/linux/printk.h |7 -
 kernel/printk/printk.c |   60 -
 lib/Makefile   |3 +-
 lib/dump_stack.c   |   60 +
 4 files changed, 67 insertions(+), 63 deletions(-)

--- linux-x86.orig/kernel/printk/printk.c
+++ linux-x86/kernel/printk/printk.c
@@ -42,13 +42,11 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
@@ -3257,62 +3255,4 @@ void kmsg_dump_rewind(struct kmsg_dumper
 }
 EXPORT_SYMBOL_GPL(kmsg_dump_rewind);
 
-static char dump_stack_arch_desc_str[128];
-
-/**
- * dump_stack_set_arch_desc - set arch-specific str to show with task dumps
- * @fmt: printf-style format string
- * @...: arguments for the format string
- *
- * The configured string will be printed right after utsname during task
- * dumps.  Usually used to add arch-specific system identifiers.  If an
- * arch wants to make use of such an ID string, it should initialize this
- * as soon as possible during boot.
- */
-void __init dump_stack_set_arch_desc(const char *fmt, ...)
-{
-   va_list args;
-
-   va_start(args, fmt);
-   vsnprintf(dump_stack_arch_desc_str, sizeof(dump_stack_arch_desc_str),
- fmt, args);
-   va_end(args);
-}
-
-/**
- * dump_stack_print_info - print generic debug info for dump_stack()
- * @log_lvl: log level
- *
- * Arch-specific dump_stack() implementations can use this function to
- * print out the same debug information as the generic dump_stack().
- */
-void dump_stack_print_info(const char *log_lvl)
-{
-   printk("%sCPU: %d PID: %d Comm: %.20s %s%s %s %.*s\n",
-  log_lvl, raw_smp_processor_id(), current->pid, current->comm,
-  kexec_crash_loaded() ? "Kdump: loaded " : "",
-  print_tainted(),
-  init_utsname()->release,
-  (int)strcspn(init_utsname()->version, " "),
-  init_utsname()->version);
-
-   if (dump_stack_arch_desc_str[0] != '\0')
-   printk("%sHardware name: %s\n",
-  log_lvl, dump_stack_arch_desc_str);
-
-   print_worker_info(log_lvl, current);
-}
-
-/**
- * show_regs_print_info - print generic debug info for show_regs()
- * @log_lvl: log level
- *
- * show_regs() implementations can use this function to print out generic
- * debug information.
- */
-void show_regs_print_info(const char *log_lvl)
-{
-   dump_stack_print_info(log_lvl);
-}
-
 #endif
--- linux-x86.orig/lib/dump_stack.c
+++ linux-x86/lib/dump_stack.c
@@ -10,6 +10,66 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+
+static char dump_stack_arch_desc_str[128];
+
+/**
+ * dump_stack_set_arch_desc - set arch-specific str to show with task dumps
+ * @fmt: printf-style format string
+ * @...: arguments for the format string
+ *
+ * The configured string will be printed right after utsname during task
+ * dumps.  Usually used to add arch-specific system identifiers.  If an
+ * arch wants to make use of such an ID string, it should initialize this
+ * as soon as possible during boot.
+ */
+void __init dump_stack_set_arch_desc(const char *fmt, ...)
+{
+   va_list args;
+
+   va_start(args, fmt);
+   vsnprintf(dump_stack_arch_desc_str, sizeof(dump_stack_arch_desc_str),
+ fmt, args);
+   va_end(args);
+}
+
+/**
+ * dump_stack_print_info - print generic debug info for dump_stack()
+ * @log_lvl: log level
+ *
+ * Arch-specific dump_stack() implementations can use this function to
+ * print out the same debug information as the generic dump_stack().
+ */
+void dump_stack_print_info(const char *log_lvl)
+{
+   printk("%sCPU: %d PID: %d Comm: %.20s %s%s %s %.*s\n",
+  log_lvl, raw_smp_processor_id(), current->pid, current->comm,
+  kexec_crash_loaded() ? "Kdump: loaded " : "",
+  print_tainted(),
+  init_utsname()->release,
+  (int)strcspn(init_utsname()->version, " "),
+  init_utsname()->version);
+
+   if (dump_stack_arch_desc_str[0] != '\0')
+   printk("%sHardware name: %s\n",
+  log_lvl, dump_stack_arch_desc_str);
+
+   print_worker_info(log_lvl, current);
+}
+
+/**
+ * show_regs_print_info - print generic debug info for show_regs()
+ * @log_lvl: log level
+ *
+ * show_regs() implementations can use this function to print out generic
+ * debug information.
+ */
+void 

[PATCH v2] printk: move dump stack related code to lib/dump_stack.c

[Dave Young]

dump_stack related stuff should belong to lib/dump_stack.c thus move them
there. Also conditionally compile lib/dump_stack.c since dump_stack code
does not make sense if printk is disabled.

Signed-off-by: Dave Young 
Suggested-by: Steven Rostedt 
Suggested-by: Sergey Senozhatsky 
---
Note: dump stack etc still share printk.h as before, I would keep it as is
since a lot of files need it.
 include/linux/printk.h |7 -
 kernel/printk/printk.c |   60 -
 lib/Makefile   |3 +-
 lib/dump_stack.c   |   60 +
 4 files changed, 67 insertions(+), 63 deletions(-)

--- linux-x86.orig/kernel/printk/printk.c
+++ linux-x86/kernel/printk/printk.c
@@ -42,13 +42,11 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
@@ -3257,62 +3255,4 @@ void kmsg_dump_rewind(struct kmsg_dumper
 }
 EXPORT_SYMBOL_GPL(kmsg_dump_rewind);
 
-static char dump_stack_arch_desc_str[128];
-
-/**
- * dump_stack_set_arch_desc - set arch-specific str to show with task dumps
- * @fmt: printf-style format string
- * @...: arguments for the format string
- *
- * The configured string will be printed right after utsname during task
- * dumps.  Usually used to add arch-specific system identifiers.  If an
- * arch wants to make use of such an ID string, it should initialize this
- * as soon as possible during boot.
- */
-void __init dump_stack_set_arch_desc(const char *fmt, ...)
-{
-   va_list args;
-
-   va_start(args, fmt);
-   vsnprintf(dump_stack_arch_desc_str, sizeof(dump_stack_arch_desc_str),
- fmt, args);
-   va_end(args);
-}
-
-/**
- * dump_stack_print_info - print generic debug info for dump_stack()
- * @log_lvl: log level
- *
- * Arch-specific dump_stack() implementations can use this function to
- * print out the same debug information as the generic dump_stack().
- */
-void dump_stack_print_info(const char *log_lvl)
-{
-   printk("%sCPU: %d PID: %d Comm: %.20s %s%s %s %.*s\n",
-  log_lvl, raw_smp_processor_id(), current->pid, current->comm,
-  kexec_crash_loaded() ? "Kdump: loaded " : "",
-  print_tainted(),
-  init_utsname()->release,
-  (int)strcspn(init_utsname()->version, " "),
-  init_utsname()->version);
-
-   if (dump_stack_arch_desc_str[0] != '\0')
-   printk("%sHardware name: %s\n",
-  log_lvl, dump_stack_arch_desc_str);
-
-   print_worker_info(log_lvl, current);
-}
-
-/**
- * show_regs_print_info - print generic debug info for show_regs()
- * @log_lvl: log level
- *
- * show_regs() implementations can use this function to print out generic
- * debug information.
- */
-void show_regs_print_info(const char *log_lvl)
-{
-   dump_stack_print_info(log_lvl);
-}
-
 #endif
--- linux-x86.orig/lib/dump_stack.c
+++ linux-x86/lib/dump_stack.c
@@ -10,6 +10,66 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+
+static char dump_stack_arch_desc_str[128];
+
+/**
+ * dump_stack_set_arch_desc - set arch-specific str to show with task dumps
+ * @fmt: printf-style format string
+ * @...: arguments for the format string
+ *
+ * The configured string will be printed right after utsname during task
+ * dumps.  Usually used to add arch-specific system identifiers.  If an
+ * arch wants to make use of such an ID string, it should initialize this
+ * as soon as possible during boot.
+ */
+void __init dump_stack_set_arch_desc(const char *fmt, ...)
+{
+   va_list args;
+
+   va_start(args, fmt);
+   vsnprintf(dump_stack_arch_desc_str, sizeof(dump_stack_arch_desc_str),
+ fmt, args);
+   va_end(args);
+}
+
+/**
+ * dump_stack_print_info - print generic debug info for dump_stack()
+ * @log_lvl: log level
+ *
+ * Arch-specific dump_stack() implementations can use this function to
+ * print out the same debug information as the generic dump_stack().
+ */
+void dump_stack_print_info(const char *log_lvl)
+{
+   printk("%sCPU: %d PID: %d Comm: %.20s %s%s %s %.*s\n",
+  log_lvl, raw_smp_processor_id(), current->pid, current->comm,
+  kexec_crash_loaded() ? "Kdump: loaded " : "",
+  print_tainted(),
+  init_utsname()->release,
+  (int)strcspn(init_utsname()->version, " "),
+  init_utsname()->version);
+
+   if (dump_stack_arch_desc_str[0] != '\0')
+   printk("%sHardware name: %s\n",
+  log_lvl, dump_stack_arch_desc_str);
+
+   print_worker_info(log_lvl, current);
+}
+
+/**
+ * show_regs_print_info - print generic debug info for show_regs()
+ * @log_lvl: log level
+ *
+ * show_regs() implementations can use this function to print out generic
+ * debug information.
+ */
+void show_regs_print_info(const char *log_lvl)
+{
+   dump_stack_print_info(log_lvl);
+}
 
 static