public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Jan Stancek <jstancek@redhat.com>
Cc: linux-kernel@vger.kernel.org, peterz@infradead.org,
	mingo@redhat.com, acme@kernel.org,
	alexander shishkin <alexander.shishkin@linux.intel.com>,
	jolsa@kernel.org, mhiramat@kernel.org,
	rui teng <rui.teng@linux.vnet.ibm.com>,
	sukadev@linux.vnet.ibm.com
Subject: Re: [PATCH] perf: fix topology test on systems with sparse CPUs
Date: Thu, 2 Feb 2017 14:01:00 +0100	[thread overview]
Message-ID: <20170202130100.GG2305@krava> (raw)
In-Reply-To: <1953280547.2065342.1486037203674.JavaMail.zimbra@redhat.com>

On Thu, Feb 02, 2017 at 07:06:43AM -0500, Jan Stancek wrote:
> > 
> > > When build_cpu_topo() encounters offline/absent CPUs,
> > > it fails to find any sysfs entries and returns failure.
> > > This leads to build_cpu_topology() and write_cpu_topology()
> > > failing as well.
> > > 
> > > Because HEADER_CPU_TOPOLOGY has not been written, read leaves
> > > cpu_topology_map NULL and we get NULL ptr deref at:
> > > 
> > >  ...
> > >   cmd_test
> > >    __cmd_test
> > >     test_and_print
> > >      run_test
> > >       test_session_topology
> > >        check_cpu_topology
> > 
> > So IIUIC that's the key issue here.. write_cpu_topology that fails
> > to write the TOPO data and following readers crashing on processing
> > uncomplete data? if thats the case write_cpu_topology needs to
> > be fixed, instead of doing workarounds
> 
> It's already late when you are in write_cpu_topology(), because
> build_cpu_topology() returned you NULL - there's nothing to write.
> That's why patch aims to fix this in build_cpu_topology().

ok, then we need to make sure we can't fail in write_cpu_topology

might be another patch scope though.. we can go with your fix so far

SNIP

> 
> > For example:
> >   _SC_NPROCESSORS_CONF == 16
> >   available: 2 nodes (0-1)
> >   node 0 cpus: 0 6 8 10 16 22 24 26
> >   node 0 size: 12004 MB
> >   node 0 free: 9470 MB
> >   node 1 cpus: 1 7 9 11 23 25 27
> >   node 1 size: 12093 MB
> >   node 1 free: 9406 MB
> >   node distances:
> >   node   0   1
> >     0:  10  20
> >     1:  20  10
> > so what's max_present_cpu in this example?
> 
> It's 28, which is the number of core_id/socket_id entries,
> for CPUs 0 up to 27.

ok, good

jirka

  reply	other threads:[~2017-02-02 13:01 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-30 16:53 [PATCH] perf: fix topology test on systems with sparse CPUs Jan Stancek
2017-01-30 18:49 ` Jiri Olsa
2017-01-30 19:29   ` Jan Stancek
2017-01-31 16:03   ` Jan Stancek
2017-02-02 11:29     ` Jiri Olsa
2017-02-02 12:06       ` Jan Stancek
2017-02-02 13:01         ` Jiri Olsa [this message]
2017-02-13 15:34           ` [PATCH v2 1/3] perf: add cpu__max_present_cpu() Jan Stancek
2017-02-13 15:34             ` [PATCH v2 2/3] perf: make build_cpu_topology skip offline/absent CPUs Jan Stancek
2017-02-14 11:01               ` Jiri Olsa
2017-02-15  8:48                 ` Jan Stancek
2017-02-17 11:10                   ` [PATCH v3 1/3] perf: add cpu__max_present_cpu() Jan Stancek
2017-02-17 11:10                     ` [PATCH v3 2/3] perf: make build_cpu_topology skip offline/absent CPUs Jan Stancek
2017-02-17 15:36                       ` Arnaldo Carvalho de Melo
2017-02-21  8:12                       ` [tip:perf/urgent] perf header: Make " tip-bot for Jan Stancek
2017-02-17 11:10                     ` [PATCH v3 3/3] perf: replace _SC_NPROCESSORS_CONF with max_present_cpu in cpu_topology_map Jan Stancek
2017-02-17 15:05                       ` Jiri Olsa
2017-02-21  8:12                       ` [tip:perf/urgent] perf tools: Replace " tip-bot for Jan Stancek
2017-02-21  8:11                     ` [tip:perf/urgent] perf cpumap: Add cpu__max_present_cpu() tip-bot for Jan Stancek
2017-02-13 15:34             ` [PATCH v2 3/3] perf: replace _SC_NPROCESSORS_CONF with max_present_cpu in cpu_topology_map Jan Stancek
2017-02-14 11:17             ` [PATCH v2 1/3] perf: add cpu__max_present_cpu() Jiri Olsa
2017-02-02 11:29     ` [PATCH] perf: fix topology test on systems with sparse CPUs Jiri Olsa
2017-01-30 18:49 ` Jiri Olsa
2017-01-30 18:49 ` Jiri Olsa

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=20170202130100.GG2305@krava \
    --to=jolsa@redhat.com \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=jolsa@kernel.org \
    --cc=jstancek@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rui.teng@linux.vnet.ibm.com \
    --cc=sukadev@linux.vnet.ibm.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