public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set
@ 2026-03-21  6:14 Ian Rogers
  2026-03-21  6:14 ` [PATCH v1 2/2] perf tests sched stats: Write output to temp file Ian Rogers
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Ian Rogers @ 2026-03-21  6:14 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Alexander Shishkin, Jiri Olsa, Ian Rogers,
	Adrian Hunter, James Clark, Swapnil Sapkal, linux-perf-users,
	linux-kernel

Doing a `perf sched record` then `perf sched stats report` crashes as
the tp_handler isn't set. Add extra checks that tp_handler is set
before accessing through it.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-sched.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index d083e2bb7703..196f263ff667 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1525,7 +1525,7 @@ static int process_sched_wakeup_event(const struct perf_tool *tool,
 {
 	struct perf_sched *sched = container_of(tool, struct perf_sched, tool);
 
-	if (sched->tp_handler->wakeup_event)
+	if (sched->tp_handler && sched->tp_handler->wakeup_event)
 		return sched->tp_handler->wakeup_event(sched, evsel, sample, machine);
 
 	return 0;
@@ -1809,7 +1809,7 @@ static int process_sched_switch_event(const struct perf_tool *tool,
 			sched->nr_context_switch_bugs++;
 	}
 
-	if (sched->tp_handler->switch_event)
+	if (sched->tp_handler && sched->tp_handler->switch_event)
 		err = sched->tp_handler->switch_event(sched, evsel, sample, machine);
 
 	sched->curr_pid[this_cpu] = next_pid;
@@ -1823,7 +1823,7 @@ static int process_sched_runtime_event(const struct perf_tool *tool,
 {
 	struct perf_sched *sched = container_of(tool, struct perf_sched, tool);
 
-	if (sched->tp_handler->runtime_event)
+	if (sched->tp_handler && sched->tp_handler->runtime_event)
 		return sched->tp_handler->runtime_event(sched, evsel, sample, machine);
 
 	return 0;
@@ -1840,7 +1840,7 @@ static int perf_sched__process_fork_event(const struct perf_tool *tool,
 	perf_event__process_fork(tool, event, sample, machine);
 
 	/* and then run additional processing needed for this command */
-	if (sched->tp_handler->fork_event)
+	if (sched->tp_handler && sched->tp_handler->fork_event)
 		return sched->tp_handler->fork_event(sched, event, machine);
 
 	return 0;
@@ -1853,7 +1853,7 @@ static int process_sched_migrate_task_event(const struct perf_tool *tool,
 {
 	struct perf_sched *sched = container_of(tool, struct perf_sched, tool);
 
-	if (sched->tp_handler->migrate_task_event)
+	if (sched->tp_handler && sched->tp_handler->migrate_task_event)
 		return sched->tp_handler->migrate_task_event(sched, evsel, sample, machine);
 
 	return 0;
-- 
2.53.0.959.g497ff81fa9-goog


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

* [PATCH v1 2/2] perf tests sched stats: Write output to temp file
  2026-03-21  6:14 [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set Ian Rogers
@ 2026-03-21  6:14 ` Ian Rogers
  2026-03-23 10:41   ` Swapnil Sapkal
  2026-03-23 10:39 ` [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set Swapnil Sapkal
  2026-03-26 23:20 ` Namhyung Kim
  2 siblings, 1 reply; 5+ messages in thread
From: Ian Rogers @ 2026-03-21  6:14 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Alexander Shishkin, Jiri Olsa, Ian Rogers,
	Adrian Hunter, James Clark, Swapnil Sapkal, linux-perf-users,
	linux-kernel

Writing to the perf.data file can fail in various contexts such as
continual test. Other tests write to a mktemp-ed file, make the "perf
sched stats tests" follow this convention.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/perf_sched_stats.sh | 37 ++++++++++++++++------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/tools/perf/tests/shell/perf_sched_stats.sh b/tools/perf/tests/shell/perf_sched_stats.sh
index 2b1410b050d0..bef7714ef37a 100755
--- a/tools/perf/tests/shell/perf_sched_stats.sh
+++ b/tools/perf/tests/shell/perf_sched_stats.sh
@@ -4,10 +4,29 @@
 
 set -e
 
+perfdata=$(mktemp /tmp/__perf_test_sched_stats.perf.data.XXXXX)
+perfdata2=$(mktemp /tmp/__perf_test_sched_stats.perf.data.XXXXX)
+
+cleanup() {
+  rm -f "${perfdata}"
+  rm -f "${perfdata}".old
+  rm -f "${perfdata2}"
+  rm -f "${perfdata2}".old
+
+  trap - EXIT TERM INT
+}
+
+trap_cleanup() {
+  echo "Unexpected signal in ${FUNCNAME[1]}"
+  cleanup
+  exit 1
+}
+trap trap_cleanup EXIT TERM INT
+
 err=0
 test_perf_sched_stats_record() {
   echo "Basic perf sched stats record test"
-  if ! perf sched stats record true 2>&1 | \
+  if ! perf sched stats record -o "${perfdata}" true 2>&1 | \
     grep -E -q "[ perf sched stats: Wrote samples to perf.data ]"
   then
     echo "Basic perf sched stats record test [Failed]"
@@ -19,15 +38,13 @@ test_perf_sched_stats_record() {
 
 test_perf_sched_stats_report() {
   echo "Basic perf sched stats report test"
-  perf sched stats record true > /dev/null
-  if ! perf sched stats report 2>&1 | grep -E -q "Description"
+  perf sched stats record -o "${perfdata}" true > /dev/null
+  if ! perf sched stats report -i "${perfdata}" 2>&1 | grep -E -q "Description"
   then
     echo "Basic perf sched stats report test [Failed]"
     err=1
-    rm perf.data
     return
   fi
-  rm perf.data
   echo "Basic perf sched stats report test [Success]"
 }
 
@@ -44,16 +61,14 @@ test_perf_sched_stats_live() {
 
 test_perf_sched_stats_diff() {
   echo "Basic perf sched stats diff test"
-  perf sched stats record true > /dev/null
-  perf sched stats record true > /dev/null
-  if ! perf sched stats diff > /dev/null
+  perf sched stats record -o "${perfdata}" true > /dev/null
+  perf sched stats record -o "${perfdata2}" true > /dev/null
+  if ! perf sched stats diff "${perfdata}" "${perfdata2}" > /dev/null
   then
     echo "Basic perf sched stats diff test [Failed]"
     err=1
-    rm perf.data.old perf.data
     return
   fi
-  rm perf.data.old perf.data
   echo "Basic perf sched stats diff test [Success]"
 }
 
@@ -61,4 +76,6 @@ test_perf_sched_stats_record
 test_perf_sched_stats_report
 test_perf_sched_stats_live
 test_perf_sched_stats_diff
+
+cleanup
 exit $err
-- 
2.53.0.959.g497ff81fa9-goog


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

* Re: [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set
  2026-03-21  6:14 [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set Ian Rogers
  2026-03-21  6:14 ` [PATCH v1 2/2] perf tests sched stats: Write output to temp file Ian Rogers
@ 2026-03-23 10:39 ` Swapnil Sapkal
  2026-03-26 23:20 ` Namhyung Kim
  2 siblings, 0 replies; 5+ messages in thread
From: Swapnil Sapkal @ 2026-03-23 10:39 UTC (permalink / raw)
  To: Ian Rogers, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	James Clark, linux-perf-users, linux-kernel

Hi Ian,

On 21-03-2026 11:44, Ian Rogers wrote:
> Doing a `perf sched record` then `perf sched stats report` crashes as
> the tp_handler isn't set. Add extra checks that tp_handler is set
> before accessing through it.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>   tools/perf/builtin-sched.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index d083e2bb7703..196f263ff667 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -1525,7 +1525,7 @@ static int process_sched_wakeup_event(const struct perf_tool *tool,
>   {
>   	struct perf_sched *sched = container_of(tool, struct perf_sched, tool);
>   
> -	if (sched->tp_handler->wakeup_event)
> +	if (sched->tp_handler && sched->tp_handler->wakeup_event)
>   		return sched->tp_handler->wakeup_event(sched, evsel, sample, machine);
>   
>   	return 0;
> @@ -1809,7 +1809,7 @@ static int process_sched_switch_event(const struct perf_tool *tool,
>   			sched->nr_context_switch_bugs++;
>   	}
>   
> -	if (sched->tp_handler->switch_event)
> +	if (sched->tp_handler && sched->tp_handler->switch_event)
>   		err = sched->tp_handler->switch_event(sched, evsel, sample, machine);
>   
>   	sched->curr_pid[this_cpu] = next_pid;
> @@ -1823,7 +1823,7 @@ static int process_sched_runtime_event(const struct perf_tool *tool,
>   {
>   	struct perf_sched *sched = container_of(tool, struct perf_sched, tool);
>   
> -	if (sched->tp_handler->runtime_event)
> +	if (sched->tp_handler && sched->tp_handler->runtime_event)
>   		return sched->tp_handler->runtime_event(sched, evsel, sample, machine);
>   
>   	return 0;
> @@ -1840,7 +1840,7 @@ static int perf_sched__process_fork_event(const struct perf_tool *tool,
>   	perf_event__process_fork(tool, event, sample, machine);
>   
>   	/* and then run additional processing needed for this command */
> -	if (sched->tp_handler->fork_event)
> +	if (sched->tp_handler && sched->tp_handler->fork_event)
>   		return sched->tp_handler->fork_event(sched, event, machine);
>   
>   	return 0;
> @@ -1853,7 +1853,7 @@ static int process_sched_migrate_task_event(const struct perf_tool *tool,
>   {
>   	struct perf_sched *sched = container_of(tool, struct perf_sched, tool);
>   
> -	if (sched->tp_handler->migrate_task_event)
> +	if (sched->tp_handler && sched->tp_handler->migrate_task_event)
>   		return sched->tp_handler->migrate_task_event(sched, evsel, sample, machine);
>   
>   	return 0;

Tested-by: Swapnil Sapkal <swapnil.sapkal@amd.com>

--
Thanks and Regards,
Swapnil

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

* Re: [PATCH v1 2/2] perf tests sched stats: Write output to temp file
  2026-03-21  6:14 ` [PATCH v1 2/2] perf tests sched stats: Write output to temp file Ian Rogers
@ 2026-03-23 10:41   ` Swapnil Sapkal
  0 siblings, 0 replies; 5+ messages in thread
From: Swapnil Sapkal @ 2026-03-23 10:41 UTC (permalink / raw)
  To: Ian Rogers, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	James Clark, linux-perf-users, linux-kernel

Hi Ian,

On 21-03-2026 11:44, Ian Rogers wrote:
> Writing to the perf.data file can fail in various contexts such as
> continual test. Other tests write to a mktemp-ed file, make the "perf
> sched stats tests" follow this convention.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>   tools/perf/tests/shell/perf_sched_stats.sh | 37 ++++++++++++++++------
>   1 file changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/perf/tests/shell/perf_sched_stats.sh b/tools/perf/tests/shell/perf_sched_stats.sh
> index 2b1410b050d0..bef7714ef37a 100755
> --- a/tools/perf/tests/shell/perf_sched_stats.sh
> +++ b/tools/perf/tests/shell/perf_sched_stats.sh
> @@ -4,10 +4,29 @@
>   
>   set -e
>   
> +perfdata=$(mktemp /tmp/__perf_test_sched_stats.perf.data.XXXXX)
> +perfdata2=$(mktemp /tmp/__perf_test_sched_stats.perf.data.XXXXX)
> +
> +cleanup() {
> +  rm -f "${perfdata}"
> +  rm -f "${perfdata}".old
> +  rm -f "${perfdata2}"
> +  rm -f "${perfdata2}".old
> +
> +  trap - EXIT TERM INT
> +}
> +
> +trap_cleanup() {
> +  echo "Unexpected signal in ${FUNCNAME[1]}"
> +  cleanup
> +  exit 1
> +}
> +trap trap_cleanup EXIT TERM INT
> +
>   err=0
>   test_perf_sched_stats_record() {
>     echo "Basic perf sched stats record test"
> -  if ! perf sched stats record true 2>&1 | \
> +  if ! perf sched stats record -o "${perfdata}" true 2>&1 | \
>       grep -E -q "[ perf sched stats: Wrote samples to perf.data ]"
>     then
>       echo "Basic perf sched stats record test [Failed]"
> @@ -19,15 +38,13 @@ test_perf_sched_stats_record() {
>   
>   test_perf_sched_stats_report() {
>     echo "Basic perf sched stats report test"
> -  perf sched stats record true > /dev/null
> -  if ! perf sched stats report 2>&1 | grep -E -q "Description"
> +  perf sched stats record -o "${perfdata}" true > /dev/null
> +  if ! perf sched stats report -i "${perfdata}" 2>&1 | grep -E -q "Description"
>     then
>       echo "Basic perf sched stats report test [Failed]"
>       err=1
> -    rm perf.data
>       return
>     fi
> -  rm perf.data
>     echo "Basic perf sched stats report test [Success]"
>   }
>   
> @@ -44,16 +61,14 @@ test_perf_sched_stats_live() {
>   
>   test_perf_sched_stats_diff() {
>     echo "Basic perf sched stats diff test"
> -  perf sched stats record true > /dev/null
> -  perf sched stats record true > /dev/null
> -  if ! perf sched stats diff > /dev/null
> +  perf sched stats record -o "${perfdata}" true > /dev/null
> +  perf sched stats record -o "${perfdata2}" true > /dev/null
> +  if ! perf sched stats diff "${perfdata}" "${perfdata2}" > /dev/null
>     then
>       echo "Basic perf sched stats diff test [Failed]"
>       err=1
> -    rm perf.data.old perf.data
>       return
>     fi
> -  rm perf.data.old perf.data
>     echo "Basic perf sched stats diff test [Success]"
>   }
>   
> @@ -61,4 +76,6 @@ test_perf_sched_stats_record
>   test_perf_sched_stats_report
>   test_perf_sched_stats_live
>   test_perf_sched_stats_diff
> +
> +cleanup
>   exit $err

Tested-by: Swapnil Sapkal <swapnil.sapkal@amd.com>

--
Thanks and Regards,
Swapnil

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

* Re: [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set
  2026-03-21  6:14 [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set Ian Rogers
  2026-03-21  6:14 ` [PATCH v1 2/2] perf tests sched stats: Write output to temp file Ian Rogers
  2026-03-23 10:39 ` [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set Swapnil Sapkal
@ 2026-03-26 23:20 ` Namhyung Kim
  2 siblings, 0 replies; 5+ messages in thread
From: Namhyung Kim @ 2026-03-26 23:20 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark,
	Swapnil Sapkal, linux-perf-users, linux-kernel

On Fri, Mar 20, 2026 at 11:14:47PM -0700, Ian Rogers wrote:
> Doing a `perf sched record` then `perf sched stats report` crashes as
> the tp_handler isn't set. Add extra checks that tp_handler is set
> before accessing through it.

Oh.. unintended use case. :)  Probably better to add a dummy handler
for `perf sched stats report`.

Thanks,
Namhyung

> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/builtin-sched.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index d083e2bb7703..196f263ff667 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -1525,7 +1525,7 @@ static int process_sched_wakeup_event(const struct perf_tool *tool,
>  {
>  	struct perf_sched *sched = container_of(tool, struct perf_sched, tool);
>  
> -	if (sched->tp_handler->wakeup_event)
> +	if (sched->tp_handler && sched->tp_handler->wakeup_event)
>  		return sched->tp_handler->wakeup_event(sched, evsel, sample, machine);
>  
>  	return 0;
> @@ -1809,7 +1809,7 @@ static int process_sched_switch_event(const struct perf_tool *tool,
>  			sched->nr_context_switch_bugs++;
>  	}
>  
> -	if (sched->tp_handler->switch_event)
> +	if (sched->tp_handler && sched->tp_handler->switch_event)
>  		err = sched->tp_handler->switch_event(sched, evsel, sample, machine);
>  
>  	sched->curr_pid[this_cpu] = next_pid;
> @@ -1823,7 +1823,7 @@ static int process_sched_runtime_event(const struct perf_tool *tool,
>  {
>  	struct perf_sched *sched = container_of(tool, struct perf_sched, tool);
>  
> -	if (sched->tp_handler->runtime_event)
> +	if (sched->tp_handler && sched->tp_handler->runtime_event)
>  		return sched->tp_handler->runtime_event(sched, evsel, sample, machine);
>  
>  	return 0;
> @@ -1840,7 +1840,7 @@ static int perf_sched__process_fork_event(const struct perf_tool *tool,
>  	perf_event__process_fork(tool, event, sample, machine);
>  
>  	/* and then run additional processing needed for this command */
> -	if (sched->tp_handler->fork_event)
> +	if (sched->tp_handler && sched->tp_handler->fork_event)
>  		return sched->tp_handler->fork_event(sched, event, machine);
>  
>  	return 0;
> @@ -1853,7 +1853,7 @@ static int process_sched_migrate_task_event(const struct perf_tool *tool,
>  {
>  	struct perf_sched *sched = container_of(tool, struct perf_sched, tool);
>  
> -	if (sched->tp_handler->migrate_task_event)
> +	if (sched->tp_handler && sched->tp_handler->migrate_task_event)
>  		return sched->tp_handler->migrate_task_event(sched, evsel, sample, machine);
>  
>  	return 0;
> -- 
> 2.53.0.959.g497ff81fa9-goog
> 

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

end of thread, other threads:[~2026-03-26 23:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-21  6:14 [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set Ian Rogers
2026-03-21  6:14 ` [PATCH v1 2/2] perf tests sched stats: Write output to temp file Ian Rogers
2026-03-23 10:41   ` Swapnil Sapkal
2026-03-23 10:39 ` [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set Swapnil Sapkal
2026-03-26 23:20 ` Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox