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

[Mike Travis <travis@sgi.com>]

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
>