Date: Thu, 22 Sep 2005 13:09:14 +0530
From: Vivek Goyal <vgoyal@in.ibm.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Morton Andrew Morton <akpm@osdl.org>,
       Anderson Dave Anderson <anderson@redhat.com>,
       Fastboot mailing list <fastboot@lists.osdl.org>,
       linux kernel mailing list <linux-kernel@vger.kernel.org>
Subject: Re: [Fastboot] [PATCH] Kdump(x86): add note type NT_KDUMPINFO to kernel core dumps
Message-ID: <20050922073914.GA3753@in.ibm.com>
Reply-To: vgoyal@in.ibm.com
References: <20050921065633.GC3780@in.ibm.com> <m1mzm6ebqn.fsf@ebiederm.dsl.xmission.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <m1mzm6ebqn.fsf@ebiederm.dsl.xmission.com>
User-Agent: Mutt/1.4.2.1i
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4430
Lines: 106

On Wed, Sep 21, 2005 at 08:28:32AM -0600, Eric W. Biederman wrote:
> 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.
> 

Yes it seems generic. And I think the reason being that I did not have a
significant number of things to report hence could not create logical groups
and giving very specific names to those groups. As of today, based on the
requirements, could think of only two items (page size, current). There
is still scope for few more things to appear. Hence gave a generic name and
thought more items can be grouped in this note itself until and unless group
becomes really big and need to break.

Do you have any suggestion to make this name better?


> - 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?
> 

Dave has already mentioned about the need of page size. I agree that page
size is a compile time information. Are you hinting at creating a separate
note for reporting page size in user space while loading the capture kernel 
and store it in reserved region alongside other elf headers. Not sure, if
it is good to create a separate note type just to report one configuration
variable. 

I also thought of enabling "Kernel .config support" (CONFIG_IKCONFIG=y) and
may be "crash" could write a script similar to scripts/extract-ikconfig to
retrieve any configuration variable required from vmlinux itself. But this
option is not enabled by default and will increase kernel image size and
seems too much to do for a single configuration variable. 

> - 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.  

It can be done but as Dave suggested but that requires significant amount 
of job to be done as one has to traverse through the active task stacks and
look for crash_kexec(). An easier/simpler way is that kernel itself can 
report it.  Netdump, diskdump already do it. I think for simplicity, it 
makes sense to export this information from kernel in the form of note.

Only issue I could think of is stack overflow and current might be 
corrupted after panic.


> If not would a little extra debug information
>   captured at compile time be better?
> 

Sorry, I did not get this. Can you elaborate a little more on this. What
kind of debug information can be helpful in determining the current.

> - 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).
> 

That's true. Probably a new note type shall be required to store
architectural control registers. May be something like NT_KDUMP_CRREG and
NT_KDUMPINFO can continue to capture miscellaneous info. 

Thanks
Vivek

> 
> > +/*
> > + * 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 */
> >  
-
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/