All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Wangnan (F)" <wangnan0@huawei.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Kan Liang <kan.liang@intel.com>
Cc: Ingo Molnar <mingo@kernel.org>, <linux-kernel@vger.kernel.org>,
	"Arnaldo Carvalho de Melo" <acme@redhat.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Borislav Petkov <bp@suse.de>, David Ahern <dsahern@gmail.com>,
	"Frederic Weisbecker" <fweisbec@gmail.com>,
	Namhyung Kim <namhyung@kernel.org>,
	"Stephane Eranian" <eranian@google.com>
Subject: Re: [PATCH 10/13] perf report: Do not blindly use env->cpu[al.cpu].socket_id
Date: Fri, 11 Sep 2015 19:50:54 +0800	[thread overview]
Message-ID: <55F2C01E.5020701@huawei.com> (raw)
In-Reply-To: <1441828225-667-11-git-send-email-acme@kernel.org>



On 2015/9/10 3:50, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> As al.cpu may be -1, i.e. no PERF_SAMPLE_CPU, and env->cpu may be NULL.
>
> Rely instead on the work now done in perf_event__preprocess_sample(),
> that does all those checks.
>
> Reported-by: Wang Nan <wangnan0@huawei.com>
> Based-on-a-patch-by: Jiri Olsa <jolsa@kernel.org>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Kan Liang <kan.liang@intel.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Cc: Wang Nan <wangnan0@huawei.com>
> Link: http://lkml.kernel.org/n/tip-2lw80g5ehsrec7tozhmnjgxw@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>   tools/perf/builtin-report.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 4b432453922f..9b5083630a56 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -150,7 +150,6 @@ static int process_sample_event(struct perf_tool *tool,
>   		.add_entry_cb 		= hist_iter__report_callback,
>   	};
>   	int ret = 0;
> -	struct perf_env *env = evsel->evlist->env;
>   
>   	if (perf_event__preprocess_sample(event, machine, &al, sample) < 0) {
>   		pr_debug("problem processing %d event, skipping it.\n",
> @@ -158,9 +157,6 @@ static int process_sample_event(struct perf_tool *tool,
>   		return -1;
>   	}
>   
> -	/* read socket id from perf.data for perf report */
> -	al.socket = env->cpu[al.cpu].socket_id;
> -
>   	if (rep->hide_unresolved && al.sym == NULL)
>   		goto out_put;
>   

I tested this patch on an 8 cores SOC. No segfault found now.

Normal case:

  # ./perf record -a ls
  # ...
  # ./perf_arm64 report -v  --stdio -s socket,cpu
  build id event received for [kernel.kallsyms]: ...
  # To display the perf.data header info, please use 
--header/--header-only options.
  #
  symsrc__init: cannot get elf header.
  Looking at the vmlinux_path (7 entries long)
  Failed to open /proc/kcore. Note /proc/kcore requires CAP_SYS_RAWIO 
capability to access.
  Using /proc/kallsyms for symbols
  Failed to open /sbin/adbd, continuing without symbols
  #
  # Total Lost Samples: 0
  #
  # Samples: 291  of event 'cycles'
  # Event count (approx.): 40968659
  #
  # Overhead  Socket  CPU
  # ........  ......  ...
  #
      58.01%     000  004
      35.61%     000  007
       3.65%     000  005
       1.23%     001  000
       1.18%     001  001
       0.19%     000  006
       0.08%     001  002
       0.07%     001  003

If one of CPUs is offlined before 'perf record':

  # ./perf record -a ls
  # ...
  # ./perf_arm64 report -v  --stdio -s socket,cpu
  build id event received for [kernel.kallsyms]: 
d287ff3393fb1a01d3a785c3a1dac6e63d973bce
  # To display the perf.data header info, please use 
--header/--header-only options.
  #
  symsrc__init: cannot get elf header.
  Looking at the vmlinux_path (7 entries long)
  Failed to open /proc/kcore. Note /proc/kcore requires CAP_SYS_RAWIO 
capability to access.
  Using /proc/kallsyms for symbols
  Failed to open /sbin/adbd, continuing without symbols
  #
  # Total Lost Samples: 0
  #
  # Samples: 304  of event 'cycles'
  # Event count (approx.): 42027298
  #
  # Overhead  Socket  CPU
  # ........  ......  ...
  #
      60.25%    -001  004
      32.69%    -001  007
       5.00%    -001  005
       1.34%    -001  003
       0.37%    -001  000
       0.20%    -001  006
       0.08%    -001  002
       0.07%    -001  001

And if CPU is offlined during perf record (between build_cpu_topology() 
and perf_env__read_cpu_topology_map()):

  # ./perf report -v  --stdio -s socket,cpu
  build id event received for [kernel.kallsyms]: ...
  core_id number is too big.You may need to upgrade the perf tool. <--- 
*please see this line*
  # To display the perf.data header info, please use 
--header/--header-only options.
  #
  symsrc__init: cannot get elf header.
  Looking at the vmlinux_path (7 entries long)
  Failed to open /proc/kcore. Note /proc/kcore requires CAP_SYS_RAWIO 
capability to access.
  Using /proc/kallsyms for symbols
  #
  # Total Lost Samples: 0
  #
  # Samples: 278  of event 'cycles'
  # Event count (approx.): 37729429
  #
  # Overhead  Socket  CPU
  # ........  ......  ...
  #
      59.06%    -001  004
      36.93%    -001  007
       1.33%    -001  000
       1.18%    -001  005
       0.82%    -001  006
       0.50%    -001  001
       0.08%    -001  002
       0.08%    -001  003



  reply	other threads:[~2015-09-11 11:51 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-09 19:50 [RFC 00/13] perf_env/CPU socket reorg/fixes Arnaldo Carvalho de Melo
2015-09-09 19:50 ` [PATCH 01/13] perf env: Move perf_env out of header.h and session.c into separate object Arnaldo Carvalho de Melo
2015-09-09 19:50 ` [PATCH 02/13] perf env: Rename some leftovers from rename to perf_env Arnaldo Carvalho de Melo
2015-09-09 19:50 ` [PATCH 03/13] perf env: Adopt perf_header__set_cmdline Arnaldo Carvalho de Melo
2015-09-09 19:50 ` [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method Arnaldo Carvalho de Melo
2015-09-09 21:41   ` Liang, Kan
2015-09-10 13:12     ` Arnaldo Carvalho de Melo
2015-09-10 20:00       ` Liang, Kan
2015-09-10 20:12         ` Arnaldo Carvalho de Melo
2015-09-10 20:14           ` Liang, Kan
2015-09-11 10:20       ` Wangnan (F)
2015-09-11 14:40         ` Arnaldo Carvalho de Melo
2015-09-11 15:33           ` Arnaldo Carvalho de Melo
2015-09-11 16:14             ` Namhyung Kim
2015-09-11 16:32               ` Arnaldo Carvalho de Melo
2015-09-14  9:09             ` [tip:perf/urgent] perf header: Fixup reading of HEADER_NRCPUS feature tip-bot for Arnaldo Carvalho de Melo
2015-09-15  7:04   ` [tip:perf/core] perf env: Introduce read_cpu_topology_map() method tip-bot for Arnaldo Carvalho de Melo
2015-09-09 19:50 ` [PATCH 05/13] perf sort: Set flag stating if the "socket" key is being used Arnaldo Carvalho de Melo
2015-09-09 19:50 ` [PATCH 06/13] perf top: Cache the cpu topology info when "-s socket" is used Arnaldo Carvalho de Melo
2015-09-09 19:50 ` [PATCH 07/13] perf hists browser: Fixup the "cpu" column width calculation Arnaldo Carvalho de Melo
2015-09-11 10:52   ` Wangnan (F)
2015-09-09 19:50 ` [PATCH 08/13] perf machine: Add pointer to sample's environment Arnaldo Carvalho de Melo
2015-09-09 19:50 ` [PATCH 09/13] perf event: Use machine->env to find the cpu -> socket mapping Arnaldo Carvalho de Melo
2015-09-09 19:50 ` [PATCH 10/13] perf report: Do not blindly use env->cpu[al.cpu].socket_id Arnaldo Carvalho de Melo
2015-09-11 11:50   ` Wangnan (F) [this message]
2015-09-09 19:50 ` [PATCH 11/13] Revert "perf evsel: Add a backpointer to the evlist a evsel is in" Arnaldo Carvalho de Melo
2015-09-09 21:42   ` Liang, Kan
2015-09-09 19:50 ` [PATCH 12/13] perf evsel: Remove forward declaration of 'struct perf_evlist' Arnaldo Carvalho de Melo
2015-09-09 19:50 ` [PATCH 13/13] Revert "perf evlist: Add backpointer for perf_env to evlist" Arnaldo Carvalho de Melo
2015-09-10  9:19 ` [RFC 00/13] perf_env/CPU socket reorg/fixes Jiri Olsa
2015-09-10 14:13   ` Arnaldo Carvalho de Melo
2015-09-11 12:20 ` Wangnan (F)
2015-09-11 13:03   ` Arnaldo Carvalho de Melo
2015-09-11 13:29     ` Arnaldo Carvalho de Melo
2015-09-11 13:30       ` Arnaldo Carvalho de Melo
2015-09-11 13:36         ` Arnaldo Carvalho de Melo
2015-09-14  1:37           ` Wangnan (F)
2015-09-14  1:26     ` Wangnan (F)

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=55F2C01E.5020701@huawei.com \
    --to=wangnan0@huawei.com \
    --cc=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=adrian.hunter@intel.com \
    --cc=bp@suse.de \
    --cc=dsahern@gmail.com \
    --cc=eranian@google.com \
    --cc=fweisbec@gmail.com \
    --cc=kan.liang@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    /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.