diff mbox series

[RFC,v3,6/6] libtraceevent: prefer to use prev_state_char introduced in sched_switch

Message ID 20230801090124.8050-7-zegao@tencent.com
State New
Headers
Series add to report task state in symbolic chars from sched tracepoint |

Commit Message

Ze Gao Aug. 1, 2023, 9:01 a.m. UTC 
  Since the sched_switch tracepoint introduces a new variable to
report sched-out task state in symbolic char, we prefer to use
it to spare from knowing internal implementations in kernel.

Also we keep the old parsing logic intact but sync the state char
array with the latest kernel.

Signed-off-by: Ze Gao <zegao@tencent.com>
---
 plugins/plugin_sched_switch.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Steven Rostedt Aug. 1, 2023, 2:19 p.m. UTC | #1 
On Tue,  1 Aug 2023 17:01:24 +0800
Ze Gao <zegao2021@gmail.com> wrote:

> Since the sched_switch tracepoint introduces a new variable to
> report sched-out task state in symbolic char, we prefer to use
> it to spare from knowing internal implementations in kernel.
> 
> Also we keep the old parsing logic intact but sync the state char
> array with the latest kernel.

This should be two patches. First sync the state char array and then add
your state_char change. The two changes are agnostic to each other, and
should be separate commits. Same goes for the perf changes.

-- Steve


> 
> Signed-off-by: Ze Gao <zegao@tencent.com>
> ---
>  plugins/plugin_sched_switch.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/plugins/plugin_sched_switch.c b/plugins/plugin_sched_switch.c
> index 8752cae..4c57322 100644
> --- a/plugins/plugin_sched_switch.c
> +++ b/plugins/plugin_sched_switch.c
> @@ -11,7 +11,7 @@
>  
>  static void write_state(struct trace_seq *s, int val)
>  {
> -	const char states[] = "SDTtZXxW";
> +	const char states[] = "SDTtXZPIp";
>  	int found = 0;
>  	int i;
>  
> @@ -99,7 +99,12 @@ static int sched_switch_handler(struct trace_seq *s,
>  	if (tep_get_field_val(s, event, "prev_prio", record, &val, 1) == 0)
>  		trace_seq_printf(s, "[%d] ", (int) val);
>  
> -	if (tep_get_field_val(s,  event, "prev_state", record, &val, 1) == 0)
> +	//find if has prev_state_char, otherwise fallback to prev_state
> +	if (tep_find_field(event, "prev_state_char")) {
> +		if (tep_get_field_val(s,  event, "prev_state_char", record, &val, 1) == 0)
> +			trace_seq_putc(s, (char) val);
> +	}
> +	else if (tep_get_field_val(s,  event, "prev_state", record, &val, 1) == 0)
>  		write_state(s, val);
>  
>  	trace_seq_puts(s, " ==> ");
Ze Gao Aug. 2, 2023, 3:08 a.m. UTC | #2 
Fair enough! Already did this in perf fixes. Will push a v4
to do this.

Thanks,
Ze


On Tue, Aug 1, 2023 at 10:19 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue,  1 Aug 2023 17:01:24 +0800
> Ze Gao <zegao2021@gmail.com> wrote:
>
> > Since the sched_switch tracepoint introduces a new variable to
> > report sched-out task state in symbolic char, we prefer to use
> > it to spare from knowing internal implementations in kernel.
> >
> > Also we keep the old parsing logic intact but sync the state char
> > array with the latest kernel.
>
> This should be two patches. First sync the state char array and then add
> your state_char change. The two changes are agnostic to each other, and
> should be separate commits. Same goes for the perf changes.
>
> -- Steve
>
>
> >
> > Signed-off-by: Ze Gao <zegao@tencent.com>
> > ---
> >  plugins/plugin_sched_switch.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/plugins/plugin_sched_switch.c b/plugins/plugin_sched_switch.c
> > index 8752cae..4c57322 100644
> > --- a/plugins/plugin_sched_switch.c
> > +++ b/plugins/plugin_sched_switch.c
> > @@ -11,7 +11,7 @@
> >
> >  static void write_state(struct trace_seq *s, int val)
> >  {
> > -     const char states[] = "SDTtZXxW";
> > +     const char states[] = "SDTtXZPIp";
> >       int found = 0;
> >       int i;
> >
> > @@ -99,7 +99,12 @@ static int sched_switch_handler(struct trace_seq *s,
> >       if (tep_get_field_val(s, event, "prev_prio", record, &val, 1) == 0)
> >               trace_seq_printf(s, "[%d] ", (int) val);
> >
> > -     if (tep_get_field_val(s,  event, "prev_state", record, &val, 1) == 0)
> > +     //find if has prev_state_char, otherwise fallback to prev_state
> > +     if (tep_find_field(event, "prev_state_char")) {
> > +             if (tep_get_field_val(s,  event, "prev_state_char", record, &val, 1) == 0)
> > +                     trace_seq_putc(s, (char) val);
> > +     }
> > +     else if (tep_get_field_val(s,  event, "prev_state", record, &val, 1) == 0)
> >               write_state(s, val);
> >
> >       trace_seq_puts(s, " ==> ");
>
diff mbox series

Patch

diff --git a/plugins/plugin_sched_switch.c b/plugins/plugin_sched_switch.c
index 8752cae..4c57322 100644
--- a/plugins/plugin_sched_switch.c
+++ b/plugins/plugin_sched_switch.c
@@ -11,7 +11,7 @@ 
 
 static void write_state(struct trace_seq *s, int val)
 {
-	const char states[] = "SDTtZXxW";
+	const char states[] = "SDTtXZPIp";
 	int found = 0;
 	int i;
 
@@ -99,7 +99,12 @@  static int sched_switch_handler(struct trace_seq *s,
 	if (tep_get_field_val(s, event, "prev_prio", record, &val, 1) == 0)
 		trace_seq_printf(s, "[%d] ", (int) val);
 
-	if (tep_get_field_val(s,  event, "prev_state", record, &val, 1) == 0)
+	//find if has prev_state_char, otherwise fallback to prev_state
+	if (tep_find_field(event, "prev_state_char")) {
+		if (tep_get_field_val(s,  event, "prev_state_char", record, &val, 1) == 0)
+			trace_seq_putc(s, (char) val);
+	}
+	else if (tep_get_field_val(s,  event, "prev_state", record, &val, 1) == 0)
 		write_state(s, val);
 
 	trace_seq_puts(s, " ==> ");