2018-02-09 08:07:36

by Dave Young

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

dump_stack related stuff should belong to lib/dump_stack.c thus move them
there.

Signed-off-by: Dave Young <[email protected]>
Suggested-by: Steven Rostedt <[email protected]>
Suggested-by: Sergey Senozhatsky <[email protected]>
---
Note: dump stack etc still share printk.h as before, I would keep it as is
since a lot of files need it.
This patch is based on printk for-4.17 branch
kernel/printk/printk.c | 60 -------------------------------------------------
lib/dump_stack.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+), 60 deletions(-)

--- linux-x86.orig/kernel/printk/printk.c
+++ linux-x86/kernel/printk/printk.c
@@ -42,13 +42,11 @@
#include <linux/rculist.h>
#include <linux/poll.h>
#include <linux/irq_work.h>
-#include <linux/utsname.h>
#include <linux/ctype.h>
#include <linux/uio.h>
#include <linux/sched/clock.h>
#include <linux/sched/debug.h>
#include <linux/sched/task_stack.h>
-#include <linux/kexec.h>

#include <linux/uaccess.h>
#include <asm/sections.h>
@@ -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 <linux/sched/debug.h>
#include <linux/smp.h>
#include <linux/atomic.h>
+#include <linux/kexec.h>
+#include <linux/utsname.h>
+
+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 void __dump_stack(void)
{


2018-02-09 08:17:08

by Sergey Senozhatsky

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

Hi,

On (02/09/18 16:06), Dave Young wrote:
[..]
> +void __init dump_stack_set_arch_desc(const char *fmt, ...)
..
> +void dump_stack_print_info(const char *log_lvl)
..
> +void show_regs_print_info(const char *log_lvl)
..

Seems that those functions are still defined in printk header.
Did you test !CONFIG_PRINTK build?

-ss

2018-02-09 08:30:21

by Dave Young

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

On 02/09/18 at 05:16pm, Sergey Senozhatsky wrote:
> Hi,
>
> On (02/09/18 16:06), Dave Young wrote:
> [..]
> > +void __init dump_stack_set_arch_desc(const char *fmt, ...)
> ..
> > +void dump_stack_print_info(const char *log_lvl)
> ..
> > +void show_regs_print_info(const char *log_lvl)
> ..
>
> Seems that those functions are still defined in printk header.
> Did you test !CONFIG_PRINTK build?

!CONFIG_PRINTK will use the dummy functions in printk.h, I did not test
the build, doing it now to double confirm..

Thanks
Dave

>
> -ss

2018-02-09 08:36:31

by Dave Young

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

On 02/09/18 at 05:16pm, Sergey Senozhatsky wrote:
> Hi,
>
> On (02/09/18 16:06), Dave Young wrote:
> [..]
> > +void __init dump_stack_set_arch_desc(const char *fmt, ...)
> ..
> > +void dump_stack_print_info(const char *log_lvl)
> ..
> > +void show_regs_print_info(const char *log_lvl)
> ..
>
> Seems that those functions are still defined in printk header.
> Did you test !CONFIG_PRINTK build?

Hmm, indeed need update printk.h for !PRINTK, thanks for catching it.
Will refresh the patch soon..

Thanks!

>
> -ss

2018-02-09 08:45:55

by Sergey Senozhatsky

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

On (02/09/18 16:27), Dave Young wrote:
> > Seems that those functions are still defined in printk header.
> > Did you test !CONFIG_PRINTK build?

Apparently dump_stack(void) is also in printk.h

extern asmlinkage void dump_stack(void) __cold;

so it's "OK" to keep those functions in printk.h, I guess. I thought
that dump_stack() had its own header file...

> !CONFIG_PRINTK will use the dummy functions in printk.h, I did not test
> the build, doing it now to double confirm..

Not sure. Please test.

-ss

2018-02-09 08:52:37

by Dave Young

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

On 02/09/18 at 05:42pm, Sergey Senozhatsky wrote:
> On (02/09/18 16:27), Dave Young wrote:
> > > Seems that those functions are still defined in printk header.
> > > Did you test !CONFIG_PRINTK build?
>
> Apparently dump_stack(void) is also in printk.h
>
> extern asmlinkage void dump_stack(void) __cold;
>
> so it's "OK" to keep those functions in printk.h, I guess. I thought
> that dump_stack() had its own header file...

It has not unfortunately.. The build failed because we have dummy
functions in printk.h and redefined in lib/dump_stack.c. I'm hesitating
to add #ifdef CONFIG_PRINTK in lib/dump_stack.c.

>
> > !CONFIG_PRINTK will use the dummy functions in printk.h, I did not test
> > the build, doing it now to double confirm..
>
> Not sure. Please test.
>
> -ss

Thanks
Dave

2018-02-09 09:02:00

by Dave Young

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

On 02/09/18 at 04:51pm, Dave Young wrote:
> On 02/09/18 at 05:42pm, Sergey Senozhatsky wrote:
> > On (02/09/18 16:27), Dave Young wrote:
> > > > Seems that those functions are still defined in printk header.
> > > > Did you test !CONFIG_PRINTK build?
> >
> > Apparently dump_stack(void) is also in printk.h
> >
> > extern asmlinkage void dump_stack(void) __cold;
> >
> > so it's "OK" to keep those functions in printk.h, I guess. I thought
> > that dump_stack() had its own header file...
>
> It has not unfortunately.. The build failed because we have dummy
> functions in printk.h and redefined in lib/dump_stack.c. I'm hesitating
> to add #ifdef CONFIG_PRINTK in lib/dump_stack.c.

Maybe conditionally build dump_stack.o only when CONFIG_PRINTK is true,
but not sure if there are some historic reason this is not done before,
will do some testing see if it works.

>
> >
> > > !CONFIG_PRINTK will use the dummy functions in printk.h, I did not test
> > > the build, doing it now to double confirm..
> >
> > Not sure. Please test.
> >
> > -ss
>
> Thanks
> Dave

2018-02-09 09:16:42

by Sergey Senozhatsky

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

On (02/09/18 17:00), Dave Young wrote:
[..]
> >
> > I'm hesitating to add #ifdef CONFIG_PRINTK in lib/dump_stack.c.

Agreed.

> Maybe conditionally build dump_stack.o only when CONFIG_PRINTK is true,
> but not sure if there are some historic reason this is not done before,
> will do some testing see if it works.
>

Thanks.

Was thinking about the same thing - dump_stack() without CONFIG_PRINTK
doesn't make that much sense anyway. Well, maybe it does in some weird
case... Need to check. But it seems that we probably can just make the
dependency, which already exists, explicit.

-ss

2018-02-13 07:31:28

by Dave Young

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

On 02/09/18 at 06:15pm, Sergey Senozhatsky wrote:
> On (02/09/18 17:00), Dave Young wrote:
> [..]
> > >
> > > I'm hesitating to add #ifdef CONFIG_PRINTK in lib/dump_stack.c.
>
> Agreed.
>
> > Maybe conditionally build dump_stack.o only when CONFIG_PRINTK is true,
> > but not sure if there are some historic reason this is not done before,
> > will do some testing see if it works.
> >
>
> Thanks.
>
> Was thinking about the same thing - dump_stack() without CONFIG_PRINTK
> doesn't make that much sense anyway. Well, maybe it does in some weird
> case... Need to check. But it seems that we probably can just make the
> dependency, which already exists, explicit.

Did some testing on x86_64 and s390, I did not find any breakage, sent
the V2 just now.

>
> -ss

Thanks
Dave