All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] perf sched: Prefer sched_waking event when it exists
@ 2020-08-07 16:48 David Ahern
  2020-08-07 19:43 ` Arnaldo Carvalho de Melo
  2020-08-11  6:42 ` Namhyung Kim
  0 siblings, 2 replies; 6+ messages in thread
From: David Ahern @ 2020-08-07 16:48 UTC (permalink / raw)
  To: acme; +Cc: namhyung, linux-kernel, jolsa, David Ahern

Commit fbd705a0c618 ("sched: Introduce the 'trace_sched_waking' tracepoint")
added sched_waking tracepoint which should be preferred over sched_wakeup
when analyzing scheduling delays.

Update 'perf sched record' to collect sched_waking events if it exists
and fallback to sched_wakeup if it does not. Similarly, update timehist
command to skip sched_wakeup events if the session includes
sched_waking (ie., sched_waking is preferred over sched_wakeup).

Signed-off-by: David Ahern <dsahern@kernel.org>
---
 tools/perf/builtin-sched.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 459e4229945e..0c7d599fa555 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -2398,6 +2398,15 @@ static void timehist_print_wakeup_event(struct perf_sched *sched,
 	printf("\n");
 }
 
+static int timehist_sched_wakeup_ignore(struct perf_tool *tool __maybe_unused,
+					union perf_event *event __maybe_unused,
+					struct evsel *evsel __maybe_unused,
+					struct perf_sample *sample __maybe_unused,
+					struct machine *machine __maybe_unused)
+{
+	return 0;
+}
+
 static int timehist_sched_wakeup_event(struct perf_tool *tool,
 				       union perf_event *event __maybe_unused,
 				       struct evsel *evsel,
@@ -2958,9 +2967,10 @@ static int timehist_check_attr(struct perf_sched *sched,
 
 static int perf_sched__timehist(struct perf_sched *sched)
 {
-	const struct evsel_str_handler handlers[] = {
+	struct evsel_str_handler handlers[] = {
 		{ "sched:sched_switch",       timehist_sched_switch_event, },
 		{ "sched:sched_wakeup",	      timehist_sched_wakeup_event, },
+		{ "sched:sched_waking",       timehist_sched_wakeup_event, },
 		{ "sched:sched_wakeup_new",   timehist_sched_wakeup_event, },
 	};
 	const struct evsel_str_handler migrate_handlers[] = {
@@ -3018,6 +3028,11 @@ static int perf_sched__timehist(struct perf_sched *sched)
 
 	setup_pager();
 
+	/* prefer sched_waking if it is captured */
+	if (perf_evlist__find_tracepoint_by_name(session->evlist,
+						  "sched:sched_waking"))
+		handlers[1].handler = timehist_sched_wakeup_ignore;
+
 	/* setup per-evsel handlers */
 	if (perf_session__set_tracepoints_handlers(session, handlers))
 		goto out;
@@ -3330,12 +3345,16 @@ static int __cmd_record(int argc, const char **argv)
 		"-e", "sched:sched_stat_iowait",
 		"-e", "sched:sched_stat_runtime",
 		"-e", "sched:sched_process_fork",
-		"-e", "sched:sched_wakeup",
 		"-e", "sched:sched_wakeup_new",
 		"-e", "sched:sched_migrate_task",
 	};
+	struct tep_event *waking_event;
 
-	rec_argc = ARRAY_SIZE(record_args) + argc - 1;
+	/*
+	 * +2 for either "-e", "sched:sched_wakeup" or
+	 * "-e", "sched:sched_waking"
+	 */
+	rec_argc = ARRAY_SIZE(record_args) + 2 + argc - 1;
 	rec_argv = calloc(rec_argc + 1, sizeof(char *));
 
 	if (rec_argv == NULL)
@@ -3344,6 +3363,13 @@ static int __cmd_record(int argc, const char **argv)
 	for (i = 0; i < ARRAY_SIZE(record_args); i++)
 		rec_argv[i] = strdup(record_args[i]);
 
+	rec_argv[i++] = "-e";
+	waking_event = trace_event__tp_format("sched", "sched_waking");
+	if (!IS_ERR(waking_event))
+		rec_argv[i++] = strdup("sched:sched_waking");
+	else
+		rec_argv[i++] = strdup("sched:sched_wakeup");
+
 	for (j = 1; j < (unsigned int)argc; j++, i++)
 		rec_argv[i] = argv[j];
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] perf sched: Prefer sched_waking event when it exists
  2020-08-07 16:48 [PATCH 1/2] perf sched: Prefer sched_waking event when it exists David Ahern
@ 2020-08-07 19:43 ` Arnaldo Carvalho de Melo
  2020-08-07 19:50   ` David Ahern
  2020-08-11  6:42 ` Namhyung Kim
  1 sibling, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-08-07 19:43 UTC (permalink / raw)
  To: David Ahern; +Cc: namhyung, linux-kernel, jolsa

Em Fri, Aug 07, 2020 at 10:48:44AM -0600, David Ahern escreveu:
> Commit fbd705a0c618 ("sched: Introduce the 'trace_sched_waking' tracepoint")
> added sched_waking tracepoint which should be preferred over sched_wakeup
> when analyzing scheduling delays.
> 
> Update 'perf sched record' to collect sched_waking events if it exists
> and fallback to sched_wakeup if it does not. Similarly, update timehist
> command to skip sched_wakeup events if the session includes
> sched_waking (ie., sched_waking is preferred over sched_wakeup).
> 
> Signed-off-by: David Ahern <dsahern@kernel.org>
> ---
>  tools/perf/builtin-sched.c | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 459e4229945e..0c7d599fa555 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -2398,6 +2398,15 @@ static void timehist_print_wakeup_event(struct perf_sched *sched,
>  	printf("\n");
>  }
>  
> +static int timehist_sched_wakeup_ignore(struct perf_tool *tool __maybe_unused,
> +					union perf_event *event __maybe_unused,
> +					struct evsel *evsel __maybe_unused,
> +					struct perf_sample *sample __maybe_unused,
> +					struct machine *machine __maybe_unused)
> +{
> +	return 0;
> +}
> +
>  static int timehist_sched_wakeup_event(struct perf_tool *tool,
>  				       union perf_event *event __maybe_unused,
>  				       struct evsel *evsel,
> @@ -2958,9 +2967,10 @@ static int timehist_check_attr(struct perf_sched *sched,
>  
>  static int perf_sched__timehist(struct perf_sched *sched)
>  {
> -	const struct evsel_str_handler handlers[] = {
> +	struct evsel_str_handler handlers[] = {
>  		{ "sched:sched_switch",       timehist_sched_switch_event, },
>  		{ "sched:sched_wakeup",	      timehist_sched_wakeup_event, },
> +		{ "sched:sched_waking",       timehist_sched_wakeup_event, },
>  		{ "sched:sched_wakeup_new",   timehist_sched_wakeup_event, },
>  	};
>  	const struct evsel_str_handler migrate_handlers[] = {
> @@ -3018,6 +3028,11 @@ static int perf_sched__timehist(struct perf_sched *sched)
>  
>  	setup_pager();
>  
> +	/* prefer sched_waking if it is captured */
> +	if (perf_evlist__find_tracepoint_by_name(session->evlist,
> +						  "sched:sched_waking"))
> +		handlers[1].handler = timehist_sched_wakeup_ignore;
> +


ouch, can't we figure out if its present and then don't ask for the
wakeup one to be recorded?

- Arnaldo

>  	/* setup per-evsel handlers */
>  	if (perf_session__set_tracepoints_handlers(session, handlers))
>  		goto out;
> @@ -3330,12 +3345,16 @@ static int __cmd_record(int argc, const char **argv)
>  		"-e", "sched:sched_stat_iowait",
>  		"-e", "sched:sched_stat_runtime",
>  		"-e", "sched:sched_process_fork",
> -		"-e", "sched:sched_wakeup",
>  		"-e", "sched:sched_wakeup_new",
>  		"-e", "sched:sched_migrate_task",
>  	};
> +	struct tep_event *waking_event;
>  
> -	rec_argc = ARRAY_SIZE(record_args) + argc - 1;
> +	/*
> +	 * +2 for either "-e", "sched:sched_wakeup" or
> +	 * "-e", "sched:sched_waking"
> +	 */
> +	rec_argc = ARRAY_SIZE(record_args) + 2 + argc - 1;
>  	rec_argv = calloc(rec_argc + 1, sizeof(char *));
>  
>  	if (rec_argv == NULL)
> @@ -3344,6 +3363,13 @@ static int __cmd_record(int argc, const char **argv)
>  	for (i = 0; i < ARRAY_SIZE(record_args); i++)
>  		rec_argv[i] = strdup(record_args[i]);
>  
> +	rec_argv[i++] = "-e";
> +	waking_event = trace_event__tp_format("sched", "sched_waking");
> +	if (!IS_ERR(waking_event))
> +		rec_argv[i++] = strdup("sched:sched_waking");
> +	else
> +		rec_argv[i++] = strdup("sched:sched_wakeup");
> +
>  	for (j = 1; j < (unsigned int)argc; j++, i++)
>  		rec_argv[i] = argv[j];
>  
> -- 
> 2.17.1
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] perf sched: Prefer sched_waking event when it exists
  2020-08-07 19:43 ` Arnaldo Carvalho de Melo
@ 2020-08-07 19:50   ` David Ahern
  2020-08-10 16:22     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 6+ messages in thread
From: David Ahern @ 2020-08-07 19:50 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, David Ahern; +Cc: namhyung, linux-kernel, jolsa

On 8/7/20 1:43 PM, Arnaldo Carvalho de Melo wrote:
>> @@ -2958,9 +2967,10 @@ static int timehist_check_attr(struct perf_sched *sched,
>>  
>>  static int perf_sched__timehist(struct perf_sched *sched)
>>  {
>> -	const struct evsel_str_handler handlers[] = {
>> +	struct evsel_str_handler handlers[] = {
>>  		{ "sched:sched_switch",       timehist_sched_switch_event, },
>>  		{ "sched:sched_wakeup",	      timehist_sched_wakeup_event, },
>> +		{ "sched:sched_waking",       timehist_sched_wakeup_event, },
>>  		{ "sched:sched_wakeup_new",   timehist_sched_wakeup_event, },
>>  	};
>>  	const struct evsel_str_handler migrate_handlers[] = {
>> @@ -3018,6 +3028,11 @@ static int perf_sched__timehist(struct perf_sched *sched)
>>  
>>  	setup_pager();
>>  
>> +	/* prefer sched_waking if it is captured */
>> +	if (perf_evlist__find_tracepoint_by_name(session->evlist,
>> +						  "sched:sched_waking"))
>> +		handlers[1].handler = timehist_sched_wakeup_ignore;
>> +
> 
> 
> ouch, can't we figure out if its present and then don't ask for the
> wakeup one to be recorded?
> 

This is the analysis side. If someone recorded with sched:* we do not
want to analyze both sched_wakeup and sched_waking. Rather, it should
prefer the latter and ignore the former.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] perf sched: Prefer sched_waking event when it exists
  2020-08-07 19:50   ` David Ahern
@ 2020-08-10 16:22     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-08-10 16:22 UTC (permalink / raw)
  To: David Ahern; +Cc: David Ahern, namhyung, linux-kernel, jolsa

Em Fri, Aug 07, 2020 at 01:50:47PM -0600, David Ahern escreveu:
> On 8/7/20 1:43 PM, Arnaldo Carvalho de Melo wrote:
> >> @@ -2958,9 +2967,10 @@ static int timehist_check_attr(struct perf_sched *sched,
> >>  
> >>  static int perf_sched__timehist(struct perf_sched *sched)
> >>  {
> >> -	const struct evsel_str_handler handlers[] = {
> >> +	struct evsel_str_handler handlers[] = {
> >>  		{ "sched:sched_switch",       timehist_sched_switch_event, },
> >>  		{ "sched:sched_wakeup",	      timehist_sched_wakeup_event, },
> >> +		{ "sched:sched_waking",       timehist_sched_wakeup_event, },
> >>  		{ "sched:sched_wakeup_new",   timehist_sched_wakeup_event, },
> >>  	};
> >>  	const struct evsel_str_handler migrate_handlers[] = {
> >> @@ -3018,6 +3028,11 @@ static int perf_sched__timehist(struct perf_sched *sched)
> >>  
> >>  	setup_pager();
> >>  
> >> +	/* prefer sched_waking if it is captured */
> >> +	if (perf_evlist__find_tracepoint_by_name(session->evlist,
> >> +						  "sched:sched_waking"))
> >> +		handlers[1].handler = timehist_sched_wakeup_ignore;
> >> +
> > 
> > 
> > ouch, can't we figure out if its present and then don't ask for the
> > wakeup one to be recorded?
> > 
> 
> This is the analysis side. If someone recorded with sched:* we do not
> want to analyze both sched_wakeup and sched_waking. Rather, it should
> prefer the latter and ignore the former.

Right you are, thans for the explanation, I should've noticed that :)

- Arnaldoi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] perf sched: Prefer sched_waking event when it exists
  2020-08-07 16:48 [PATCH 1/2] perf sched: Prefer sched_waking event when it exists David Ahern
  2020-08-07 19:43 ` Arnaldo Carvalho de Melo
@ 2020-08-11  6:42 ` Namhyung Kim
  2020-08-12 12:19   ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2020-08-11  6:42 UTC (permalink / raw)
  To: David Ahern; +Cc: Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa

Hi David,

On Sat, Aug 8, 2020 at 1:48 AM David Ahern <dsahern@kernel.org> wrote:
>
> Commit fbd705a0c618 ("sched: Introduce the 'trace_sched_waking' tracepoint")
> added sched_waking tracepoint which should be preferred over sched_wakeup
> when analyzing scheduling delays.
>
> Update 'perf sched record' to collect sched_waking events if it exists
> and fallback to sched_wakeup if it does not. Similarly, update timehist
> command to skip sched_wakeup events if the session includes
> sched_waking (ie., sched_waking is preferred over sched_wakeup).
>
> Signed-off-by: David Ahern <dsahern@kernel.org>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks

Namhyung

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] perf sched: Prefer sched_waking event when it exists
  2020-08-11  6:42 ` Namhyung Kim
@ 2020-08-12 12:19   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-08-12 12:19 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: David Ahern, linux-kernel, Jiri Olsa

Em Tue, Aug 11, 2020 at 03:42:45PM +0900, Namhyung Kim escreveu:
> Hi David,
> 
> On Sat, Aug 8, 2020 at 1:48 AM David Ahern <dsahern@kernel.org> wrote:
> >
> > Commit fbd705a0c618 ("sched: Introduce the 'trace_sched_waking' tracepoint")
> > added sched_waking tracepoint which should be preferred over sched_wakeup
> > when analyzing scheduling delays.
> >
> > Update 'perf sched record' to collect sched_waking events if it exists
> > and fallback to sched_wakeup if it does not. Similarly, update timehist
> > command to skip sched_wakeup events if the session includes
> > sched_waking (ie., sched_waking is preferred over sched_wakeup).
> >
> > Signed-off-by: David Ahern <dsahern@kernel.org>
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks, applied.

- Arnaldo

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-08-12 12:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-07 16:48 [PATCH 1/2] perf sched: Prefer sched_waking event when it exists David Ahern
2020-08-07 19:43 ` Arnaldo Carvalho de Melo
2020-08-07 19:50   ` David Ahern
2020-08-10 16:22     ` Arnaldo Carvalho de Melo
2020-08-11  6:42 ` Namhyung Kim
2020-08-12 12:19   ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.