[ This is helpful information for trace users and developers so I've
included the two lists ]
On Tue, 30 Aug 2022 12:59:46 +0200 (CEST)
Michael Petlan <mpetlan@xxxxxxxxxx> wrote:
> Hello Steve,
>
> during perf testing, I have found out that the print formats of some tracepoints
> are translated to quite complicated code, that differs per architecture and quite
> often contains various builtins and functions that are not recognized by the parser
> in libtraceevent.
>
> It looks like this:
>
> [root@gigabyte-r120-03 base_kmem]# perf kmem --page record -o ./perf.data -- true
> libtraceevent: No such file or directory
> [kmem:mm_page_alloc] function __builtin_constant_p not defined
> [kmem:mm_page_free] function __builtin_constant_p not defined
>
> I have digged down to event-parse.c source and commits like 1b20d9491cf9 ("tools
> lib traceevent: Add handler for __builtin_expect()") that add some missing func
> support.
>
> I have added __builtin_constant_p, it helped to suppress the warning, but then
> I found out I was missing sizeof()...
>
> On an x86_64 F35 machine, I have found the following list of missing funcs:
> __builtin_choose_expr
> decode_prq_descriptor
> jiffies_to_msecs
> libata_trace_parse_eh_action
> libata_trace_parse_host_stat
> libata_trace_parse_qc_flags
> libata_trace_parse_subcmd
> libata_trace_parse_tf_flags
> mc_event_error_type
> pgd_val
> pmd_val
> pte_val
> pud_val
> scsi_trace_parse_cdb
> sizeof
The problem with a lot of the above is that user space does not know how to
parse them. "jiffies_to_msecs" depends on knowing the HZ value. Now if the
kernel exported this, we could look for it and save this information, and
then be able to parse it correctly.
> xen_hypercall_name
> xhci_decode_ctrl_ctx
> xhci_decode_doorbell
> xhci_decode_ep_context
> xhci_decode_portsc
> xhci_decode_slot_context
> xhci_ring_type_string
>
> There are of course more, since on other platforms, the print format expands to
> different code.
>
> I wonder, whether it might be useful to add these functions to libtraceevent
> parser. Finally, what is the purpose of the handlers, such as __builtin_expect()
> you have added? It looks like that the parser machine now accepts this token and
> handles that it has to have two parameters, but does nothing more with it. It
> lets the parser to dive deeper and process the first arg and forgets the second,
> just like replacing
> __builtin_expect( exp, c)
> by
> exp
> so when something executes the code, it ignores the builtin? Am I right?
Correct. As __builtin_expect() is just a compile time optimization that
"likely()" and "unlikely()" are defined to, it has no meaning to the
parser. But we still want the contents of the expression.
I had to update libtraceevent to basically ignore the __builtin_expect().
That is, "unlikely(cond)" turns into "!!__builtin_exepect(cond, 0)"
libtraceevent does not care about the "unlikely()" part, only the
condition, so the parser basically treats it the same:
"unlikely(cond)" == "cond"
>
> The __builtin_constant_p(arg) should decide whether arg is constant or not and
> return 1 or 0 respectively...
>
What is using __builting_constant_p() inside the TP_printk()?
> My questions are:
> 1) Does it matther whether or not the above functions are accepted by the parser
> grammar or not?
When it can't parse something, it falls back to just printing out the raw
fields the best it can.
> 2) Does (1) affect how the code is later executed somewhere?
It affects how it prints the event. It then ignores the TP_printk() format,
and then, as mentioned above, just prints the raw fields.
> 3) Would e.g. replacing __builtin_constant_p(arg) by 0 by default help/simplify
> anything?
I have to see how it's used. What event does that?
-- Steve
|