Date: Fri, 1 May 2015 09:27:33 +0200
From: Ingo Molnar <mingo@kernel.org>
To: George Beshers <gbeshers@sgi.com>
Cc: Mike Travis <travis@sgi.com>, linux-kernel@vger.kernel.org,
        Alex Thorlton <athorlton@sgi.com>, Dimitri Sivanich <sivanich@sgi.com>,
        Ingo Molnar <mingo@redhat.com>, Hedi Berriche <hedi@sgi.com>,
        Russ Anderson <rja@sgi.com>
Subject: Re: [PATCH 2/2] UV: NMI: simple dump failover if kdump fails
Message-ID: <20150501072733.GD19852@gmail.com>
References: <20150430132914.28122.11850.sendpatchset@gulag2.americas.sgi.com>
 <20150430132919.28122.53176.sendpatchset@gulag2.americas.sgi.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150430132919.28122.53176.sendpatchset@gulag2.americas.sgi.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2799
Lines: 105


* 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/