public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] UV: NMI: insert per_cpu accessor function on uv_hub_nmi.
@ 2015-04-30 13:29 George Beshers
  2015-04-30 13:29 ` [PATCH 2/2] UV: NMI: simple dump failover if kdump fails George Beshers
  2015-05-01  7:21 ` [PATCH 1/2] UV: NMI: insert per_cpu accessor function on uv_hub_nmi Ingo Molnar
  0 siblings, 2 replies; 8+ messages in thread
From: George Beshers @ 2015-04-30 13:29 UTC (permalink / raw)
  To: George Beshers, Mike Travis, linux-kernel, Alex Thorlton,
	Dimitri Sivanich, Ingo Molnar, Hedi Berriche, Russ Anderson
  Cc: George Beshers

UV: NMI: insert this_cpu_read accessor function on uv_hub_nmi.

UV NMI was accidently broken by this patch.

    commit e16321709c8270f9803bbfdb51e5e02235078c7f
    Author: Christoph Lameter <cl@linux.com>
    Date:   Sun Aug 17 12:30:41 2014 -0500

This patch insert this_cpu_read() on when accessing the
PER_CPU uv_cpu_nmi variable.

Signed-off-by: George Beshers <gbeshers@sgi.com>
Acked-by: Mike Travis <travis@sgi.com>
Cc: Russ Anderson <rja@sgi.com>
Cc: Dimitri Sivanich <sivanich@sgi.com>
Cc: Hedi Berriche <hedi@sgi.com>
Cc: Alex Thorlton <athorlton@sgi.com>
Cc: Christoph Lameter <cl@linux.com>

diff --git a/arch/x86/include/asm/uv/uv_hub.h b/arch/x86/include/asm/uv/uv_hub.h
index a00ad8f..ea707478 100644
--- a/arch/x86/include/asm/uv/uv_hub.h
+++ b/arch/x86/include/asm/uv/uv_hub.h
@@ -609,7 +609,7 @@ struct uv_cpu_nmi_s {
 
 DECLARE_PER_CPU(struct uv_cpu_nmi_s, uv_cpu_nmi);
 
-#define uv_hub_nmi			(uv_cpu_nmi.hub)
+#define uv_hub_nmi			this_cpu_read(uv_cpu_nmi.hub)
 #define uv_cpu_nmi_per(cpu)		(per_cpu(uv_cpu_nmi, cpu))
 #define uv_hub_nmi_per(cpu)		(uv_cpu_nmi_per(cpu).hub)
 

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] UV: NMI: simple dump failover if kdump fails
  2015-04-30 13:29 [PATCH 1/2] UV: NMI: insert per_cpu accessor function on uv_hub_nmi George Beshers
