Re: [PATCH v1 3/3] perf probe: Generate hash event for long symbol

[Google]

On Thu, 10 Oct 2024 16:53:05 +0100
Leo Yan <leo.yan@xxxxxxx> wrote:

> Hi Masami,
> 
> On 10/10/24 16:34, Masami Hiramatsu (Google) wrote:
> > 
> > 
> > On Mon,  7 Oct 2024 15:11:16 +0100
> > Leo Yan <leo.yan@xxxxxxx> wrote:
> > 
> >> If a symbol name is longer than the maximum event length (64 bytes),
> >> generate an new event name with below combination:
> >>
> >>    TruncatedSymbol + '_' + HashString + '__return' + '\0'
> >>      `> 46B        + 1B  +   8B       +    8B      + 1B   = 64 Bytes.
> >>
> >> With this change, a probe can be injected for long symbol.
> >>
> >> Before:
> >>
> >>    # nm test_cpp_mangle | grep -E "print_data|Point"
> >>    0000000000000cac t _GLOBAL__sub_I__Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi
> >>    0000000000000b50 T _Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzR5Point
> >>    0000000000000b14 T _Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi
> >>
> >>    # perf probe -x test_cpp_mangle --add \
> >>          "_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi"
> >>    snprintf() failed: -7; the event name nbase='_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi' is too long
> >>    Error: Failed to add events.
> >>
> >> After:
> >>
> >>    # perf probe -x test_cpp_mangle --add \
> >>        "_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi"
> >>
> >>    Probe event='_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi' is too long (>= 64 bytes).
> >>    Generate hashed event name='_Z62this_is_a_very_very_long_print_data_abcdef_91f40679'
> >>
> >>    Added new event:
> >>      probe_test_cpp_mangle: _Z62this_is_a_very_very_long_print_data_abcdef_91f40679
> >>      (on _Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi in /mnt/test_cpp_mangle)
> >>
> >>    You can now use it in all perf tools, such as:
> >>
> >>        perf record -e probe_test_cpp_mangle: _Z62this_is_a_very_very_long_print_data_abcdef_91f40679 -aR sleep 1
> > 
> > OK, personally, I recommend you to specify event name instead of generating
> > long event name in this case. But I understand sometimes this kind of feature
> > is good for someone.
> 
> Sometimes, users try to add probe for long symbol and returns error, but there 
>   have no clue for proceeding.

OK, no warning messsage is not good.
It should warn them to recommend adding it with their own event name too.

> Either we automatically generate a hashed name, or a guidance in the failure 
> log for setting event name would be helpful. If you have concern for hashed 
> name, maybe we can refine the log to give info for setting event name?

Yeah, I think this long event name is not useful for user to type.

> > BTW, I would like to confirm. Can't we demangle the symbol name and parse it?
> 
> I did test for C++ demangle symbols with the command:
> 
>    perf probe -x /mnt/test_cpp_mangle -F --demangle
> 
> The command doesn't work as I cannot see it output correctly for demangled 
> symbols. But I don't look into details why this does not work at my side.

Oops, that is another issue to be solved.

Thank you,

> 
> Thanks for review.
> 
> Leo


-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>