Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1216330

RE: [PATCH V2 1/2] perf,tools: store cpu socket_id and core_id in perf.date

From "Liang, Kan" <kan.liang@intel.com>
Newsgroups linux.kernel
Subject RE: [PATCH V2 1/2] perf,tools: store cpu socket_id and core_id in perf.date
Date 2015-08-31 19:20 +0200
Message-ID <q3AUp-5Ol-7@gated-at.bofh.it> (permalink)
References <q2yUG-60W-9@gated-at.bofh.it> <q2LRT-7Ou-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw



> On Fri, Aug 28, 2015 at 09:43:38AM -0400, Kan Liang wrote:
> > From: Kan Liang <kan.liang@intel.com>
> >
> > This patch stores cpu socket_id and core_id in perf.date, and read
> > them to perf_env in header process.
> >
> > Signed-off-by: Kan Liang <kan.liang@intel.com>
> > ---
> >
> > Changes since V1:
> >  - Store core_id and socket_id in perf.date
> >
> >  tools/perf/util/header.c  | 97
> > +++++++++++++++++++++++++++++++++++++++++++++--
> >  tools/perf/util/header.h  |  6 +++
> >  tools/perf/util/session.c |  1 +
> >  3 files changed, 100 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c index
> > 4181454..482749f 100644
> > --- a/tools/perf/util/header.c
> > +++ b/tools/perf/util/header.c
> > @@ -439,14 +439,42 @@ static int write_cmdline(int fd, struct
> perf_header *h __maybe_unused,
> >  	"/sys/devices/system/cpu/cpu%d/topology/core_siblings_list"
> >  #define THRD_SIB_FMT \
> >  	"/sys/devices/system/cpu/cpu%d/topology/thread_siblings_list"
> > +#define CORE_ID_FMT \
> > +	"/sys/devices/system/cpu/cpu%d/topology/core_id"
> > +#define PHY_PKG_ID_FMT \
> > +	"/sys/devices/system/cpu/cpu%d/topology/physical_package_id"
> 
> hardcoded /sys
> 
> >
> >  struct cpu_topo {
> > +	u32 cpu_nr;
> >  	u32 core_sib;
> >  	u32 thread_sib;
> >  	char **core_siblings;
> >  	char **thread_siblings;
> > +	int *core_id;
> > +	int *phy_pkg_id;
> >  };
> >
> > +static int read_id(const char *path, int cpu) {
> > +	FILE *fp;
> > +	char filename[MAXPATHLEN];
> > +	char *buf = NULL;
> > +	size_t len = 0;
> > +	int ret = -1;
> > +
> > +	sprintf(filename, path, cpu);
> > +	fp = fopen(filename, "r");
> > +	if (fp == NULL)
> > +		return ret;
> > +
> > +	if (getline(&buf, &len, fp) > 0)
> > +		ret = atoi(buf);
> > +
> > +	fclose(fp);
> > +	free(buf);
> > +	return ret;
> > +}
> > +
> >  static int build_cpu_topo(struct cpu_topo *tp, int cpu)  {
> >  	FILE *fp;
> > @@ -507,6 +535,9 @@ try_threads:
> >  	}
> >  	ret = 0;
> >  done:
> > +	tp->core_id[cpu] = read_id(CORE_ID_FMT, cpu);
> > +	tp->phy_pkg_id[cpu] = read_id(PHY_PKG_ID_FMT, cpu);
> 
> hu,m dont we read this already somehow in cpumap.c ?

Yes, but we need to modify the functions in cpumap.c a little bit.
I will write a new patch for it in next version.

> 
> > +
> >  	if(fp)
> >  		fclose(fp);
> >  	free(buf);
> > @@ -534,7 +565,7 @@ static struct cpu_topo *build_cpu_topology(void)
> >  	struct cpu_topo *tp;
> >  	void *addr;
> >  	u32 nr, i;
> > -	size_t sz;
> > +	size_t sz, sz_id;
> >  	long ncpus;
> >  	int ret = -1;
> >
> 
> SNIP
> 
> > @@ -1631,10 +1686,44 @@ static int process_cpu_topology(struct
> perf_file_section *section __maybe_unused
> >  		free(str);
> >  	}
> >  	ph->env.sibling_threads = strbuf_detach(&sb, NULL);
> > +
> > +	for (i = 0; i < (u32)cpu_nr; i++) {
> > +		ret = readn(fd, &nr, sizeof(nr));
> > +		if (ret != sizeof(nr))
> > +			goto free_cpu;
> > +
> > +		if (ph->needs_swap)
> > +			nr = bswap_32(nr);
> > +
> > +		if (nr > (u32)cpu_nr) {
> > +			pr_debug("core_id number is too big."
> > +				 "You may need to upgrade the perf
> tool.\n");
> > +			goto free_cpu;
> > +		}
> > +		ph->env.cpu[i].core_id = nr;
> > +
> > +		ret = readn(fd, &nr, sizeof(nr));
> > +		if (ret != sizeof(nr))
> > +			goto free_cpu;
> > +
> > +		if (ph->needs_swap)
> > +			nr = bswap_32(nr);
> > +
> > +		if (nr > (u32)cpu_nr) {
> > +			pr_debug("socket_id number is too big."
> > +				 "You may need to upgrade the perf
> tool.\n");
> > +			goto free_cpu;
> 
> we should keep some degree of backward compatibility, you changed the
> CPU_TOPOLOGY feature.. are we still able read older perf.data? can old
> perf read new perf.data?
> 
The old perf can definitely read new perf.data, because the new codes are
added at the end of the section. It can ignore the extra data.
The new perf can read part of the old perf.data. It's because it will error
out  when core_id and socket_id are not detected. So the following sections
will not be read. But we have message for them to let them upgrade
the perf tool. It should be enough.


> this needs to be explained in some comment, or if it's breakage we need
> separate feature or event
>
OK. I will describe it in the changelog.
 
Thanks,
Kan
--
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 | NextPrevious in thread | Find similar | Unroll thread


Thread

[PATCH V2 1/2] perf,tools: store cpu socket_id and core_id in perf.date Kan Liang <kan.liang@intel.com> - 2015-08-28 23:00 +0200
  [PATCH V2 2/2] perf,test: test cpu topology Kan Liang <kan.liang@intel.com> - 2015-08-28 23:10 +0200
    Re: [PATCH V2 2/2] perf,test: test cpu topology Jiri Olsa <jolsa@redhat.com> - 2015-08-29 12:50 +0200
  Re: [PATCH V2 1/2] perf,tools: store cpu socket_id and core_id in  perf.date Jiri Olsa <jolsa@redhat.com> - 2015-08-29 12:50 +0200
    RE: [PATCH V2 1/2] perf,tools: store cpu socket_id and core_id in  perf.date "Liang, Kan" <kan.liang@intel.com> - 2015-08-31 19:20 +0200

csiph-web