Re: [Fastboot] [PATCH] Kdump(x86): add note type NT_KDUMPINFO to kernel core dumps

[ebiederm@xmission.com (Eric W. Biederman)]

Vivek Goyal <vgoyal@in.ibm.com> writes:

> o This patch adds a new note type NT_KDUMPINFO. This note is added with 
>   kernel core dumps generated by kdump.
>
> o This note mainly communicates the information required by kernel dump
>   analysis tool "crash" to analyze the kernel core dump. As of now it contains 
>   the pointer to task struct of panicing task and page size. Page size is 
>   irrelevant for i386 but is required for architectures like ia64 and ppc64. 
>
> o gdb is not affected by this change as gdb need not to parse this note.

A couple of things.
- The name of your note is terribly generic, so it seems a poor choice.

- Why do we need to capture the page size at the time of the
  crash?  Isn't the page size a compile time option?  Won't
  sys_getpagesize() get you this information before the crash?  
  Why do we need the kernels page size at all?

- Why do you avoid storing the current task on the other cpus?

- Can't we derive the current task from the existing register information
  already captured.  If not would a little extra debug information
  captured at compile time be better?

- You don't address the issue of architectural control registers.
  So you are going to need another note at some point. (Not 
  necessarily a bad thing).


> +/*
> + * NT_KDUMPINFO can be used to communicate additional information required for
> + * kernel core dumps. Additional information includes kernel configuration
> + * variables like page size and any other relevant data required by
> + * debugger (crash in this case).
> +*/
> +struct elf_kdumpinfo
> +{
> +	char page_shift;		/* Page size */
> +	struct task_struct *panic_tsk;	/* Pointer to panic task_struct */
> +};
> +

>  #define NT_TASKSTRUCT	4
>  #define NT_AUXV		6
>  #define NT_PRXFPREG 0x46e62b7f /* copied from gdb5.1/include/elf/common.h */
> -
> +#define NT_KDUMPINFO	7		/* Used for kernel core dumps */
>