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 <[email protected]> > 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 <[email protected]> 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
