Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1222939
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method |
| Date | 2015-09-11 18:20 +0200 |
| Message-ID | <q7zdo-6M4-13@gated-at.bofh.it> (permalink) |
| References | <q7xOi-4C9-19@gated-at.bofh.it> <q7yAF-5N1-17@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
Hi Arnaldo,
On Fri, Sep 11, 2015 at 12:33:23PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Sep 11, 2015 at 11:40:28AM -0300, Arnaldo Carvalho de Melo escreveu:
> > So this is a problem before and after my patches, i.e. If I go on and
> > do, with what we have in acme/perf/core, i.e. none of the changes I'm
> > playing with in perf/env:
> >
> > $ git remote update acme
> > Fetching acme
> > $ git checkout -b tmp acme/perf/core
> > Branch tmp set up to track remote branch perf/core from acme.
> > Switched to a new branch 'tmp'
> > $ git log --oneline | head -5
> > 7e150fb33a91 perf tests: Introduce iterator function for tests
> > 1765d9b26f84 perf test: Add entry for hists socket filter
> > 207bb55e9193 perf hists browser: Zoom in/out for processor socket
> > 9a2843a5f421 perf report: Introduce --socket-filter option
> > 99851c76436a perf tools: Introduce new sort type "socket" for the processor socket
> > $ rm -rf /tmp/build/perf ; mkdir -p /tmp/build/perf ; make DEBUG=1 -C tools/perf O=/tmp/build/perf install-bin
> > # echo 0 > /sys/devices/system/cpu/cpu2/online
> > $ cat /sys/devices/system/cpu/cpu2/topology/core_id
> > cat: /sys/devices/system/cpu/cpu2/topology/core_id: No such file or directory
> > $ ls -la /sys/devices/system/cpu/cpu2/topology/
> > ls: cannot access /sys/devices/system/cpu/cpu2/topology/: No such file or directory
> > $ perf record usleep 1
> > [ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 0.012 MB perf.data (7 samples) ]
> > [acme@felicio linux]$ perf report --header-only -I
> > # ========
> > # captured on: Fri Sep 11 11:34:18 2015
> > # hostname : felicio.ghostprotocols.net
> > # os release : 4.2.0
> > # perf version : 4.2.g7e150f
> > # arch : x86_64
> > # nrcpus online : 4
> > # nrcpus avail : 3
> > # cpudesc : Intel(R) Core(TM) i5-2400 CPU @ 3.10GHz
> > <SNIP>
> > # node0 meminfo : total = 8085412 kB, free = 5317596 kB
> > # node0 cpu list : 0-1,3
> > <SNIP>
> > $
>
> Stephane, Namhyung, the bug report below is about the online/avail
> above, they are swapped, full explanation below.
>
> > We can see multiple bugs here, right? online/avail is swapped, and when
> > online != avail we simply do not record the cpu topology info at all!
>
> > If we get that CPU back online:
> >
> > # echo 1 > /sys/devices/system/cpu/cpu2/online
> >
> > Then all works:
> >
> > $ perf record usleep 1
> > [ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 0.012 MB perf.data (7 samples) ]
> > $ perf report --header-only -I
> > # ========
> > # captured on: Fri Sep 11 11:37:31 2015
> > # hostname : felicio.ghostprotocols.net
> > # os release : 4.2.0
> > # perf version : 4.2.g7e150f
> > # arch : x86_64
> > # nrcpus online : 4
> > # nrcpus avail : 4
> > # cpudesc : Intel(R) Core(TM) i5-2400 CPU @ 3.10GHz
> > <SNIP>
> > # sibling cores : 0-3
> > # sibling threads : 0
> > # sibling threads : 1
> > # sibling threads : 2
> > # sibling threads : 3
> > # CPU 0: Core ID 0, Socket ID 0
> > # CPU 1: Core ID 1, Socket ID 0
> > # CPU 2: Core ID 2, Socket ID 0
> > # CPU 3: Core ID 3, Socket ID 0
> > # node0 meminfo : total = 8085412 kB, free = 5316992 kB
> > # node0 cpu list : 0-3
> >
> > So, again, this was not introduced by this patchkit, but it is good that you did
> > these offline tests, so we can fix it!
>
> Stephane, this was introduced in:
>
> commit fbe96f29ce4b33e0a22219cc7f5996d9157717e3
> Author: Stephane Eranian <eranian@google.com>
> Date: Fri Sep 30 15:40:40 2011 +0200
>
> perf tools: Make perf.data more self-descriptive (v8)
>
> ------------------
>
> When you write this part:
>
> - HEADER_NRCPUS: number of online/avail cpus
>
> You do:
>
> static int write_nrcpus(int fd, struct perf_header *h __used,
> struct perf_evlist *evlist __used)
> {
> long nr;
> u32 nrc, nra;
> int ret;
>
> nr = sysconf(_SC_NPROCESSORS_CONF);
> if (nr < 0)
> return -1;
>
> nrc = (u32)(nr & UINT_MAX);
>
> nr = sysconf(_SC_NPROCESSORS_ONLN);
> if (nr < 0)
> return -1;
>
> nra = (u32)(nr & UINT_MAX);
>
> ret = do_write(fd, &nrc, sizeof(nrc));
> if (ret < 0)
> return ret;
>
> return do_write(fd, &nra, sizeof(nra));
> }
>
> I.e. write what you called 'nrc' using what is in SC_NRPROCESSORS_CONF, that in
> the documentation for glibc reads:
>
> ---------------------
>
> http://www.gnu.org/software/libc/manual/html_node/Processor-Resources.html
>
> sysconf (_SC_NPROCESSORS_CONF)
>
> which returns the number of processors the operating system configured. But it
> might be possible for the operating system to disable individual processors and
> so the call
>
> ---------------------
>
> Which menas "NR_AVAILABLE", right?
>
> But then you call a variable 'nra' which sounds like you think that what is in
> _SC_NPROCESSORS_ONLN is the "available" number of CPUs, which is confused a bit
> more by the glibc docs when refering to _SC_NPROCESSORS_ONLN:
>
> ---------------------
> sysconf (_SC_NPROCESSORS_ONLN)
>
> returns the number of processors which are currently online (i.e., available).
> ---------------------
>
> Then, when printing the number of CPUs encoded in the perf.data file by the
> above write_nrcpus() routine, you did:
>
> static void print_nrcpus(struct perf_header *ph, int fd, FILE *fp)
> {
> ssize_t ret;
> u32 nr;
>
> ret = read(fd, &nr, sizeof(nr));
> if (ret != (ssize_t)sizeof(nr))
> nr = -1; /* interpreted as error */
>
> if (ph->needs_swap)
> nr = bswap_32(nr);
>
> fprintf(fp, "# nrcpus online : %u\n", nr);
>
> ret = read(fd, &nr, sizeof(nr));
> if (ret != (ssize_t)sizeof(nr))
> nr = -1; /* interpreted as error */
>
> if (ph->needs_swap)
> nr = bswap_32(nr);
>
> fprintf(fp, "# nrcpus avail : %u\n", nr);
> }
>
> You inverted it, no?
>
> So, could you please check if the below patch can have your Acked-by?
> Namhyung?
Looks good to me.
Acked-by: Namhyung Kim <namhyung@kernel.org>
Thanks,
Namhyung
>
> Thanks,
>
> - Arnaldo
>
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 41814547da15..fce6634aebe2 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -1438,7 +1438,7 @@ static int process_nrcpus(struct perf_file_section *section __maybe_unused,
> if (ph->needs_swap)
> nr = bswap_32(nr);
>
> - ph->env.nr_cpus_online = nr;
> + ph->env.nr_cpus_avail = nr;
>
> ret = readn(fd, &nr, sizeof(nr));
> if (ret != sizeof(nr))
> @@ -1447,7 +1447,7 @@ static int process_nrcpus(struct perf_file_section *section __maybe_unused,
> if (ph->needs_swap)
> nr = bswap_32(nr);
>
> - ph->env.nr_cpus_avail = nr;
> + ph->env.nr_cpus_online = nr;
> return 0;
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[RFC 00/13] perf_env/CPU socket reorg/fixes Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
[PATCH 04/13] perf env: Introduce read_cpu_topology_map() method Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
RE: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method "Liang, Kan" <kan.liang@intel.com> - 2015-09-09 23:50 +0200
Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-10 15:20 +0200
RE: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method "Liang, Kan" <kan.liang@intel.com> - 2015-09-10 22:10 +0200
RE: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method "Liang, Kan" <kan.liang@intel.com> - 2015-09-10 22:20 +0200
Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-10 22:20 +0200
Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method "Wangnan (F)" <wangnan0@huawei.com> - 2015-09-11 12:30 +0200
Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-11 16:50 +0200
Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-11 17:40 +0200
Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method Namhyung Kim <namhyung@kernel.org> - 2015-09-11 18:20 +0200
Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-11 18:40 +0200
[tip:perf/urgent] perf header: Fixup reading of HEADER_NRCPUS feature tip-bot for Arnaldo Carvalho de Melo <tipbot@zytor.com> - 2015-09-14 11:20 +0200
[tip:perf/core] perf env: Introduce read_cpu_topology_map() method tip-bot for Arnaldo Carvalho de Melo <tipbot@zytor.com> - 2015-09-15 09:10 +0200
[PATCH 07/13] perf hists browser: Fixup the "cpu" column width calculation Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
Re: [PATCH 07/13] perf hists browser: Fixup the "cpu" column width calculation "Wangnan (F)" <wangnan0@huawei.com> - 2015-09-11 13:00 +0200
[PATCH 12/13] perf evsel: Remove forward declaration of 'struct perf_evlist' Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
[PATCH 02/13] perf env: Rename some leftovers from rename to perf_env Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
[PATCH 01/13] perf env: Move perf_env out of header.h and session.c into separate object Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
[PATCH 13/13] Revert "perf evlist: Add backpointer for perf_env to evlist" Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
[PATCH 08/13] perf machine: Add pointer to sample's environment Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
[PATCH 06/13] perf top: Cache the cpu topology info when "-s socket" is used Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
[PATCH 03/13] perf env: Adopt perf_header__set_cmdline Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
[PATCH 09/13] perf event: Use machine->env to find the cpu -> socket mapping Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
[PATCH 10/13] perf report: Do not blindly use env->cpu[al.cpu].socket_id Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
Re: [PATCH 10/13] perf report: Do not blindly use env->cpu[al.cpu].socket_id "Wangnan (F)" <wangnan0@huawei.com> - 2015-09-11 14:00 +0200
Re: [RFC 00/13] perf_env/CPU socket reorg/fixes Jiri Olsa <jolsa@redhat.com> - 2015-09-10 11:20 +0200
Re: [RFC 00/13] perf_env/CPU socket reorg/fixes Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-10 16:20 +0200
Re: [RFC 00/13] perf_env/CPU socket reorg/fixes "Wangnan (F)" <wangnan0@huawei.com> - 2015-09-11 14:30 +0200
Re: [RFC 00/13] perf_env/CPU socket reorg/fixes Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-11 15:10 +0200
Re: [RFC 00/13] perf_env/CPU socket reorg/fixes Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-11 15:30 +0200
Re: [RFC 00/13] perf_env/CPU socket reorg/fixes Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-11 15:40 +0200
Re: [RFC 00/13] perf_env/CPU socket reorg/fixes Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-11 15:40 +0200
Re: [RFC 00/13] perf_env/CPU socket reorg/fixes "Wangnan (F)" <wangnan0@huawei.com> - 2015-09-14 03:40 +0200
Re: [RFC 00/13] perf_env/CPU socket reorg/fixes "Wangnan (F)" <wangnan0@huawei.com> - 2015-09-14 03:30 +0200
csiph-web