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
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
