All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Namhyung Kim <namhyung@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	James Clark <james.clark@linaro.org>,
	Swapnil Sapkal <swapnil.sapkal@amd.com>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 1/2] perf sched: Avoid segv if tp_handler not set
Date: Wed, 6 May 2026 10:10:15 -0300	[thread overview]
Message-ID: <afs9tx6hpi9CiEzN@x1> (raw)
In-Reply-To: <CAP-5=fW-H-ri3MdVv4Vh4xiyB-qpDqnO3gqXR+XyfMK+kXpHQA@mail.gmail.com>

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

  reply	other threads:[~2026-05-06 13:10 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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
2026-04-01  5:59   ` Ian Rogers
2026-04-03  2:42     ` Namhyung Kim
2026-04-03 15:29       ` Ian Rogers
2026-04-06 17:53         ` Namhyung Kim
2026-05-05 21:56           ` Ian Rogers
2026-05-06 13:10             ` Arnaldo Carvalho de Melo [this message]
2026-05-06 16:17               ` Namhyung Kim
2026-05-06 17:12                 ` Ian Rogers
2026-05-06 18:54                   ` Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=afs9tx6hpi9CiEzN@x1 \
    --to=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=swapnil.sapkal@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.