[Crash-utility] [RFC][PATCH v3] Use register value in elf note NT_PRSTATUS to do backtrace

[Dave Anderson]

Hello Wang,

I've re-studied this section, and I still have a few more issues 
to resolve with this patch.  But this should be the final review,
and I will wait for your v4 update before releasing 5.1.5.

> 1) Add a new option --no_elf_notes. If specified, it'll skip processing
>    elf notes in kdump-compressed dump files.

OK good -- but please move the (dd->flags & NO_ELF_NOTES) check from 
x86_process_elf_notes() into read_dump_header() so as to prevent the 
allocation of any buffers if they're never going to be used, i.e.:

        /* process elf notes data */
        if (KDUMP_CMPRS_VALID() && !(dd->flags & NO_ELF_NOTES) &&
                (dd->header->header_version >= 4) &&
  
And please make the --no_elf_notes option only applicable to x86 and x86_64
in order to prevent it from being applied to the s390x arch:

      else if (STREQ(long_options[option_index].name, "no_elf_notes")) {
              if (machine_type("X86") || machine_type("X86_64"))
                      *diskdump_flags |= NO_ELF_NOTES;
              else
                      error(INFO,
                              "--no_elf_notes is only applicable to ",
                              "the X86 and X86_64 architectures.\n");
      }

> 2) Dump notes related information including total number of notes, their
>    buffers' pointer value and finally offset in the dump file.
> 3) Move nt_prstatus_percpu and nt_prstatus_percpu to diskdump_data struct.
>    However, bt command may need sp/ip from the notes later, so I guess
>    the buffer should not be freed after diskdump_read_header().

I wish I had understood the s390x implementation a bit better when I
reviewed your v2 patch.  Currently read_dump_header() malloc's a 
single "notes_buf" buffer and copies the complete ELF notes section 
from the dumpfile into it.  I believe (but am not completely sure) that
the s390x code references pieces of that buffer, and that's why they
originally kept the "notes_buf" buffer permanently allocated.  That
being the case, the "notes_buf" pointer should be moved to the
diskdump header.  

Anyway, in the case of x86/x86_64, your patch:

(1) malloc's dd->nt_prstatus_percpu as an array of pointers, 
(2) malloc's a bunch of individual per-cpu ELF prstatus buffers, and
    then copies the ELF data from the "notes_buf" buffer into
    each per-cpu buffer.
(4) puts a pointer to each per-cpu buffer into each dd->nt_prstatus_percpu
    pointer.  

But given that the original "notes_buf" buffer is permanent, your patch
should only have to store per-cpu pointers into the original note_buf
buffer -- rather than redundantly malloc'ing a bunch of new buffers that
contain copies of what's already permanently available.  So I would suggest
setting it up like this:

  dd->nt_prstatus_percpu[cpu] => points into relevant "notes_buf" location

Also, even though the s390x will not utilize the dd->nt_prstatus_percpu[]
array, your dump_nt_prstatus_offset() function should be usable by
the s390x since it uses the same Elf64_Nhdr as x86_64.

And lastly, two other minor things:

Move the user_regs_struct_eip to the bottom of the offset_table.
The offset_table is exported to extension modules, so changing 
members in the middle of the structure could easily break a module
that was compiled against an earlier version.  Note that it has 
this at the top:

  struct offset_table {                    /* stash of commonly-used offsets */
          long list_head_next;             /* add new entries to end of table */
          long list_head_prev;
          long task_struct_pid;
          long task_struct_state;
          ...

Finally, add a display of the user_regs_struct_eip offset_table entry value 
into dump_offset_table(), which is used by "help -o".  In that function you
can put the user_regs_struct_eip display next to its other user_regs_struct
partners.

Thanks,
  Dave