@ 2015-04-30 13:29 ` George Beshers
  2015-05-01  7:27   ` Ingo Molnar
  2015-05-01  7:21 ` [PATCH 1/2] UV: NMI: insert per_cpu accessor function on uv_hub_nmi Ingo Molnar
  1 sibling, 1 reply; 8+ messages in thread
From: George Beshers @ 2015-04-30 13:29 UTC (permalink / raw)
  To: George Beshers, Mike Travis, linux-kernel, Alex Thorlton,
	Dimitri Sivanich, Ingo Molnar, Hedi Berriche, Russ Anderson
  Cc: George Beshers

UV: NMI: simple dump failover if kdump fails
    
The ability to trigger a kdump using the system NMI command
was added by

    commit 12ba6c990fab50fe568f3ad8715e81e356552428
    Author: Mike Travis <travis@sgi.com>
    Date:   Mon Sep 23 16:25:03 2013 -0500

When kdump is works it is preferable to the set of backtraces
that dump provides; however a number of things can go wrong and
the backtraces are much more useful than nothing.

The two most common reason for kdump not to be available are
a problem during boot or the kdump daemon fails to start.
In either case the call to crash_kexec() returns unexpectedly;
when this happens uv_nmi_kdump() also returns with the
uv_nmi_kexec_failed flag set.  This condition now causes a
standard dump.

One other minor change is that dump now generates both the
show_regs() stack trace and the uv_nmi_dump_ip{,_hdr} information
that is generated by the "ips" action; the additional information
has proved to be useful.

Signed-off-by: George Beshers <gbeshers@sgi.com>
Acked-by: Mike Travis <travis@sgi.com>
Cc: Russ Anderson <rja@sgi.com>
Cc: Dimitri Sivanich <sivanich@sgi.com>
Cc: Hedi Berriche <hedi@sgi.com>
Cc: Alex Thorlton <athorlton@sgi.com>
Cc: Christoph Lameter <cl@linux.com>

diff --git a/arch/x86/platform/uv/uv_nmi.c b/arch/x86/platform/uv/uv_nmi.c
index 7488caf..89f37c7 100644
--- a/arch/x86/platform/uv/uv_nmi.c
+++ b/arch/x86/platform/uv/uv_nmi.c
@@ -391,23 +391,27 @@ static void uv_nmi_dump_cpu_ip(int cpu, struct pt_regs *regs)
 	printk_address(regs->ip);
 }
 
-/* Dump this cpu's state */
+/*
+ * Dump this cpu's state.  Note that "kdump" only happens
+ * when crash_kexec() has failed and we are providing the user
+ * a standard dump instead.
+ */
 static void uv_nmi_dump_state_cpu(int cpu, struct pt_regs *regs)
 {
 	const char *dots = " ................................. ";
 
-	if (uv_nmi_action_is("ips")) {
-		if (cpu == 0)
-			uv_nmi_dump_cpu_ip_hdr();
-
-		if (current->pid != 0)
-			uv_nmi_dump_cpu_ip(cpu, regs);
-
-	} else if (uv_nmi_action_is("dump")) {
+	if (uv_nmi_action_is("dump") || uv_nmi_action_is("kdump")) {
 		printk(KERN_DEFAULT
 			"UV:%sNMI process trace for CPU %d\n", dots, cpu);
 		show_regs(regs);
 	}
+
+	if (cpu == 0)
+		uv_nmi_dump_cpu_ip_hdr();
+
+	if (current->pid != 0)
+		uv_nmi_dump_cpu_ip(cpu, regs);
+
 	this_cpu_write(uv_cpu_nmi.state, UV_NMI_STATE_DUMP_DONE);
 }
 
@@ -492,8 +496,9 @@ static void uv_nmi_touch_watchdogs(void)
 	touch_nmi_watchdog();
 }
 
-#if defined(CONFIG_KEXEC)
 static atomic_t uv_nmi_kexec_failed;
+
+#if defined(CONFIG_KEXEC)
 static void uv_nmi_kdump(int cpu, int master, struct pt_regs *regs)
 {
 	/* Call crash to dump system state */
@@ -502,9 +507,9 @@ static void uv_nmi_kdump(int cpu, int master, struct pt_regs *regs)
 		crash_kexec(regs);
 
 		pr_emerg("UV: crash_kexec unexpectedly returned, ");
+		atomic_set(&uv_nmi_kexec_failed, 1);
 		if (!kexec_crash_image) {
 			pr_cont("crash kernel not loaded\n");
-			atomic_set(&uv_nmi_kexec_failed, 1);
 			uv_nmi_sync_exit(1);
 			return;
 		}
@@ -524,6 +529,7 @@ static inline void uv_nmi_kdump(int cpu, int master, struct pt_regs *regs)
 {
 	if (master)
 		pr_err("UV: NMI kdump: KEXEC not supported in this kernel\n");
+	atomic_set(&uv_nmi_kexec_failed, 1);
 }
 #endif /* !CONFIG_KEXEC */
 
@@ -620,7 +626,8 @@ int uv_handle_nmi(unsigned int reason, struct pt_regs *regs)
 	uv_nmi_wait(master);
 
 	/* Dump state of each cpu */
-	if (uv_nmi_action_is("ips") || uv_nmi_action_is("dump"))
+	if (uv_nmi_action_is("ips") || uv_nmi_action_is("dump") ||
+			atomic_read(&uv_nmi_kexec_failed) == 1)
 		uv_nmi_dump_state(cpu, regs, master);
 
 	/* Call KGDB/KDB if enabled */
@@ -640,6 +647,7 @@ int uv_handle_nmi(unsigned int reason, struct pt_regs *regs)
 		atomic_set(&uv_nmi_cpus_in_nmi, -1);
 		atomic_set(&uv_nmi_cpu, -1);
 		atomic_set(&uv_in_nmi, 0);
+		atomic_set(&uv_nmi_kexec_failed, 0);
 	}
 
 	uv_nmi_touch_watchdogs();

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] UV: NMI: insert per_cpu accessor function on uv_hub_nmi.
  2015-04-30 13:29 [PATCH 1/2] UV: NMI: insert per_cpu accessor function on uv_hub_nmi George Beshers
  2015-04-30 13:29 ` [PATCH 2/2] UV: NMI: simple dump failover if kdump fails George Beshers
