[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAErSpo7Hh7UNkeH=dKrNwPywQZFRfzaX5o7ReUsNvWjM4aD-eA@mail.gmail.com>
Date: Mon, 1 Apr 2013 12:11:17 -0600
From: Bjorn Helgaas <bhelgaas@...gle.com>
To: Tejun Heo <tj@...nel.org>
Cc: linux-arch@...r.kernel.org,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Ingo Molnar <mingo@...hat.com>,
"x86@...nel.org" <x86@...nel.org>,
Richard Henderson <rth@...ddle.net>,
Russell King <linux@....linux.org.uk>, msalter@...hat.com,
starvik@...s.com, David Howells <dhowells@...hat.com>,
Tony Luck <tony.luck@...el.com>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
takata@...ux-m32r.org, Geert Uytterhoeven <geert@...ux-m68k.org>,
james.hogan@...tec.com, Michal Simek <monstr@...str.eu>,
Ralf Baechle <ralf@...ux-mips.org>, jonas@...thpole.se,
rkuo@...eaurora.org, Martin Schwidefsky <schwidefsky@...ibm.com>,
liqin.chen@...plusct.com, David Miller <davem@...emloft.net>,
Paul Mundt <lethal@...ux-sh.org>, vgupta@...opsys.com,
Chris Zankel <chris@...kel.net>,
Chris Metcalf <cmetcalf@...era.com>,
ysato@...rs.sourceforge.jp, Guan Xuetao <gxt@...c.pku.edu.cn>,
jdike@...toit.com
Subject: Re: [PATCH 4/5] dump_stack: implement arch-specific hardware
description in task dumps
On Fri, Mar 29, 2013 at 8:27 PM, Tejun Heo <tj@...nel.org> wrote:
> x86 and ia64 can acquire extra hardware identification information
> from DMI and print it along with task dumps; however, the usage isn't
> consistent.
>
> * x86 show_regs() collects vendor, product and board strings and print
> them out with PID, comm and utsname. Some of the information is
> printed again later in the same dump.
>
> * warn_slowpath_common() explicitly accesses the DMI board and prints
> it out with "Hardware name:" label. This applies to both x86 and
> ia64 but is irrelevant on all other archs.
>
> * ia64 doesn't show DMI information on other non-WARN dumps.
>
> This patch introduces arch-specific hardware description used by
> dump_stack(). It can be set by calling dump_stack_set_arch_desc()
> during boot and printed out along with utsname.
>
> dmi_set_dump_stack_arch_desc() is added which sets arch-specific
> description from DMI data. It uses the same information and
> formatting as x86 show_regs() is using. The function is called from
> x86 and ia64 boot code right after dmi_scan_machine().
>
> This makes the explicit DMI handling in warn_slowpath_common()
> unnecessary. Removed.
>
> show_regs() isn't yet converted to use generic debug information
> printing and this patch doesn't remove the duplicate DMI handling in
> x86 show_regs(). The next patch will unify show_regs() handling and
> remove the duplication.
>
> An example WARN dump follows.
>
> WARNING: at /work/os/work/kernel/workqueue.c:4840 init_workqueues+0x35/0x505()
> Modules linked in:
> Pid: 1, comm: swapper/0 Not tainted 3.9.0-rc1-work+ #18 empty empty/S3992
> 0000000000000009 ffff88007c861e08 ffffffff81c61525 ffff88007c861e48
> ffffffff8108f500 ffffffff82228240 0000000000000040 ffffffff8234a041
> 0000000000000000 0000000000000000 0000000000000000 ffff88007c861e58
> Call Trace:
> [<ffffffff81c61525>] dump_stack+0x19/0x1b
> [<ffffffff8108f500>] warn_slowpath_common+0x70/0xa0
> [<ffffffff8108f54a>] warn_slowpath_null+0x1a/0x20
> [<ffffffff8234a076>] init_workqueues+0x35/0x505
> ...
>
> Signed-off-by: Tejun Heo <tj@...nel.org>
> ---
> arch/ia64/kernel/setup.c | 1 +
> arch/x86/kernel/setup.c | 1 +
> drivers/firmware/dmi_scan.c | 26 ++++++++++++++++++++++++++
> include/linux/dmi.h | 2 ++
> include/linux/printk.h | 1 +
> kernel/panic.c | 6 ------
> lib/dump_stack.c | 26 ++++++++++++++++++++++++--
> 7 files changed, 55 insertions(+), 8 deletions(-)
>
> diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
> index 2029cc0..13bfdd2 100644
> --- a/arch/ia64/kernel/setup.c
> +++ b/arch/ia64/kernel/setup.c
> @@ -1063,6 +1063,7 @@ check_bugs (void)
> static int __init run_dmi_scan(void)
> {
> dmi_scan_machine();
> + dmi_set_dump_stack_arch_desc();
> return 0;
> }
> core_initcall(run_dmi_scan);
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 90d8cc9..91b9e7c 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -970,6 +970,7 @@ void __init setup_arch(char **cmdline_p)
> efi_init();
>
> dmi_scan_machine();
> + dmi_set_dump_stack_arch_desc();
>
> /*
> * VMware detection requires dmi to be available, so this
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 4cd392d..9e7766b 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -521,6 +521,32 @@ void __init dmi_scan_machine(void)
> }
>
> /**
> + * dmi_set_dump_stack_arch_desc - set arch description for dump_stack()
> + *
> + * Invoke dump_stack_set_arch_desc() with DMI system information so that
> + * DMI identifiers are printed out on task dumps. Arch boot code should
> + * call this function after dmi_scan_machine() if it wants to print out DMI
> + * identifiers on task dumps.
> + */
> +void __init dmi_set_dump_stack_arch_desc(void)
> +{
> + const char *vendor, *product, *board;
> +
> + vendor = dmi_get_system_info(DMI_SYS_VENDOR);
> + if (!vendor)
> + vendor = "";
> + product = dmi_get_system_info(DMI_PRODUCT_NAME);
> + if (!product)
> + product = "";
> +
> + /* Board Name is optional */
> + board = dmi_get_system_info(DMI_BOARD_NAME);
> +
> + dump_stack_set_arch_desc("%s %s%s%s", vendor, product,
> + board ? "/" : "", board ? board : "");
> +}
Should this be combined or made consistent with dmi_dump_ids()? They
look almost the same but not quite (no BIOS version here).
> +
> +/**
> * dmi_matches - check if dmi_system_id structure matches system DMI data
> * @dmi: pointer to the dmi_system_id structure to check
> */
> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
> index f156cca..b6eb7a0 100644
> --- a/include/linux/dmi.h
> +++ b/include/linux/dmi.h
> @@ -99,6 +99,7 @@ extern const char * dmi_get_system_info(int field);
> extern const struct dmi_device * dmi_find_device(int type, const char *name,
> const struct dmi_device *from);
> extern void dmi_scan_machine(void);
> +extern void dmi_set_dump_stack_arch_desc(void);
> extern bool dmi_get_date(int field, int *yearp, int *monthp, int *dayp);
> extern int dmi_name_in_vendors(const char *str);
> extern int dmi_name_in_serial(const char *str);
> @@ -114,6 +115,7 @@ static inline const char * dmi_get_system_info(int field) { return NULL; }
> static inline const struct dmi_device * dmi_find_device(int type, const char *name,
> const struct dmi_device *from) { return NULL; }
> static inline void dmi_scan_machine(void) { return; }
> +static inline void dmi_set_dump_stack_arch_desc(void) { }
> static inline bool dmi_get_date(int field, int *yearp, int *monthp, int *dayp)
> {
> if (yearp)
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index 952c1b2..73788ff 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -177,6 +177,7 @@ static inline void setup_log_buf(int early)
> }
> #endif
>
> +extern void dump_stack_set_arch_desc(const char *fmt, ...);
> extern void dump_stack_print_info(const char *log_lvl);
> extern void dump_stack(void) __cold;
>
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 7c57cc9..167ec09 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -22,7 +22,6 @@
> #include <linux/sysrq.h>
> #include <linux/init.h>
> #include <linux/nmi.h>
> -#include <linux/dmi.h>
>
> #define PANIC_TIMER_STEP 100
> #define PANIC_BLINK_SPD 18
> @@ -400,13 +399,8 @@ struct slowpath_args {
> static void warn_slowpath_common(const char *file, int line, void *caller,
> unsigned taint, struct slowpath_args *args)
> {
> - const char *board;
> -
> printk(KERN_WARNING "------------[ cut here ]------------\n");
> printk(KERN_WARNING "WARNING: at %s:%d %pS()\n", file, line, caller);
> - board = dmi_get_system_info(DMI_PRODUCT_NAME);
> - if (board)
> - printk(KERN_WARNING "Hardware name: %s\n", board);
>
> if (args)
> vprintk(args->fmt, args->args);
> diff --git a/lib/dump_stack.c b/lib/dump_stack.c
> index 5a67cfc..93573ec 100644
> --- a/lib/dump_stack.c
> +++ b/lib/dump_stack.c
> @@ -8,6 +8,28 @@
> #include <linux/utsname.h>
> #include <linux/export.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
> @@ -17,11 +39,11 @@
> */
> void dump_stack_print_info(const char *log_lvl)
> {
> - printk("%sPid: %d, comm: %.20s %s %s %.*s\n",
> + printk("%sPid: %d, comm: %.20s %s %s %.*s %s\n",
> log_lvl, current->pid, current->comm, print_tainted(),
> init_utsname()->release,
> (int)strcspn(init_utsname()->version, " "),
> - init_utsname()->version);
> + init_utsname()->version, dump_stack_arch_desc_str);
> }
>
> /**
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists