Hi Dave,
On Thu, Sep 24, 2009 at 03:00:49PM -0400, Dave Anderson wrote:
----- "John Wright (ALPS, Fort Collins)" <john.wright(a)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(a)hp.com> |
| HP Mission Critical OS Enablement & Solution Test (MOST) |
+----------------------------------------------------------+