[Crash-utility] Re: [RFC] Crash patch for DWARF CFI based unwind support

[Rachita Kothiyal]

On Tue, Oct 17, 2006 at 03:37:16PM -0400, Dave Anderson wrote:
> 
> This netdump.c patch doesn't make sense to me -- we really don't
> want to be doing any ELFSTORE operations in the debug-only
> netdump_memory_dump() debug function:
> 
> @@ -771,8 +772,11 @@ netdump_memory_dump(FILE *fp)
>                         dump_Elf64_Phdr(nd->load64 + i, ELFREAD);
>                 offset64 = nd->notes64->p_offset;
>                 for (tot = 0; tot < nd->notes64->p_filesz; tot += len) {
> +                  if (has_unwind_info)
> +                       len = dump_Elf64_Nhdr(offset64, ELFSTORE);
> +                  else
>                         len = dump_Elf64_Nhdr(offset64, ELFREAD);
> -                       offset64 += len;
> +                   offset64 += len;
>                 }
>                 break;
>         }

Yes, you are right! I missed this one..
The store is already being done by the is_kdump().

> 
> This patch below looks to only be necessary in dumpfiles, but it seems
> like, given that the x86_64 user_regs_struct is unavailable in 2.6
> vmlinux files, that the initializations in get_netdump_regs_x86_64()
> would never get done -- because VALID_STRUCT(user_regs_struct) would
> fail, right?
> 
> @@ -1562,8 +1566,10 @@ get_netdump_regs_x86_64(struct bt_info *
>          if (is_task_active(bt->task))
>                  bt->flags |= BT_DUMPFILE_SEARCH;
> 
> -       if ((NETDUMP_DUMPFILE() || KDUMP_DUMPFILE()) &&
> -            VALID_STRUCT(user_regs_struct) && (bt->task == tt->panic_task)) {
> +       if (((NETDUMP_DUMPFILE() || KDUMP_DUMPFILE()) &&
> +             VALID_STRUCT(user_regs_struct) && (bt->task == tt->panic_task)) ||
> +             (KDUMP_DUMPFILE() && has_unwind_info && (bt->flags &
> +                       BT_DUMPFILE_SEARCH))) {

I add this last check to workaround the user_regs_struct not getting
initialized problem. Thats why I omit the VALID_STRUCT() check, and let it 
pass if we have the unwind_info.
This should be a temporary arrangement till we fix the user_regs_struct
problem. The idea here is to read the register states for all the active
tasks from the NT_PRSTATUS section of the dumpfile for kdump(in the case
where we have the has_unwind_info set). The other case for kdump where we 
do not have the unwind_info stays as it is.

>                 if (nd->num_prstatus_notes > 1)
>                         note = (Elf64_Nhdr *)
>                                 nd->nt_prstatus_percpu[bt->tc->processor];
> @@ -1574,9 +1580,21 @@ get_netdump_regs_x86_64(struct bt_info *
>                  len = roundup(len + note->n_namesz, 4);
>                  len = roundup(len + note->n_descsz, 4);
> 
> +               if KDUMP_DUMPFILE() {
> +                       ASSIGN_SIZE(user_regs_struct) = 27 * sizeof(unsigned long);
> +                       ASSIGN_OFFSET(user_regs_struct_rsp) = 19 * sizeof(unsigned long);
> +                       ASSIGN_OFFSET(user_regs_struct_rip) = 16 * sizeof(unsigned long);
> +               }
>                  user_regs = ((char *)note + len)
>                          - SIZE(user_regs_struct) - sizeof(long);
> 
> +               if KDUMP_DUMPFILE() {
> +                       *rspp = *(ulong *)(user_regs + OFFSET(user_regs_struct_rsp));
> +                       *ripp = *(ulong *)(user_regs + OFFSET(user_regs_struct_rip));
> +                       if (*ripp && *rspp)
> +                               return;
> +               }
> +
> 
> But then again, perhaps you never needed the user_regs_struct_rsp and
> user_regs_struct_rip offsets in your test scenario?

I needed them., thats why had to sort of hard code them here :)
But again, this is temporary till we fix the user_regs_struct issue :)

> 
> There does seem to be some unnecessary "kernel-port" left-overs that
> should be pruned.  Like the __get_user_nocheck(), __get_user_size()
> and __get_user_asm() definitions are superfluous, since they're only
> needed by __get_user(), which is not used.

Actually __get_user is a TBD. 
> 
  <...snipped the rest for another mail...>
> 
> Thanks,
>   Dave

Thanks for all the comments..

Thanks
Rachita