@ 2015-05-01  7:21 ` Ingo Molnar
  1 sibling, 0 replies; 8+ messages in thread
From: Ingo Molnar @ 2015-05-01  7:21 UTC (permalink / raw)
  To: George Beshers
  Cc: Mike Travis, linux-kernel, Alex Thorlton, Dimitri Sivanich,
	Ingo Molnar, Hedi Berriche, Russ Anderson


* George Beshers <gbeshers@sgi.com> wrote:

> UV: NMI: insert this_cpu_read accessor function on uv_hub_nmi.
> 
> UV NMI was accidently broken by this patch.

Broken in what way?

>     commit e16321709c8270f9803bbfdb51e5e02235078c7f
>     Author: Christoph Lameter <cl@linux.com>
>     Date:   Sun Aug 17 12:30:41 2014 -0500

That's a rather old patch. Was no upstream kernel tested since ~August 
last year on UV hardware, or is the bug sporadic? The changelog does 
not tell us.

> This patch insert this_cpu_read() on when accessing the PER_CPU 
> uv_cpu_nmi variable.

Why? What problem does it solve?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] UV: NMI: simple dump failover if kdump fails
  2015-04-30 13:29 ` [PATCH 2/2] UV: NMI: simple dump failover if kdump fails George Beshers
@ 2015-05-01  7:27   ` Ingo Molnar
  2015-05-01 16:33     ` Mike Travis
  0 siblings, 1 reply; 8+ messages in thread
From: Ingo Molnar @ 2015-05-01  7:27 UTC (permalink / raw)
  To: George Beshers
  Cc: Mike Travis, linux-kernel, Alex Thorlton, Dimitri Sivanich,
	Ingo Molnar, Hedi Berriche, Russ Anderson


* George Beshers <gbeshers@sgi.com> wrote:

> UV: NMI: simple dump failover if kdump fails
>     
> The ability to trigger a kdump using the system NMI command
> was added by
> 
>     commit 12ba6c990fab50fe568f3ad8715e81e356552428
>     Author: Mike Travis <travis@sgi.com>
>     Date:   Mon Sep 23 16:25:03 2013 -0500
> 
> When kdump is works it is preferable to the set of backtraces

(spelling error)

> that dump provides; however a number of things can go wrong and
> the backtraces are much more useful than nothing.
> 
> The two most common reason for kdump not to be available are

(spelling error)

> a problem during boot or the kdump daemon fails to start.

(spelling error)

> In either case the call to crash_kexec() returns unexpectedly;
> when this happens uv_nmi_kdump() also returns with the
> uv_nmi_kexec_failed flag set.  This condition now causes a
> standard dump.

'standard dump' == printing an NMI backtrace on all CPUs?

> One other minor change is that dump now generates both the
> show_regs() stack trace and the uv_nmi_dump_ip{,_hdr} information
> that is generated by the "ips" action; the additional information
> has proved to be useful.

Looks like a useful change.

