| Message ID | 20200619122451.31162-1-nborisov@suse.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix enum values print in tracepoints | expand |
On Fri, Jun 19, 2020 at 03:24:45PM +0300, Nikolay Borisov wrote: > While looking at tracepoint dumps with trace-cmd report I observed that > tracepoints that should have printed text instead of raw values weren't > doing so: > > 13 kworker/u8:1-61 [000] 66.299527: btrfs_flush_space: 5302ee13-c65e-45bb-98ef-8fe3835bd943: state=3(0x3) flags=4(METADATA) num_bytes=2621440 ret=0 > > In the above line instead of (0x3) BTRFS_RESERVE_FLUSH_ALL should be printed. I.e > the correct output should be: > > 6 fio-370 [002] 56.762402: btrfs_trigger_flush: d04cd7ac-38e2-452f-a7f5-8157529fd5f0: preempt: flush=3(BTRFS_RESERVE_FLUSH_ALL) flags=4(METADATA) bytes=655360 > > Investigating this turned out to be caused because enum values weren't exported > to user space via TRACE_DEFINE_ENUM. This is required in order for user space > tools to correctly map the raw binary values to their textual representation. > More information can be found in commit 190f0b76ca49 ("mm: tracing: Export enums in tracepoints to user space") > > This series follows the approach taken by 190f0b76ca49 in defining the various > enum mapping structures. So I see where the unintuitive naming EM and EMe comes from, but ok let's stick to that. I've looked at the result and it could really use the comments from 190f0b76ca49 where the definitions switch the output and some whitespace fixups in the new definitions. Not all enums are converted, just search for use of __print_symbolic inside the show_TYPE helpers (eg. show_ref_action, show_ref_type), please add them too. Thanks.
On 22.06.20 г. 17:21 ч., David Sterba wrote: > On Fri, Jun 19, 2020 at 03:24:45PM +0300, Nikolay Borisov wrote: >> While looking at tracepoint dumps with trace-cmd report I observed that >> tracepoints that should have printed text instead of raw values weren't >> doing so: >> >> 13 kworker/u8:1-61 [000] 66.299527: btrfs_flush_space: 5302ee13-c65e-45bb-98ef-8fe3835bd943: state=3(0x3) flags=4(METADATA) num_bytes=2621440 ret=0 >> >> In the above line instead of (0x3) BTRFS_RESERVE_FLUSH_ALL should be printed. I.e >> the correct output should be: >> >> 6 fio-370 [002] 56.762402: btrfs_trigger_flush: d04cd7ac-38e2-452f-a7f5-8157529fd5f0: preempt: flush=3(BTRFS_RESERVE_FLUSH_ALL) flags=4(METADATA) bytes=655360 >> >> Investigating this turned out to be caused because enum values weren't exported >> to user space via TRACE_DEFINE_ENUM. This is required in order for user space >> tools to correctly map the raw binary values to their textual representation. >> More information can be found in commit 190f0b76ca49 ("mm: tracing: Export enums in tracepoints to user space") >> >> This series follows the approach taken by 190f0b76ca49 in defining the various >> enum mapping structures. > > So I see where the unintuitive naming EM and EMe comes from, but ok > let's stick to that. Yeah, the difference between EM and EMe is the ending comma that's needed when actually defining the array passed to __print_symbols. > > I've looked at the result and it could really use the comments from > 190f0b76ca49 where the definitions switch the output and some whitespace > fixups in the new definitions. > > Not all enums are converted, just search for use of __print_symbolic > inside the show_TYPE helpers (eg. show_ref_action, show_ref_type), > please add them too. Thanks. I beg you to differ: show_ref_action/show_ref_type/__show_root_type/__show_map_type/ - those are defined and they are OK as is, because defines don't emit separate symbols. However, when/if in the future those defines are switched to enums then the respective tracepoint code should be converted to using the EM macros as well. >
On Mon, Jun 22, 2020 at 06:19:39PM +0300, Nikolay Borisov wrote: > > I've looked at the result and it could really use the comments from > > 190f0b76ca49 where the definitions switch the output and some whitespace > > fixups in the new definitions. > > > > Not all enums are converted, just search for use of __print_symbolic > > inside the show_TYPE helpers (eg. show_ref_action, show_ref_type), > > please add them too. Thanks. > > I beg you to differ: > > show_ref_action/show_ref_type/__show_root_type/__show_map_type/ - those > are defined and they are OK as is, because defines don't emit separate > symbols. However, when/if in the future those defines are switched to > enums then the respective tracepoint code should be converted to using > the EM macros as well. I see, it's define vs enum.
On Fri, Jun 19, 2020 at 03:24:45PM +0300, Nikolay Borisov wrote: > While looking at tracepoint dumps with trace-cmd report I observed that > tracepoints that should have printed text instead of raw values weren't > doing so: > > 13 kworker/u8:1-61 [000] 66.299527: btrfs_flush_space: 5302ee13-c65e-45bb-98ef-8fe3835bd943: state=3(0x3) flags=4(METADATA) num_bytes=2621440 ret=0 > > In the above line instead of (0x3) BTRFS_RESERVE_FLUSH_ALL should be printed. I.e > the correct output should be: > > 6 fio-370 [002] 56.762402: btrfs_trigger_flush: d04cd7ac-38e2-452f-a7f5-8157529fd5f0: preempt: flush=3(BTRFS_RESERVE_FLUSH_ALL) flags=4(METADATA) bytes=655360 > > Investigating this turned out to be caused because enum values weren't exported > to user space via TRACE_DEFINE_ENUM. This is required in order for user space > tools to correctly map the raw binary values to their textual representation. > More information can be found in commit 190f0b76ca49 ("mm: tracing: Export enums in tracepoints to user space") > > This series follows the approach taken by 190f0b76ca49 in defining the various > enum mapping structures. > > Nikolay Borisov (6): > btrfs: tracepoints: Fix btrfs_trigger_flush printout > btrfs: tracepoints: Fix extent type symbolic name print > btrfs: tracepoints: Move FLUSH_ACTIONS define > btrfs: tracepoints: Fix qgroup reservation type printing > btrfs: tracepoints: Switch extent_io_tree_owner to using EM macro > btrfs: tracepoints: Convert flush states to using EM macros Whitespace fixed and series added to misc-next, thanks.
While looking at tracepoint dumps with trace-cmd report I observed that tracepoints that should have printed text instead of raw values weren't doing so: 13 kworker/u8:1-61 [000] 66.299527: btrfs_flush_space: 5302ee13-c65e-45bb-98ef-8fe3835bd943: state=3(0x3) flags=4(METADATA) num_bytes=2621440 ret=0 In the above line instead of (0x3) BTRFS_RESERVE_FLUSH_ALL should be printed. I.e the correct output should be: 6 fio-370 [002] 56.762402: btrfs_trigger_flush: d04cd7ac-38e2-452f-a7f5-8157529fd5f0: preempt: flush=3(BTRFS_RESERVE_FLUSH_ALL) flags=4(METADATA) bytes=655360 Investigating this turned out to be caused because enum values weren't exported to user space via TRACE_DEFINE_ENUM. This is required in order for user space tools to correctly map the raw binary values to their textual representation. More information can be found in commit 190f0b76ca49 ("mm: tracing: Export enums in tracepoints to user space") This series follows the approach taken by 190f0b76ca49 in defining the various enum mapping structures. Nikolay Borisov (6): btrfs: tracepoints: Fix btrfs_trigger_flush printout btrfs: tracepoints: Fix extent type symbolic name print btrfs: tracepoints: Move FLUSH_ACTIONS define btrfs: tracepoints: Fix qgroup reservation type printing btrfs: tracepoints: Switch extent_io_tree_owner to using EM macro btrfs: tracepoints: Convert flush states to using EM macros include/trace/events/btrfs.h | 128 +++++++++++++++++++---------------- 1 file changed, 68 insertions(+), 60 deletions(-) -- 2.17.1