[Crash-utility] [PATCH] netdump: Add a helper function to check if registers is available for a given active task in ELF notes

[HATAYAMA Daisuke]

Thanks for the comments, Dave.

From: Dave Anderson <anderson at redhat.com>
Subject: Re: [Crash-utility] [PATCH] netdump: Add a helper function to check if registers is available for a given active task in ELF notes
Date: Fri, 1 Apr 2011 14:22:38 -0400 (EDT)

> 
> 
> ----- Original Message -----
>> Add a helper function, exist_regs_in_elf_notes(tc), which checks
>> whether or not register values for a given active task is available in
>> ELF notes.
>> 
>> I intend to use the helper function in gcore extension module. vmcore
>> generated by diskdump has NT_PRSTATUS for a panic task only, and so
>> specifying get_regs_from_elf_notes() directly to non-panic active
>> tasks leads to a fatal action. So, it's necessary to check, in
>> advance, that an active task can get registers from ELF notes, but the
>> variable holding vmcore's data including ELF notes', nd, is defined as
>> a static global variable in netdump.c and thus the new helper function
>> needs to be introduced.
> 
> Why not make the "nd" pointer available to extension modules?  
> 

I agree. It's a reasonable solution.

> A get_kdump_vmcore_data() function already exists.  If you 
> create a new get_netdump_vmcore_data() function, it seems that
> most (if not all) of this netdump.c code that is *only* used by
> your extension module could be moved into your extension module
> source code, where it really belongs.  The vmcore_data structure
> declaration could be moved into defs.h so that you would not have
> to #include netdump.h.

It's no problem for gcore extension module. But the comment below,
located just above the relevant functions, appears to indicate there's
a potencial user who develops some extension modules but doesn't
distribute them at the website. I guess it will affect at least him,
and other similar users.

/*
 *  The following set of functions are not used by the crash
 *  source code, but are available to extension modules for
 *  gathering register sets from ELF NT_PRSTATUS note sections.
 *
 *  Contributed by: Sharyathi Nagesh (sharyath at in.ibm.com)
 */

> 
> BTW, you seemed to have sent a duplicate patch-set, i.e.,
> 0001-Check-non-support-machine-check-first-01.patch and
> 0001-Check-non-support-machine-check-first.patch, etc.

?? Sorry, I don't know the first patch. Four patchs are attached to
the mail I sent and appear as I intended.

From: Dave Anderson <anderson at redhat.com>
Subject: Re: [Crash-utility] [PATCH] netdump: Add a helper function to check if registers is available for a given active task in ELF notes
Date: Fri, 1 Apr 2011 16:08:42 -0400 (EDT)

> 
> A couple other questions...
> 
> As I understand it, if you attempt to do a gcore on an active
> task from a netdump vmcore, but it is not the panic task, the
> registers are not saved, and the command will abort due to the
> fatal error here in get_regs_from_elf_notes():
> 
>        if (!exist_regs_in_elf_notes(tc))
>                 error(FATAL, "cannot determine register set "
>                       "for %s task: %lx comm: \"%s\"\n",
>                       (tc->task == tt->panic_task) ? "panic" : "active",
>                       tc->task, tc->comm);
> 
> In that case, why not fall through and get the registers from 
> the kernel-entry exception frame in gcore_get_regs_from_eframe()?

Sorry for confusing you. I should have attached a patch for gcore
extension module together, like below. So, I've considered originally
as you've suggested.

diff --git a/src/libgcore/gcore_x86.c b/src/libgcore/gcore_x86.c
index 340531c..e650053 100644
--- a/src/libgcore/gcore_x86.c
+++ b/src/libgcore/gcore_x86.c
@@ -1471,8 +1471,9 @@ static int get_active_regs(struct task_context *target,
                return TRUE;
        }

-       if (get_netdump_arch() != EM_NONE) {
-               struct user_regs_struct *note = get_regs_from_elf_notes(target);
+       if (exist_regs_in_elf_notes(target)) {
+               struct user_regs_struct *note =
+                       get_x86_64_regs_from_elf_notes(target);
                memcpy(regs, note, sizeof(struct user_regs_struct));
                return TRUE;
        }

> 
> But I admit I still don't understand why you don't get
> the registers from the kernel-entry exception frame
> for *all* tasks -- even if the task was the panic task
> or one of the active tasks?
> 

Saved at the kenrel-entry exception frame are not all the registers:
note information contains but exception frame doesn't contain some
segment registers; My understanding is: strictly, they could be
changed at runtime and so impossible to resotre from kernel data
only. If note information is available, the problem is easy; we get
all the registers with no difficulty.

The other reason is for dwarf unwinder receiving RSP and RIP as a
starting point of back trace operation. Restoring callee-saved
registers fully relies on dwarf unwinder now. So, it's neccesary to
pass active RSP and RIP to active tasks.

> And related to the above, the fatal error message above
> prints either "panic" or "active".  How would it be possible
> for the "panic" task to fail?  The exist_regs_in_elf_notes()
> always returns TRUE for the panic task.

I think you are correct.

Thanks.
HATAYAMA Daisuke