> -/* Dump this cpu's state */
> +/*
> + * Dump this cpu's state.  Note that "kdump" only happens

s/CPU's

> + * when crash_kexec() has failed and we are providing the user
> + * a standard dump instead.

So this sentence does not parse for me: kdump only happens if kdump 
fails??

> + */
>  static void uv_nmi_dump_state_cpu(int cpu, struct pt_regs *regs)
>  {
>  	const char *dots = " ................................. ";
>  
> -	if (uv_nmi_action_is("ips")) {
> -		if (cpu == 0)
> -			uv_nmi_dump_cpu_ip_hdr();
> -
> -		if (current->pid != 0)
> -			uv_nmi_dump_cpu_ip(cpu, regs);
> -
> -	} else if (uv_nmi_action_is("dump")) {
> +	if (uv_nmi_action_is("dump") || uv_nmi_action_is("kdump")) {
>  		printk(KERN_DEFAULT
>  			"UV:%sNMI process trace for CPU %d\n", dots, cpu);

pr_info().

>  		show_regs(regs);
>  	}
> +
> +	if (cpu == 0)
> +		uv_nmi_dump_cpu_ip_hdr();
> +
> +	if (current->pid != 0)
> +		uv_nmi_dump_cpu_ip(cpu, regs);

What is an 'ip header'? If it's not an Internet IP address then it's 
probably horribly named.

> +
> +#if defined(CONFIG_KEXEC)

#ifdef

> @@ -502,9 +507,9 @@ static void uv_nmi_kdump(int cpu, int master, struct pt_regs *regs)
>  		crash_kexec(regs);
>  
>  		pr_emerg("UV: crash_kexec unexpectedly returned, ");
> +		atomic_set(&uv_nmi_kexec_failed, 1);

Why is this flag an atomic variable?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] UV: NMI: simple dump failover if kdump fails
  2015-05-01  7:27   ` Ingo Molnar
@ 2015-05-01 16:33     ` Mike Travis
  2015-05-01 16:42       ` Ingo Molnar
  0 siblings, 1 reply; 8+ messages in thread
From: Mike Travis @ 2015-05-01 16:33 UTC (permalink / raw)
  To: Ingo Molnar, George Beshers
  Cc: linux-kernel, Alex Thorlton, Dimitri Sivanich, Ingo Molnar,
	Hedi Berriche, Russ Anderson



On 5/1/2015 12:27 AM, Ingo Molnar wrote:
> 
> * George Beshers <gbeshers@sgi.com> wrote:
> 
>> UV: NMI: simple dump failover if kdump fails
>>     
>> The ability to trigger a kdump using the system NMI command
>> was added by
>>
>>     commit 12ba6c990fab50fe568f3ad8715e81e356552428
>>     Author: Mike Travis <travis@sgi.com>
>>     Date:   Mon Sep 23 16:25:03 2013 -0500
>>
>> When kdump is works it is preferable to the set of backtraces
> 
> (spelling error)
> 
>> that dump provides; however a number of things can go wrong and
>> the backtraces are much more useful than nothing.
>>
>> The two most common reason for kdump not to be available are
> 
> (spelling error)
> 
>> a problem during boot or the kdump daemon fails to start.
> 
> (spelling error)
> 
>> In either case the call to crash_kexec() returns unexpectedly;
>> when this happens uv_nmi_kdump() also returns with the
>> uv_nmi_kexec_failed flag set.  This condition now causes a
>> standard dump.
> 
> 'standard dump' == printing an NMI backtrace on all CPUs?

Yes.
> 
>> One other minor change is that dump now generates both the
>> show_regs() stack trace and the uv_nmi_dump_ip{,_hdr} information
>> that is generated by the "ips" action; the additional information
>> has proved to be useful.
> 
> Looks like a useful change.
> 
>> -/* Dump this cpu's state */
>> +/*
>> + * Dump this cpu's state.  Note that "kdump" only happens
> 
> s/CPU's
> 
>> + * when crash_kexec() has failed and we are providing the user
>> + * a standard dump instead.
> 
> So this sentence does not parse for me: kdump only happens if kdump 
> fails??
> 
>> + */
>>  static void uv_nmi_dump_state_cpu(int cpu, struct pt_regs *regs)
>>  {
>>  	const char *dots = " ................................. ";
>>  
>> -	if (uv_nmi_action_is("ips")) {
>> -		if (cpu == 0)
>> -			uv_nmi_dump_cpu_ip_hdr();
>> -
>> -		if (current->pid != 0)
>> -			uv_nmi_dump_cpu_ip(cpu, regs);
>> -
>> -	} else if (uv_nmi_action_is("dump")) {
>> +	if (uv_nmi_action_is("dump") || uv_nmi_action_is("kdump")) {
>>  		printk(KERN_DEFAULT
>>  			"UV:%sNMI process trace for CPU %d\n", dots, cpu);
> 
> pr_info().
> 
>>  		show_regs(regs);
>>  	}
>> +
>> +	if (cpu == 0)
>> +		uv_nmi_dump_cpu_ip_hdr();
>> +
>> +	if (current->pid != 0)
>> +		uv_nmi_dump_cpu_ip(cpu, regs);
> 
> What is an 'ip header'? If it's not an Internet IP address then it's 
> probably horribly named.

The IP or Instruction Pointer register.  The "show ips" is sort of a
simplified ps showing the processes on non-idle CPUs.  We'd need to
blame Intel for that name... :)

Currently you can have either the IPs or the stack dump, but both
contain useful info.  So George's idea was if you asked for the dump
you'd get both, if you asked only for IPs, you'd just get them.

> 
>> +
>> +#if defined(CONFIG_KEXEC)
> 
> #ifdef
> 
>> @@ -502,9 +507,9 @@ static void uv_nmi_kdump(int cpu, int master, struct pt_regs *regs)
>>  		crash_kexec(regs);
>>  
>>  		pr_emerg("UV: crash_kexec unexpectedly returned, ");
>> +		atomic_set(&uv_nmi_kexec_failed, 1);
> 
> Why is this flag an atomic variable?
> 
> Thanks,
> 
> 	Ingo
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] UV: NMI: simple dump failover if kdump fails
  2015-05-01 16:33     ` Mike Travis
@ 2015-05-01 16:42       ` Ingo Molnar
  0 siblings, 0 replies; 8+ messages in thread
From: Ingo Molnar @ 2015-05-01 16:42 UTC (permalink / raw)
  To: Mike Travis
  Cc: George Beshers, linux-kernel, Alex Thorlton, Dimitri Sivanich,
	Ingo Molnar, Hedi Berriche, Russ Anderson


* Mike Travis <travis@sgi.com> wrote:

> 
> 
> On 5/1/2015 12:27 AM, Ingo Molnar wrote:
> > 
> > * George Beshers <gbeshers@sgi.com> wrote:
> > 
> >> UV: NMI: simple dump failover if kdump fails
> >>     
> >> The ability to trigger a kdump using the system NMI command
> >> was added by
> >>
> >>     commit 12ba6c990fab50fe568f3ad8715e81e356552428
> >>     Author: Mike Travis <travis@sgi.com>
> >>     Date:   Mon Sep 23 16:25:03 2013 -0500
> >>
> >> When kdump is works it is preferable to the set of backtraces
> > 
> > (spelling error)
> > 
> >> that dump provides; however a number of things can go wrong and
> >> the backtraces are much more useful than nothing.
> >>
> >> The two most common reason for kdump not to be available are
> > 
> > (spelling error)
> > 
> >> a problem during boot or the kdump daemon fails to start.
> > 
> > (spelling error)
> > 
> >> In either case the call to crash_kexec() returns unexpectedly;
> >> when this happens uv_nmi_kdump() also returns with the
> >> uv_nmi_kexec_failed flag set.  This condition now causes a
> >> standard dump.
> > 
> > 'standard dump' == printing an NMI backtrace on all CPUs?
> 
> Yes.
> > 
> >> One other minor change is that dump now generates both the
> >> show_regs() stack trace and the uv_nmi_dump_ip{,_hdr} information
> >> that is generated by the "ips" action; the additional information
> >> has proved to be useful.
> > 
> > Looks like a useful change.
> > 
> >> -/* Dump this cpu's state */
> >> +/*
> >> + * Dump this cpu's state.  Note that "kdump" only happens
> > 
> > s/CPU's
> > 
> >> + * when crash_kexec() has failed and we are providing the user
> >> + * a standard dump instead.
> > 
> > So this sentence does not parse for me: kdump only happens if kdump 
> > fails??
> > 
> >> + */
> >>  static void uv_nmi_dump_state_cpu(int cpu, struct pt_regs *regs)
> >>  {
> >>  	const char *dots = " ................................. ";
> >>  
> >> -	if (uv_nmi_action_is("ips")) {
> >> -		if (cpu == 0)
> >> -			uv_nmi_dump_cpu_ip_hdr();
> >> -
> >> -		if (current->pid != 0)
> >> -			uv_nmi_dump_cpu_ip(cpu, regs);
> >> -
> >> -	} else if (uv_nmi_action_is("dump")) {
> >> +	if (uv_nmi_action_is("dump") || uv_nmi_action_is("kdump")) {
> >>  		printk(KERN_DEFAULT
> >>  			"UV:%sNMI process trace for CPU %d\n", dots, cpu);
> > 
> > pr_info().
> > 
> >>  		show_regs(regs);
> >>  	}
> >> +
> >> +	if (cpu == 0)
> >> +		uv_nmi_dump_cpu_ip_hdr();
> >> +
> >> +	if (current->pid != 0)
> >> +		uv_nmi_dump_cpu_ip(cpu, regs);
> > 
> > What is an 'ip header'? If it's not an Internet IP address then it's 
> > probably horribly named.
> 
> The IP or Instruction Pointer register.  The "show ips" is sort of a
> simplified ps showing the processes on non-idle CPUs.  We'd need to
> blame Intel for that name... :)

Yes, but this is 64-bit code, why not call it RIP? :-)

that's kind of not unambiguous either, but at least in technical 
discussions it should be ;-)

So what I found confusing is the ip_hdr - that sounds very network-ish 
...

> Currently you can have either the IPs or the stack dump, but both 
> contain useful info.  So George's idea was if you asked for the dump 
> you'd get both, if you asked only for IPs, you'd just get them.

Yeah, I'm not against the idea at all. The patch needs a bit of a face 
lift and then it looks good to me.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/2] UV: NMI: simple dump failover if kdump fails
  2015-09-13  2:51 George Beshers
@ 2015-09-13  2:51 ` George Beshers
  2015-09-13  7:23   ` Ingo Molnar
  0 siblings, 1 reply; 8+ messages in thread
From: George Beshers @ 2015-09-13  2:51 UTC (permalink / raw)
  To: George Beshers, Mike Travis, linux-kernel, Alex Thorlton,
	Dimitri Sivanich, Ingo Molnar, Hedi Berriche, Russ Anderson
  Cc: George Beshers

Subject: [PATCH 2/2] UV: NMI: simple dump failover if kdump fails

The ability to trigger a kdump using the system NMI command
was added by

    commit 12ba6c990fab ("x86/UV: Add kdump to UV NMI handler")
    Author: Mike Travis <travis@sgi.com>
    Date:   Mon Sep 23 16:25:03 2013 -0500

This is useful because when kdump is working the information gathered
is more informative than the original per CPU stack traces or "dump"
option.  However a number of things can go wrong with kdump and then
the stack traces are more useful than nothing.

The two most common reasons for kdump to not be available are:
  1) if a problem occurs during boot before the kdump service is
     started, or
  2) the kdump daemon failed to start.
In either case the call to crash_kexec() returns unexpectedly.

When this happens uv_nmi_kdump() also sets the uv_nmi_kexec_failed
flag which causes the slave CPU's to also return to the NMI handler.
Upon this unexpected return to the NMI handler, the NMI handler
will revert to the "dump" action which uses show_regs() to obtain
a process trace dump for all the CPU's.

Other minor changes:
    The "dump" action now generates both the show_regs() stack trace
    and show instruction pointer information.  Whereas the "ips"
    action only shows instruction pointers for non-idle CPU's.  This
    is more like an abbreviated "ps" display.

    Change printk(KERN_DEFAULT...) --> pr_info()

Signed-off-by: George Beshers <gbeshers@sgi.com>
Signed-off-by: Mike Travis <travis@sgi.com>
Cc: Russ Anderson <rja@sgi.com>
Cc: Dimitri Sivanich <sivanich@sgi.com>
Cc: Hedi Berriche <hedi@sgi.com>
Cc: Alex Thorlton <athorlton@sgi.com>
Cc: Christoph Lameter <cl@linux.com>


diff --git a/arch/x86/platform/uv/uv_nmi.c b/arch/x86/platform/uv/uv_nmi.c
index 5c9f63f..327f21c 100644
--- a/arch/x86/platform/uv/uv_nmi.c
+++ b/arch/x86/platform/uv/uv_nmi.c
@@ -376,38 +376,42 @@ static void uv_nmi_wait(int master)
 		atomic_read(&uv_nmi_cpus_in_nmi), num_online_cpus());
 }
 
+/* Dump Instruction Pointer header */
 static void uv_nmi_dump_cpu_ip_hdr(void)
 {
-	printk(KERN_DEFAULT
-		"\nUV: %4s %6s %-32s %s   (Note: PID 0 not listed)\n",
+	pr_info("\nUV: %4s %6s %-32s %s   (Note: PID 0 not listed)\n",
 		"CPU", "PID", "COMMAND", "IP");
 }
 
+/* Dump Instruction Pointer info */
 static void uv_nmi_dump_cpu_ip(int cpu, struct pt_regs *regs)
 {
-	printk(KERN_DEFAULT "UV: %4d %6d %-32.32s ",
-		cpu, current->pid, current->comm);
-
+	pr_info("UV: %4d %6d %-32.32s ", cpu, current->pid, current->comm);
 	printk_address(regs->ip);
 }
 
-/* Dump this cpu's state */
+/*
+ * Dump this CPU's state.  If action was set to "kdump" and the crash_kexec
+ * failed, then we provide "dump" as an alternate action.  Action "dump" now
+ * also includes the show "ips" (instruction pointers) action whereas the
+ * action "ips" only displays instruction pointers for the non-idle CPU's.
+ * This is an abbreviated form of the "ps" command.
+ */
 static void uv_nmi_dump_state_cpu(int cpu, struct pt_regs *regs)
 {
 	const char *dots = " ................................. ";
 
-	if (uv_nmi_action_is("ips")) {
-		if (cpu == 0)
-			uv_nmi_dump_cpu_ip_hdr();
+	if (cpu == 0)
+		uv_nmi_dump_cpu_ip_hdr();
 
-		if (current->pid != 0)
-			uv_nmi_dump_cpu_ip(cpu, regs);
+	if (current->pid != 0 || !uv_nmi_action_is("ips"))
+		uv_nmi_dump_cpu_ip(cpu, regs);
 
-	} else if (uv_nmi_action_is("dump")) {
-		printk(KERN_DEFAULT
-			"UV:%sNMI process trace for CPU %d\n", dots, cpu);
+	if (uv_nmi_action_is("dump")) {
+		pr_info("UV:%sNMI process trace for CPU %d\n", dots, cpu);
 		show_regs(regs);
 	}
+
 	this_cpu_write(uv_cpu_nmi.state, UV_NMI_STATE_DUMP_DONE);
 }
 
@@ -469,8 +473,7 @@ static void uv_nmi_dump_state(int cpu, struct pt_regs *regs, int master)
 				uv_nmi_trigger_dump(tcpu);
 		}
 		if (ignored)
-			printk(KERN_DEFAULT "UV: %d CPUs ignored NMI\n",
-				ignored);
+			pr_alert("UV: %d CPUs ignored NMI\n", ignored);
 
 		console_loglevel = saved_console_loglevel;
 		pr_alert("UV: process trace complete\n");
@@ -492,8 +495,9 @@ static void uv_nmi_touch_watchdogs(void)
 	touch_nmi_watchdog();
 }
 
-#if defined(CONFIG_KEXEC_CORE)
 static atomic_t uv_nmi_kexec_failed;
+
+#if defined(CONFIG_KEXEC_CORE)
 static void uv_nmi_kdump(int cpu, int master, struct pt_regs *regs)
 {
 	/* Call crash to dump system state */
@@ -502,10 +506,9 @@ static void uv_nmi_kdump(int cpu, int master, struct pt_regs *regs)
 		crash_kexec(regs);
 
 		pr_emerg("UV: crash_kexec unexpectedly returned, ");
+		atomic_set(&uv_nmi_kexec_failed, 1);
 		if (!kexec_crash_image) {
 			pr_cont("crash kernel not loaded\n");
-			atomic_set(&uv_nmi_kexec_failed, 1);
-			uv_nmi_sync_exit(1);
 			return;
 		}
 		pr_cont("kexec busy, stalling cpus while waiting\n");
@@ -514,9 +517,6 @@ static void uv_nmi_kdump(int cpu, int master, struct pt_regs *regs)
 	/* If crash exec fails the slaves should return, otherwise stall */
 	while (atomic_read(&uv_nmi_kexec_failed) == 0)
 		mdelay(10);
-
-	/* Crash kernel most likely not loaded, return in an orderly fashion */
-	uv_nmi_sync_exit(0);
 }
 
 #else /* !CONFIG_KEXEC_CORE */
@@ -524,6 +524,7 @@ static inline void uv_nmi_kdump(int cpu, int master, struct pt_regs *regs)
 {
 	if (master)
 		pr_err("UV: NMI kdump: KEXEC not supported in this kernel\n");
+	atomic_set(&uv_nmi_kexec_failed, 1);
 }
 #endif /* !CONFIG_KEXEC_CORE */
 
@@ -613,9 +614,14 @@ int uv_handle_nmi(unsigned int reason, struct pt_regs *regs)
 	master = (atomic_read(&uv_nmi_cpu) == cpu);
 
 	/* If NMI action is "kdump", then attempt to do it */
-	if (uv_nmi_action_is("kdump"))
+	if (uv_nmi_action_is("kdump")) {
 		uv_nmi_kdump(cpu, master, regs);
 
+		/* Unexpected return, revert action to "dump" */
+		if (master)
+			strncpy(uv_nmi_action, "dump", strlen(uv_nmi_action));
+	}
+
 	/* Pause as all cpus enter the NMI handler */
 	uv_nmi_wait(master);
 
@@ -640,6 +646,7 @@ int uv_handle_nmi(unsigned int reason, struct pt_regs *regs)
 		atomic_set(&uv_nmi_cpus_in_nmi, -1);
 		atomic_set(&uv_nmi_cpu, -1);
 		atomic_set(&uv_in_nmi, 0);
+		atomic_set(&uv_nmi_kexec_failed, 0);
 	}
 
 	uv_nmi_touch_watchdogs();

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] UV: NMI: simple dump failover if kdump fails
  2015-09-13  2:51 ` [PATCH 2/2] UV: NMI: simple dump failover if kdump fails George Beshers
@ 2015-09-13  7:23   ` Ingo Molnar
  0 siblings, 0 replies; 8+ messages in thread
From: Ingo Molnar @ 2015-09-13  7:23 UTC (permalink / raw)
  To: George Beshers
  Cc: Mike Travis, linux-kernel, Alex Thorlton, Dimitri Sivanich,
	Ingo Molnar, Hedi Berriche, Russ Anderson


* George Beshers <gbeshers@sgi.com> wrote:

>     The "dump" action now generates both the show_regs() stack trace
>     and show instruction pointer information.  Whereas the "ips"
>     action only shows instruction pointers for non-idle CPU's.  This
>     is more like an abbreviated "ps" display.
> 
>     Change printk(KERN_DEFAULT...) --> pr_info()
> 
> Signed-off-by: George Beshers <gbeshers@sgi.com>
> Signed-off-by: Mike Travis <travis@sgi.com>

That's an invalid SOB chain. Did Mike author it? Then there should be a 'From:' 
field for him. If he acks it, it should be Acked-by or Reviewed-by.

I changed it to Acked-by for now.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-09-13  7:23 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-30 13:29 [PATCH 1/2] UV: NMI: insert per_cpu accessor function on uv_hub_nmi George Beshers
2015-04-30 13:29 ` [PATCH 2/2] UV: NMI: simple dump failover if kdump fails George Beshers
2015-05-01  7:27   ` Ingo Molnar
2015-05-01 16:33     ` Mike Travis
2015-05-01 16:42       ` Ingo Molnar
2015-05-01  7:21 ` [PATCH 1/2] UV: NMI: insert per_cpu accessor function on uv_hub_nmi Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2015-09-13  2:51 George Beshers
2015-09-13  2:51 ` [PATCH 2/2] UV: NMI: simple dump failover if kdump fails George Beshers
2015-09-13  7:23   ` Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox