2022-03-11 05:51:49

by Ritesh Harjani

[permalink] [raw]
Subject: Re: [PATCHv2 00/10] ext4: Improve FC trace events

On 22/03/10 09:33PM, Steven Rostedt wrote:
> On Fri, 11 Mar 2022 07:49:31 +0530
> Ritesh Harjani <[email protected]> wrote:
>
> > > # cat /sys/kernel/tracing/events/ext4/ext4_fc_commit_stop/format
> >
> > I think you meant ext4_fc_stats.
>
> Sure.
>
> >
> > >
> > > and show me what it outputs.
> >
> > root@qemu:/home/qemu# cat /sys/kernel/tracing/events/ext4/ext4_fc_stats/format
> > name: ext4_fc_stats
> > ID: 986
> > format:
> > field:unsigned short common_type; offset:0; size:2; signed:0;
> > field:unsigned char common_flags; offset:2; size:1; signed:0;
> > field:unsigned char common_preempt_count; offset:3; size:1; signed:0;
> > field:int common_pid; offset:4; size:4; signed:1;
> >
> > field:dev_t dev; offset:8; size:4; signed:0;
> > field:unsigned int fc_ineligible_rc[EXT4_FC_REASON_MAX]; offset:12; size:36; signed:0;
>
> Bah, the above tells us how many items, and the TRACE_DEFINE_ENUM() doesn't
> modify this part of the file.

Then shall I just define TRACE_DEFINE_ENUM(EXT4_FC_REASON_MAX) in this patch.
Would that be the correct approach? Like how we have defined other enums.
We haven't yet defined EXT4_FC_REASON_MAX in current patch.
(as I saw it doesn't affect TP_STRUCT__entry())


>
> I could update it to do so though.

Please let me know if you have any patch for me to try.

Thanks
-ritesh


>
> -- Steve
>
>
> > field:unsigned long fc_commits; offset:48; size:8; signed:0;
> > field:unsigned long fc_ineligible_commits; offset:56; size:8; signed:0;
> > field:unsigned long fc_numblks; offset:64; size:8; signed:0;
>


2022-03-11 14:05:27

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCHv2 00/10] ext4: Improve FC trace events

On Fri, 11 Mar 2022 08:44:31 +0530
Ritesh Harjani <[email protected]> wrote:

> > I could update it to do so though.
>
> Please let me know if you have any patch for me to try.

Can you try this?

-- Steve

From 392b91c598da2a8c5bbaebad08cd0410f4607bf4 Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (Google)" <[email protected]>
Date: Thu, 10 Mar 2022 23:27:38 -0500
Subject: [PATCH] tracing: Have TRACE_DEFINE_ENUM affect trace event types as
well

The macro TRACE_DEFINE_ENUM is used to convert enums in the kernel to
their actual value when they are exported to user space via the trace
event format file.

Currently only the enums in the "print fmt" (TP_printk in the TRACE_EVENT
macro) have the enums converted. But the enums can be used to denote array
size:

field:unsigned int fc_ineligible_rc[EXT4_FC_REASON_MAX]; offset:12; size:36; signed:0;

The EXT4_FC_REASON_MAX has no meaning to userspace but it needs to know
that information to know how to parse the array.

Have the array indexes also be parsed as well.

Link: https://lore.kernel.org/all/[email protected]/

Reported-by: Ritesh Harjani <[email protected]>
Signed-off-by: Steven Rostedt (Google) <[email protected]>
---
kernel/trace/trace_events.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 38afd66d80e3..ae9a3b8481f5 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2633,6 +2633,33 @@ static void update_event_printk(struct trace_event_call *call,
}
}

+static void update_event_fields(struct trace_event_call *call,
+ struct trace_eval_map *map)
+{
+ struct ftrace_event_field *field;
+ struct list_head *head;
+ char *ptr;
+ int len = strlen(map->eval_string);
+
+ head = trace_get_fields(call);
+ list_for_each_entry(field, head, link) {
+ ptr = strchr(field->type, '[');
+ if (!ptr)
+ continue;
+ ptr++;
+
+ if (!isalpha(*ptr) && *ptr != '_')
+ continue;
+
+ if (strncmp(map->eval_string, ptr, len) != 0)
+ continue;
+
+ ptr = eval_replace(ptr, map, len);
+ /* enum/sizeof string smaller than value */
+ WARN_ON_ONCE(!ptr);
+ }
+}
+
void trace_event_eval_update(struct trace_eval_map **map, int len)
{
struct trace_event_call *call, *p;
@@ -2668,6 +2695,7 @@ void trace_event_eval_update(struct trace_eval_map **map, int len)
first = false;
}
update_event_printk(call, map[i]);
+ update_event_fields(call, map[i]);
}
}
}
--
2.34.1