linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf record: Fix wrong comm in system-wide mode with delay
@ 2021-08-27 23:32 Namhyung Kim
  2021-08-30 17:33 ` Ian Rogers
  0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2021-08-27 23:32 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ingo Molnar, Peter Zijlstra, LKML, Andi Kleen, Ian Rogers,
	Adrian Hunter, Stephane Eranian

Stephane found that the name of the forked process in a system-wide
mode is wrong when --delay option is used.  For example,

  # perf record -a --delay=1000  noploop 3

The noploop process will run a busy loop for 3 second.  And on an idle
machine it should show up at the top in the perf report.  It works
well without the --delay option.  But if I add the option, it showed
'perf' not 'noploop'.

  # perf report -s comm -q | head -3
      52.94%  perf
      16.65%  swapper
      12.04%  chrome

It turned out that the dummy event didn't work at all and it missed
COMM and MMAP events for the noploop process (and others too).  We
should enable the dummy event immediately in system-wide mode, as the
enable-on-exec would work only for task events.

With this change,

  # perf report -s comm -q | head -3
      52.75%  noploop
      17.03%  swapper
      12.83%  chrome

Cc: Adrian Hunter <adrian.hunter@intel.com>
Reported-by: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-record.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 548c1dbde6c5..acfe66e31cf0 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -910,7 +910,8 @@ static int record__open(struct record *rec)
 		 * Enable the dummy event when the process is forked for
 		 * initial_delay, immediately for system wide.
 		 */
-		if (opts->initial_delay && !pos->immediate)
+		if (opts->initial_delay && !pos->immediate &&
+		    !target__has_cpu(&opts->target))
 			pos->core.attr.enable_on_exec = 1;
 		else
 			pos->immediate = 1;
-- 
2.33.0.259.gc128427fd7-goog


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

* Re: [PATCH] perf record: Fix wrong comm in system-wide mode with delay
  2021-08-27 23:32 [PATCH] perf record: Fix wrong comm in system-wide mode with delay Namhyung Kim
@ 2021-08-30 17:33 ` Ian Rogers
  2021-08-30 18:54   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2021-08-30 17:33 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra,
	LKML, Andi Kleen, Adrian Hunter, Stephane Eranian

On Fri, Aug 27, 2021 at 4:32 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Stephane found that the name of the forked process in a system-wide
> mode is wrong when --delay option is used.  For example,
>
>   # perf record -a --delay=1000  noploop 3
>
> The noploop process will run a busy loop for 3 second.  And on an idle
> machine it should show up at the top in the perf report.  It works
> well without the --delay option.  But if I add the option, it showed
> 'perf' not 'noploop'.
>
>   # perf report -s comm -q | head -3
>       52.94%  perf
>       16.65%  swapper
>       12.04%  chrome
>
> It turned out that the dummy event didn't work at all and it missed
> COMM and MMAP events for the noploop process (and others too).  We
> should enable the dummy event immediately in system-wide mode, as the
> enable-on-exec would work only for task events.
>
> With this change,
>
>   # perf report -s comm -q | head -3
>       52.75%  noploop
>       17.03%  swapper
>       12.83%  chrome
>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Reported-by: Stephane Eranian <eranian@google.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/builtin-record.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 548c1dbde6c5..acfe66e31cf0 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -910,7 +910,8 @@ static int record__open(struct record *rec)
>                  * Enable the dummy event when the process is forked for
>                  * initial_delay, immediately for system wide.
>                  */
> -               if (opts->initial_delay && !pos->immediate)
> +               if (opts->initial_delay && !pos->immediate &&
> +                   !target__has_cpu(&opts->target))
>                         pos->core.attr.enable_on_exec = 1;
>                 else
>                         pos->immediate = 1;
> --
> 2.33.0.259.gc128427fd7-goog
>

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

* Re: [PATCH] perf record: Fix wrong comm in system-wide mode with delay
  2021-08-30 17:33 ` Ian Rogers
@ 2021-08-30 18:54   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-08-30 18:54 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Namhyung Kim, Jiri Olsa, Ingo Molnar, Peter Zijlstra, LKML,
	Andi Kleen, Adrian Hunter, Stephane Eranian

Em Mon, Aug 30, 2021 at 10:33:16AM -0700, Ian Rogers escreveu:
> On Fri, Aug 27, 2021 at 4:32 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > Stephane found that the name of the forked process in a system-wide
> > mode is wrong when --delay option is used.  For example,
> >
> >   # perf record -a --delay=1000  noploop 3
> >
> > The noploop process will run a busy loop for 3 second.  And on an idle
> > machine it should show up at the top in the perf report.  It works
> > well without the --delay option.  But if I add the option, it showed
> > 'perf' not 'noploop'.
> >
> >   # perf report -s comm -q | head -3
> >       52.94%  perf
> >       16.65%  swapper
> >       12.04%  chrome
> >
> > It turned out that the dummy event didn't work at all and it missed
> > COMM and MMAP events for the noploop process (and others too).  We
> > should enable the dummy event immediately in system-wide mode, as the
> > enable-on-exec would work only for task events.
> >
> > With this change,
> >
> >   # perf report -s comm -q | head -3
> >       52.75%  noploop
> >       17.03%  swapper
> >       12.83%  chrome
> >
> > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > Reported-by: Stephane Eranian <eranian@google.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> Acked-by: Ian Rogers <irogers@google.com>

Thanks, applied.

- Arnaldo


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

end of thread, other threads:[~2021-08-30 18:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-27 23:32 [PATCH] perf record: Fix wrong comm in system-wide mode with delay Namhyung Kim
2021-08-30 17:33 ` Ian Rogers
2021-08-30 18:54   ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).