[PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set

[PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set

[Ian Rogers]

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

[PATCH v1 2/2] perf tests sched stats: Write output to temp file

[Ian Rogers]

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

Re: [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set

[Swapnil Sapkal]

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

Re: [PATCH v1 2/2] perf tests sched stats: Write output to temp file

[Swapnil Sapkal]

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

Re: [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set

[Namhyung Kim]

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
> 

Re: [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set

[Ian Rogers]

On Thu, Mar 26, 2026 at 4:20 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> 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`.

Perhaps. I'd prefer to land the simpler change as-is.

Thanks,
Ian

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

Re: [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set

[Namhyung Kim]

On Tue, Mar 31, 2026 at 10:59:52PM -0700, Ian Rogers wrote:
> On Thu, Mar 26, 2026 at 4:20 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > 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`.
> 
> Perhaps. I'd prefer to land the simpler change as-is.

Well.. I guess the dummy handler approach would be simpler and perform
better. :)

Thanks,
Namhyung


diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index d083e2bb770303a4..9fb5447f9014d026 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -4955,6 +4955,7 @@ int cmd_sched(int argc, const char **argv)
                .switch_event       = replay_switch_event,
                .fork_event         = replay_fork_event,
        };
+       struct trace_sched_handler stats_ops  = {};
        int ret;
 
        perf_tool__init(&sched.tool, /*ordered_events=*/true);
@@ -5037,6 +5038,7 @@ int cmd_sched(int argc, const char **argv)
        } else if (!strcmp(argv[0], "stats")) {
                const char *const stats_subcommands[] = {"record", "report", NULL};
 
+               sched.tp_handler = &stats_ops;
                argc = parse_options_subcommand(argc, argv, stats_options,
                                                stats_subcommands,
                                                stats_usage,

Re: [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set

[Ian Rogers]

On Thu, Apr 2, 2026 at 7:42 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Tue, Mar 31, 2026 at 10:59:52PM -0700, Ian Rogers wrote:
> > On Thu, Mar 26, 2026 at 4:20 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > 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`.
> >
> > Perhaps. I'd prefer to land the simpler change as-is.
>
> Well.. I guess the dummy handler approach would be simpler and perform
> better. :)
>
> Thanks,
> Namhyung
>
>
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index d083e2bb770303a4..9fb5447f9014d026 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -4955,6 +4955,7 @@ int cmd_sched(int argc, const char **argv)
>                 .switch_event       = replay_switch_event,
>                 .fork_event         = replay_fork_event,
>         };
> +       struct trace_sched_handler stats_ops  = {};

Oh, because there's an existing null test on the function pointer we
don't need dummy functions, which is what I thought you were asking
for. I'm still not convinced this is very intention revealing, but if
it works for you then you can have my reviewed-by tag.

Thanks,
Ian

>         int ret;
>
>         perf_tool__init(&sched.tool, /*ordered_events=*/true);
> @@ -5037,6 +5038,7 @@ int cmd_sched(int argc, const char **argv)
>         } else if (!strcmp(argv[0], "stats")) {
>                 const char *const stats_subcommands[] = {"record", "report", NULL};
>
> +               sched.tp_handler = &stats_ops;
>                 argc = parse_options_subcommand(argc, argv, stats_options,
>                                                 stats_subcommands,
>                                                 stats_usage,
>

Re: [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set

[Namhyung Kim]

On Fri, Apr 03, 2026 at 08:29:25AM -0700, Ian Rogers wrote:
> On Thu, Apr 2, 2026 at 7:42 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Tue, Mar 31, 2026 at 10:59:52PM -0700, Ian Rogers wrote:
> > > On Thu, Mar 26, 2026 at 4:20 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > >
> > > > 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`.
> > >
> > > Perhaps. I'd prefer to land the simpler change as-is.
> >
> > Well.. I guess the dummy handler approach would be simpler and perform
> > better. :)
> >
> > Thanks,
> > Namhyung
> >
> >
> > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> > index d083e2bb770303a4..9fb5447f9014d026 100644
> > --- a/tools/perf/builtin-sched.c
> > +++ b/tools/perf/builtin-sched.c
> > @@ -4955,6 +4955,7 @@ int cmd_sched(int argc, const char **argv)
> >                 .switch_event       = replay_switch_event,
> >                 .fork_event         = replay_fork_event,
> >         };
> > +       struct trace_sched_handler stats_ops  = {};
> 
> Oh, because there's an existing null test on the function pointer we
> don't need dummy functions, which is what I thought you were asking
> for. I'm still not convinced this is very intention revealing, but if
> it works for you then you can have my reviewed-by tag.

Applied this and the patch 2 to perf-tools-next, thanks!

Best regards,
Namhyung

Re: [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set

[Ian Rogers]

On Mon, Apr 6, 2026 at 10:53 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Fri, Apr 03, 2026 at 08:29:25AM -0700, Ian Rogers wrote:
> > On Thu, Apr 2, 2026 at 7:42 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > On Tue, Mar 31, 2026 at 10:59:52PM -0700, Ian Rogers wrote:
> > > > On Thu, Mar 26, 2026 at 4:20 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > >
> > > > > 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`.
> > > >
> > > > Perhaps. I'd prefer to land the simpler change as-is.
> > >
> > > Well.. I guess the dummy handler approach would be simpler and perform
> > > better. :)
> > >
> > > Thanks,
> > > Namhyung
> > >
> > >
> > > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> > > index d083e2bb770303a4..9fb5447f9014d026 100644
> > > --- a/tools/perf/builtin-sched.c
> > > +++ b/tools/perf/builtin-sched.c
> > > @@ -4955,6 +4955,7 @@ int cmd_sched(int argc, const char **argv)
> > >                 .switch_event       = replay_switch_event,
> > >                 .fork_event         = replay_fork_event,
> > >         };
> > > +       struct trace_sched_handler stats_ops  = {};
> >
> > Oh, because there's an existing null test on the function pointer we
> > don't need dummy functions, which is what I thought you were asking
> > for. I'm still not convinced this is very intention revealing, but if
> > it works for you then you can have my reviewed-by tag.
>
> Applied this and the patch 2 to perf-tools-next, thanks!

Thanks Namhyung. Did this get accidentally dropped? I'm not seeing it
in perf-tools-next.

Ian

> Best regards,
> Namhyung
>

Re: [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set

[Arnaldo Carvalho de Melo]

On Tue, May 05, 2026 at 02:56:02PM -0700, Ian Rogers wrote:
> On Mon, Apr 6, 2026 at 10:53 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > On Fri, Apr 03, 2026 at 08:29:25AM -0700, Ian Rogers wrote:
> > > On Thu, Apr 2, 2026 at 7:42 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > On Tue, Mar 31, 2026 at 10:59:52PM -0700, Ian Rogers wrote:
> > > > > On Thu, Mar 26, 2026 at 4:20 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > > > 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`.

> > > > > Perhaps. I'd prefer to land the simpler change as-is.

> > > > Well.. I guess the dummy handler approach would be simpler and perform
> > > > better. :)

> > > > +++ b/tools/perf/builtin-sched.c
> > > > @@ -4955,6 +4955,7 @@ int cmd_sched(int argc, const char **argv)
> > > >                 .switch_event       = replay_switch_event,
> > > >                 .fork_event         = replay_fork_event,
> > > >         };
> > > > +       struct trace_sched_handler stats_ops  = {};
> > >
> > > Oh, because there's an existing null test on the function pointer we
> > > don't need dummy functions, which is what I thought you were asking
> > > for. I'm still not convinced this is very intention revealing, but if
> > > it works for you then you can have my reviewed-by tag.

> > Applied this and the patch 2 to perf-tools-next, thanks!
 
> Thanks Namhyung. Did this get accidentally dropped? I'm not seeing it
> in perf-tools-next.

Not applying:

⬢ [acme@toolbx perf-tools-next2]$ b4 am -ctsl --cc-trailers CAP-5=fW-H-ri3MdVv4Vh4xiyB-qpDqnO3gqXR+XyfMK+kXpHQA@mail.gmail.com
Grabbing thread from lore.kernel.org/all/CAP-5%3DfW-H-ri3MdVv4Vh4xiyB-qpDqnO3gqXR%2BXyfMK%2BkXpHQA@mail.gmail.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 10 messages in the thread
Analyzing 0 code-review messages
Checking attestation on all messages, may take a moment...
---
  ✗ [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set
    + Tested-by: Swapnil Sapkal <swapnil.sapkal@amd.com> (✓ DKIM/amd.com)
    + Link: https://lore.kernel.org/r/20260321061448.810525-1-irogers@google.com
    + Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
  ✗ [PATCH v1 2/2] perf tests sched stats: Write output to temp file
    + Tested-by: Swapnil Sapkal <swapnil.sapkal@amd.com> (✓ DKIM/amd.com)
    + Link: https://lore.kernel.org/r/20260321061448.810525-2-irogers@google.com
    + Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
  ---
  ✗ BADSIG: DKIM/google.com
---
Total patches: 2
---
 Link: https://lore.kernel.org/r/20260321061448.810525-1-irogers@google.com
 Base: not specified
       git am ./20260320_irogers_perf_sched_avoid_segv_if_tp_handler_not_set.mbx
⬢ [acme@toolbx perf-tools-next]$        git am ./20260320_irogers_perf_sched_avoid_segv_if_tp_handler_not_set.mbx
Applying: perf sched: Avoid segv if tp_handler not set
Applying: perf tests sched stats: Write output to temp file
error: patch failed: tools/perf/tests/shell/perf_sched_stats.sh:4
error: tools/perf/tests/shell/perf_sched_stats.sh: patch does not apply
Patch failed at 0002 perf tests sched stats: Write output to temp file
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
⬢ [acme@toolbx perf-tools-next2]$ git log --oneline -5
5a5b82a2a9bf1d3f (HEAD -> perf-tools-next) perf sched: Avoid segv if tp_handler not set
2d840126356314bf perf sched stats: Fix segmentation faults, memory leaks, and stale pointers in diff mode
8c8f2093614373ea perf build: Remove NO_GTK2 build test
53bc032892782e54 perf build: Add -fms-extensions for GEN_VMLINUX_H=1
e41cac8617a9b399 perf build: Update error message for BUILD_NONDISTRO=1
⬢ [acme@toolbx perf-tools-next]$

Re: [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set

[Namhyung Kim]

On Wed, May 06, 2026 at 10:10:15AM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, May 05, 2026 at 02:56:02PM -0700, Ian Rogers wrote:
> > On Mon, Apr 6, 2026 at 10:53 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > > On Fri, Apr 03, 2026 at 08:29:25AM -0700, Ian Rogers wrote:
> > > > On Thu, Apr 2, 2026 at 7:42 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > > On Tue, Mar 31, 2026 at 10:59:52PM -0700, Ian Rogers wrote:
> > > > > > On Thu, Mar 26, 2026 at 4:20 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > > > > 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`.
> 
> > > > > > Perhaps. I'd prefer to land the simpler change as-is.
> 
> > > > > Well.. I guess the dummy handler approach would be simpler and perform
> > > > > better. :)
> 
> > > > > +++ b/tools/perf/builtin-sched.c
> > > > > @@ -4955,6 +4955,7 @@ int cmd_sched(int argc, const char **argv)
> > > > >                 .switch_event       = replay_switch_event,
> > > > >                 .fork_event         = replay_fork_event,
> > > > >         };
> > > > > +       struct trace_sched_handler stats_ops  = {};
> > > >
> > > > Oh, because there's an existing null test on the function pointer we
> > > > don't need dummy functions, which is what I thought you were asking
> > > > for. I'm still not convinced this is very intention revealing, but if
> > > > it works for you then you can have my reviewed-by tag.
> 
> > > Applied this and the patch 2 to perf-tools-next, thanks!
>  
> > Thanks Namhyung. Did this get accidentally dropped? I'm not seeing it
> > in perf-tools-next.

I think I've applied my approach and test fix.

Thanks,
Namhyung

Re: [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set

[Ian Rogers]

On Wed, May 6, 2026 at 9:17 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Wed, May 06, 2026 at 10:10:15AM -0300, Arnaldo Carvalho de Melo wrote:
> > On Tue, May 05, 2026 at 02:56:02PM -0700, Ian Rogers wrote:
> > > On Mon, Apr 6, 2026 at 10:53 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > On Fri, Apr 03, 2026 at 08:29:25AM -0700, Ian Rogers wrote:
> > > > > On Thu, Apr 2, 2026 at 7:42 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > > > On Tue, Mar 31, 2026 at 10:59:52PM -0700, Ian Rogers wrote:
> > > > > > > On Thu, Mar 26, 2026 at 4:20 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > > > > > 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`.
> >
> > > > > > > Perhaps. I'd prefer to land the simpler change as-is.
> >
> > > > > > Well.. I guess the dummy handler approach would be simpler and perform
> > > > > > better. :)
> >
> > > > > > +++ b/tools/perf/builtin-sched.c
> > > > > > @@ -4955,6 +4955,7 @@ int cmd_sched(int argc, const char **argv)
> > > > > >                 .switch_event       = replay_switch_event,
> > > > > >                 .fork_event         = replay_fork_event,
> > > > > >         };
> > > > > > +       struct trace_sched_handler stats_ops  = {};
> > > > >
> > > > > Oh, because there's an existing null test on the function pointer we
> > > > > don't need dummy functions, which is what I thought you were asking
> > > > > for. I'm still not convinced this is very intention revealing, but if
> > > > > it works for you then you can have my reviewed-by tag.
> >
> > > > Applied this and the patch 2 to perf-tools-next, thanks!
> >
> > > Thanks Namhyung. Did this get accidentally dropped? I'm not seeing it
> > > in perf-tools-next.
>
> I think I've applied my approach and test fix.

Ok, that wasn't clear. I think it is good to post such changes to the
list. For example, just sync-ing the header files prompted sashiko to
point out a bug in our beauty scripts:
https://lore.kernel.org/linux-perf-users/20260504193334.9F978C2BCB8@smtp.kernel.org/

Thanks,
Ian

> Thanks,
> Namhyung
>

Re: [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set

[Namhyung Kim]

On Wed, May 06, 2026 at 10:12:05AM -0700, Ian Rogers wrote:
> On Wed, May 6, 2026 at 9:17 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Wed, May 06, 2026 at 10:10:15AM -0300, Arnaldo Carvalho de Melo wrote:
> > > On Tue, May 05, 2026 at 02:56:02PM -0700, Ian Rogers wrote:
> > > > On Mon, Apr 6, 2026 at 10:53 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > > On Fri, Apr 03, 2026 at 08:29:25AM -0700, Ian Rogers wrote:
> > > > > > On Thu, Apr 2, 2026 at 7:42 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > > > > On Tue, Mar 31, 2026 at 10:59:52PM -0700, Ian Rogers wrote:
> > > > > > > > On Thu, Mar 26, 2026 at 4:20 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > > > > > > 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`.
> > >
> > > > > > > > Perhaps. I'd prefer to land the simpler change as-is.
> > >
> > > > > > > Well.. I guess the dummy handler approach would be simpler and perform
> > > > > > > better. :)
> > >
> > > > > > > +++ b/tools/perf/builtin-sched.c
> > > > > > > @@ -4955,6 +4955,7 @@ int cmd_sched(int argc, const char **argv)
> > > > > > >                 .switch_event       = replay_switch_event,
> > > > > > >                 .fork_event         = replay_fork_event,
> > > > > > >         };
> > > > > > > +       struct trace_sched_handler stats_ops  = {};
> > > > > >
> > > > > > Oh, because there's an existing null test on the function pointer we
> > > > > > don't need dummy functions, which is what I thought you were asking
> > > > > > for. I'm still not convinced this is very intention revealing, but if
> > > > > > it works for you then you can have my reviewed-by tag.
> > >
> > > > > Applied this and the patch 2 to perf-tools-next, thanks!
> > >
> > > > Thanks Namhyung. Did this get accidentally dropped? I'm not seeing it
> > > > in perf-tools-next.
> >
> > I think I've applied my approach and test fix.
> 
> Ok, that wasn't clear. I think it is good to post such changes to the
> list. For example, just sync-ing the header files prompted sashiko to
> point out a bug in our beauty scripts:
> https://lore.kernel.org/linux-perf-users/20260504193334.9F978C2BCB8@smtp.kernel.org/

Ok, will do but patches are there.  I also plan to send out v2 for this
header sync.

Thanks,
Namhyung