[Crash-utility] Re: Fix for source line numbers for x86_64 modules

[John Wright (ALPS, Fort Collins)]

Hi Dave,

On Thu, Sep 24, 2009 at 03:00:49PM -0400, Dave Anderson wrote:
> 
> ----- "John Wright (ALPS, Fort Collins)" <john.wright at hp.com> wrote:
> 
> > On Wed, Sep 16, 2009 at 04:44:32PM -0600, Bob Montgomery wrote:
> > > John and I think that this code in gdb searches things too many times,
> > > particularly with this patch, but it's a start since it seems to fix the
> > > problem.
> > 
> > I'm attaching a new version of the patch, that performs way better when
> > disassembling functions that live in the kernel.  (Bob found that the
> > original patch made crash disassemble in-kernel functions at least 3
> > times slower, but that number will be larger depending on how close the
> > symbol table the function lives in is to the head of the psymtabs list.
> > Module disassembly speed wasn't changed much at all.)  With this updated
> > patch, we found the performance penalty of "dis -l" to be marginal.
> > 
> > The problem with the original patch is that once the address we want is
> > found in a symbol table, it then looks through the rest of the symbol
> > tables in that objfile for a better match.  The original code would then
> > return the best pst out of that objfile (and never get the next pst
> > from ALL_PSYMTABS), but we want to go through the rest of the objfiles
> > just in case, so I moved the return statement outside of the
> > ALL_PSYMTABS loop.  But the next pst from ALL_PSYMTABS will not be from
> > a new objfile - so we would wind up traversing the list (minus one
> > element) again, and again, and again...
> > 
> > The new patch removes the inner list traversal, and just takes advantage
> > of the fact that we already iterate through every pst via
> > ALL_PSYMTABS.
> 
> Hi John
> 
> Yeah, this second patch works much better.  In fact, I only noticed today -- using
> your first patch -- that if I pick a text address in the kernel proper that
> is very close to "_etext", the disassembly never returns from gdb.
> 
> For example, on an 2.6.29.4-167.fc11 kernel, if I do this with the
> first patch applied:
> 
>   crash> sym -l
>   ... [ snip ] ...
>   ffffffff813afac4 (T) register_kprobes  
>   ffffffff813afb26 (T) recycle_rp_inst  
>   ffffffff813afbbb (T) kprobe_flush_task  
>   ffffffff813afc75 (t) collect_one_slot  
>   ffffffff813afd18 (t) collect_garbage_slots  
>   ffffffff813afda9 (T) free_insn_slot  
>   ffffffff813afe4d (T) get_insn_slot  
>   ffffffff813aff85 (T) __kprobes_text_end  
>   ffffffff813b02ec (t) bad_iret  
>   ffffffff813b0313 (t) bad_gs  
>   ffffffff813b2250 (T) bad_from_user  
>   ffffffff813b2256 (t) bad_to_user  
>   ffffffff813b2830 (T) __start_notes  
>   ffffffff813b2830 (T) _etext
>   ...
> 
>   crash> dis -l register_kprobes
>   < never returns > 

I wonder if it would eventually return after minutes or hours?  (Not
that this is acceptable - I'm just curious.)

> With your new patch (and unpatched for that matter) it works OK.

Great!  Sorry for the original mess.  It was quick and dirty, and solved
the specific problem Bob and I were having.  I think this version is
correct, though.

-- 
+----------------------------------------------------------+
| John Wright <john.wright at hp.com>                         |
| HP Mission Critical OS Enablement & Solution Test (MOST) |
+----------------------------------------------------------+