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

[Ritesh Harjani]

On 22/03/10 11:32PM, Steven Rostedt wrote:
> On Fri, 11 Mar 2022 08:44:31 +0530
> Ritesh Harjani <riteshh@xxxxxxxxxxxxx> 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?

Thanks a lot Steve for quick patch.
I tested your patch and it seems to be working fine.

Below are the details -

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[9]; offset:12;      size:36;        signed:0;
^^^^ It's now taking the enum value of EXT4_FC_REASON_MAX in array field too.

Also output from trace-cmd and perf script shows the right data

xfs_io  1856 [000]   173.411127:        ext4:ext4_fc_stats: dev 7,2 fc ineligible reasons:
XATTR:0, CROSS_RENAME:0, JOURNAL_FLAG_CHANGE:0, NO_MEM:0, SWAP_BOOT:0, RESIZE:0, RENAME_DIR:0, FALLOC_RANGE:2, INODE_JOURNAL_DATA:0 num_commits:0, ineligible: 1, numblks: 0

>
> -- Steve
>
> From 392b91c598da2a8c5bbaebad08cd0410f4607bf4 Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>
> 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/cover.1646922487.git.riteshh@xxxxxxxxxxxxx/
>
> Reported-by: Ritesh Harjani <riteshh@xxxxxxxxxxxxx>
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
> ---
>  kernel/trace/trace_events.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)

You may add below, if you like:-

Reported-and-tested-by: Ritesh Harjani <riteshh@xxxxxxxxxxxxx>

-ritesh


>
> 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
>