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

[Dave Anderson]

----- Original Message -----
> Hi Dave and all,
> 
> Sent on 2011-5-7 0:11, Dave Anderson wrote:
> 
> >> 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
> >
> 
> Oh, sorry for my misunderstood.
> 
> Since we will later refer to ip/sp value in the notes_buf, we should keep the
> notes_buf permanently and do not free them at the end of read_diskdump_header.
> s390x don't have such problem because the notes are copied to the s390x_cpu_vec
> when processing notes_buf.

The dd->notes_buf only needs to be kept permanently if the read_dump_header()
function succeeds (returns TRUE).  If we do a "goto err" in read_dump_header(),
that means that dumpfile is not being recognized as a diskdump or compressed
kdump, and it returns FALSE.  When that is the case, there is no possible way 
that we could ever refer to the ip/sp values at a later point in time.  

Accordingly, I have added this to your patch:

 ...
        return TRUE;

 err:
         free(header);
         if (sub_header)
                 free(sub_header);
         if (sub_header_kdump)
                 free(sub_header_kdump);
         if (dd->bitmap)
                 free(dd->bitmap);
         if (dd->dumpable_bitmap)
                 free(dd->dumpable_bitmap);
+        if (dd->notes_buf)
+                free(dd->notes_buf);
+        if (dd->nt_prstatus_percpu)
+                free(dd->nt_prstatus_percpu);

         dd->flags &= ~(DISKDUMP_LOCAL|KDUMP_CMPRS_LOCAL);
         return FALSE;
 }


> So just as what you suggested, notes_buf was moved to diskdump header in the
> attached v4 patch and nt_prstatus_percpu will store the address in notes_buf then.

I also slightly modified the patches to get_netdump_regs_x86() and
get_netdump_regs_x86_64() to avoid the goto's, but with no functional
changes.

Queued for crash version 5.1.5.

Thanks,
  Dave