public inbox for linux-kernel@vger.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox