public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Arnaldo Carvalho de Melo <acme@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 11:54:02 -0700	[thread overview]
Message-ID: <afuOSvbCaUgFRQUI@google.com> (raw)
In-Reply-To: <CAP-5=fUuF4LpSMy30AuLoz-wKy9O8ZLkh+UWBrDJTwNV-D=eqw@mail.gmail.com>

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


      reply	other threads:[~2026-05-06 18:54 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
2026-05-06 16:17               ` Namhyung Kim
2026-05-06 17:12                 ` Ian Rogers
2026-05-06 18:54                   ` Namhyung Kim [this message]

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=afuOSvbCaUgFRQUI@google.com \
    --to=namhyung@kernel.org \
    --cc=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=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox