[PATCH 1/3] perf kvm: Get rid of unused guest_kallsyms

[PATCH 1/3] perf kvm: Get rid of unused guest_kallsyms

[Gui Jianfeng]

guest_kallsyms is redundant here, remove it.

Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
---
 tools/perf/builtin-record.c |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index dc3435e..abaf0f8 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -439,8 +439,6 @@ static void atexit_header(void)
 static void event__synthesize_guest_os(struct machine *machine, void *data)
 {
 	int err;
-	char *guest_kallsyms;
-	char path[PATH_MAX];
 	struct perf_session *psession = data;
 
 	if (machine__is_host(machine))
@@ -460,13 +458,6 @@ static void event__synthesize_guest_os(struct machine *machine, void *data)
 		pr_err("Couldn't record guest kernel [%d]'s reference"
 		       " relocation symbol.\n", machine->pid);
 
-	if (machine__is_default_guest(machine))
-		guest_kallsyms = (char *) symbol_conf.default_guest_kallsyms;
-	else {
-		sprintf(path, "%s/proc/kallsyms", machine->root_dir);
-		guest_kallsyms = path;
-	}
-
 	/*
 	 * We use _stext for guest kernel because guest kernel's /proc/kallsyms
 	 * have no _text sometimes.
-- 
1.6.5.2

[PATCH 2/3] perf: Remove dead code in buildin-record.c

[Gui Jianfeng]

group_fd related code is dead here, remove them.

Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
---
 tools/perf/builtin-record.c |   12 +-----------
 1 files changed, 1 insertions(+), 11 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index abaf0f8..84d58b7 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -45,7 +45,6 @@ static int			freq				=   1000;
 static int			output;
 static int			pipe_output			=      0;
 static const char		*output_name			= "perf.data";
-static int			group				=      0;
 static int			realtime_prio			=      0;
 static bool			raw_samples			=  false;
 static bool			system_wide			=  false;
@@ -203,8 +202,6 @@ static void sig_atexit(void)
 	kill(getpid(), signr);
 }
 
-static int group_fd;
-
 static struct perf_header_attr *get_header_attr(struct perf_event_attr *a, int nr)
 {
 	struct perf_header_attr *h_attr;
@@ -291,7 +288,7 @@ static void create_counter(int counter, int cpu)
 	for (thread_index = 0; thread_index < thread_num; thread_index++) {
 try_again:
 		fd[nr_cpu][counter][thread_index] = sys_perf_event_open(attr,
-				all_tids[thread_index], cpu, group_fd, 0);
+				all_tids[thread_index], cpu, -1, 0);
 
 		if (fd[nr_cpu][counter][thread_index] < 0) {
 			int err = errno;
@@ -359,12 +356,6 @@ try_again:
 		assert(fd[nr_cpu][counter][thread_index] >= 0);
 		fcntl(fd[nr_cpu][counter][thread_index], F_SETFL, O_NONBLOCK);
 
-		/*
-		 * First counter acts as the group leader:
-		 */
-		if (group && group_fd == -1)
-			group_fd = fd[nr_cpu][counter][thread_index];
-
 		if (counter || thread_index) {
 			ret = ioctl(fd[nr_cpu][counter][thread_index],
 					PERF_EVENT_IOC_SET_OUTPUT,
@@ -406,7 +397,6 @@ static void open_counters(int cpu)
 {
 	int counter;
 
-	group_fd = -1;
 	for (counter = 0; counter < nr_counters; counter++)
 		create_counter(counter, cpu);
 
-- 1.6.5.2 

Re: [PATCH 2/3] perf: Remove dead code in buildin-record.c

[Arnaldo Carvalho de Melo]

Em Fri, Jun 25, 2010 at 10:16:59AM +0800, Gui Jianfeng escreveu:
> group_fd related code is dead here, remove them.

I agree it is dead at the moment, but I think Peter (or was it Ingo at
the time?) had a reason for it to live, Peter, care to take a look?

- Arnaldo
 
> Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
> ---
>  tools/perf/builtin-record.c |   12 +-----------
>  1 files changed, 1 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index abaf0f8..84d58b7 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -45,7 +45,6 @@ static int			freq				=   1000;
>  static int			output;
>  static int			pipe_output			=      0;
>  static const char		*output_name			= "perf.data";
> -static int			group				=      0;
>  static int			realtime_prio			=      0;
>  static bool			raw_samples			=  false;
>  static bool			system_wide			=  false;
> @@ -203,8 +202,6 @@ static void sig_atexit(void)
>  	kill(getpid(), signr);
>  }
>  
> -static int group_fd;
> -
>  static struct perf_header_attr *get_header_attr(struct perf_event_attr *a, int nr)
>  {
>  	struct perf_header_attr *h_attr;
> @@ -291,7 +288,7 @@ static void create_counter(int counter, int cpu)
>  	for (thread_index = 0; thread_index < thread_num; thread_index++) {
>  try_again:
>  		fd[nr_cpu][counter][thread_index] = sys_perf_event_open(attr,
> -				all_tids[thread_index], cpu, group_fd, 0);
> +				all_tids[thread_index], cpu, -1, 0);
>  
>  		if (fd[nr_cpu][counter][thread_index] < 0) {
>  			int err = errno;
> @@ -359,12 +356,6 @@ try_again:
>  		assert(fd[nr_cpu][counter][thread_index] >= 0);
>  		fcntl(fd[nr_cpu][counter][thread_index], F_SETFL, O_NONBLOCK);
>  
> -		/*
> -		 * First counter acts as the group leader:
> -		 */
> -		if (group && group_fd == -1)
> -			group_fd = fd[nr_cpu][counter][thread_index];
> -
>  		if (counter || thread_index) {
>  			ret = ioctl(fd[nr_cpu][counter][thread_index],
>  					PERF_EVENT_IOC_SET_OUTPUT,
> @@ -406,7 +397,6 @@ static void open_counters(int cpu)
>  {
>  	int counter;
>  
> -	group_fd = -1;
>  	for (counter = 0; counter < nr_counters; counter++)
>  		create_counter(counter, cpu);
>  
> -- 1.6.5.2 

Re: [PATCH 2/3] perf: Remove dead code in buildin-record.c

[Peter Zijlstra]

On Fri, 2010-06-25 at 10:13 -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jun 25, 2010 at 10:16:59AM +0800, Gui Jianfeng escreveu:
> > group_fd related code is dead here, remove them.
> 
> I agree it is dead at the moment, but I think Peter (or was it Ingo at
> the time?) had a reason for it to live, Peter, care to take a look?

Ah, yes, it would be nice for SH (which doesn't have a PMI) to couple a
software timer with their hardware counter to get samples.

Never got around to actually implementing that though, maybe Paul has a
minion interested in making that work?

> > Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
> > ---
> >  tools/perf/builtin-record.c |   12 +-----------
> >  1 files changed, 1 insertions(+), 11 deletions(-)
> > 
> > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > index abaf0f8..84d58b7 100644
> > --- a/tools/perf/builtin-record.c
> > +++ b/tools/perf/builtin-record.c
> > @@ -45,7 +45,6 @@ static int			freq				=   1000;
> >  static int			output;
> >  static int			pipe_output			=      0;
> >  static const char		*output_name			= "perf.data";
> > -static int			group				=      0;
> >  static int			realtime_prio			=      0;
> >  static bool			raw_samples			=  false;
> >  static bool			system_wide			=  false;
> > @@ -203,8 +202,6 @@ static void sig_atexit(void)
> >  	kill(getpid(), signr);
> >  }
> >  
> > -static int group_fd;
> > -
> >  static struct perf_header_attr *get_header_attr(struct perf_event_attr *a, int nr)
> >  {
> >  	struct perf_header_attr *h_attr;
> > @@ -291,7 +288,7 @@ static void create_counter(int counter, int cpu)
> >  	for (thread_index = 0; thread_index < thread_num; thread_index++) {
> >  try_again:
> >  		fd[nr_cpu][counter][thread_index] = sys_perf_event_open(attr,
> > -				all_tids[thread_index], cpu, group_fd, 0);
> > +				all_tids[thread_index], cpu, -1, 0);
> >  
> >  		if (fd[nr_cpu][counter][thread_index] < 0) {
> >  			int err = errno;
> > @@ -359,12 +356,6 @@ try_again:
> >  		assert(fd[nr_cpu][counter][thread_index] >= 0);
> >  		fcntl(fd[nr_cpu][counter][thread_index], F_SETFL, O_NONBLOCK);
> >  
> > -		/*
> > -		 * First counter acts as the group leader:
> > -		 */
> > -		if (group && group_fd == -1)
> > -			group_fd = fd[nr_cpu][counter][thread_index];
> > -
> >  		if (counter || thread_index) {
> >  			ret = ioctl(fd[nr_cpu][counter][thread_index],
> >  					PERF_EVENT_IOC_SET_OUTPUT,
> > @@ -406,7 +397,6 @@ static void open_counters(int cpu)
> >  {
> >  	int counter;
> >  
> > -	group_fd = -1;
> >  	for (counter = 0; counter < nr_counters; counter++)
> >  		create_counter(counter, cpu);
> >  
> > -- 1.6.5.2 

Re: [PATCH 2/3] perf: Remove dead code in buildin-record.c

[Matt Fleming]

On Fri, 25 Jun 2010 16:48:05 +0200, Peter Zijlstra <peterz@infradead.org> wrote:
> 
> Ah, yes, it would be nice for SH (which doesn't have a PMI) to couple a
> software timer with their hardware counter to get samples.
> 
> Never got around to actually implementing that though, maybe Paul has a
> minion interested in making that work?

I'll take a look at it.

Re: [PATCH 2/3] perf: Remove dead code in buildin-record.c

[Arnaldo Carvalho de Melo]

Em Sat, Jun 26, 2010 at 04:14:46PM +0100, Matt Fleming escreveu:
> On Fri, 25 Jun 2010 16:48:05 +0200, Peter Zijlstra <peterz@infradead.org> wrote:
> > Ah, yes, it would be nice for SH (which doesn't have a PMI) to couple a
> > software timer with their hardware counter to get samples.
> > 
> > Never got around to actually implementing that though, maybe Paul has a
> > minion interested in making that work?
> 
> I'll take a look at it.

Thanks!

- Arnaldo

Re: [PATCH 2/3] perf: Remove dead code in buildin-record.c

[Matt Fleming]

On Sat, 26 Jun 2010 16:14:46 +0100, Matt Fleming <matt@console-pimps.org> wrote:
> On Fri, 25 Jun 2010 16:48:05 +0200, Peter Zijlstra <peterz@infradead.org> wrote:
> > 
> > Ah, yes, it would be nice for SH (which doesn't have a PMI) to couple a
> > software timer with their hardware counter to get samples.
> > 
> > Never got around to actually implementing that though, maybe Paul has a
> > minion interested in making that work?
> 
> I'll take a look at it.

How does the 'group_fd' parameter relate to the lack of PMI? Is the idea
to have one hrtimer that, when it fires, we sample all the counters? So
the first counter to be created is the group leader, which starts the
hrtimer, and all other counters are linked to this one? I had a go at
using a hrtimer per counter (minus any weighting of samples) and it
worked OK and seemed sensible given that we may want to sample counters
at different frequencies.

Is this what you had in mind with the 'group_fd' paramter, Peter? That
there'd be only one hrtimer?

Re: [PATCH 2/3] perf: Remove dead code in buildin-record.c

[Peter Zijlstra]

On Sun, 2010-07-18 at 21:06 +0100, Matt Fleming wrote:
> On Sat, 26 Jun 2010 16:14:46 +0100, Matt Fleming <matt@console-pimps.org> wrote:
> > On Fri, 25 Jun 2010 16:48:05 +0200, Peter Zijlstra <peterz@infradead.org> wrote:
> > > 
> > > Ah, yes, it would be nice for SH (which doesn't have a PMI) to couple a
> > > software timer with their hardware counter to get samples.
> > > 
> > > Never got around to actually implementing that though, maybe Paul has a
> > > minion interested in making that work?
> > 
> > I'll take a look at it.
> 
> How does the 'group_fd' parameter relate to the lack of PMI? Is the idea
> to have one hrtimer that, when it fires, we sample all the counters? So
> the first counter to be created is the group leader, which starts the
> hrtimer, and all other counters are linked to this one? I had a go at
> using a hrtimer per counter (minus any weighting of samples) and it
> worked OK and seemed sensible given that we may want to sample counters
> at different frequencies.
> 
> Is this what you had in mind with the 'group_fd' paramter, Peter? That
> there'd be only one hrtimer?

Right. So sys_perf_event_open() creates a stand alone event, but if you
supply the group_fd param it will attach the newly created one to the
leader indicated by group_fd.

Groups have the properly that they will always be scheduled together,
and read/sample can access/provide data of all of them.

So you can have a sampling leader report the counts of its siblings.

You can make multiple groups, like {hrtimer, cycles} and {hrtimer,
instructions} and {hrtimer, dcache-miss} and perf will schedule the
stuff. If you'd try to do {hrtimer, cycles, insns, dcache-miss} but only
have 2 hardware counters the group creation should fail because the
implementation should dis-allow creating groups that cannot be
scheduled 

Re: [PATCH 2/3] perf: Remove dead code in buildin-record.c

[Matt Fleming]

On Wed, Aug 04, 2010 at 12:56:06PM +0200, Peter Zijlstra wrote:
> On Sun, 2010-07-18 at 21:06 +0100, Matt Fleming wrote:
> > 
> > How does the 'group_fd' parameter relate to the lack of PMI? Is the idea
> > to have one hrtimer that, when it fires, we sample all the counters? So
> > the first counter to be created is the group leader, which starts the
> > hrtimer, and all other counters are linked to this one? I had a go at
> > using a hrtimer per counter (minus any weighting of samples) and it
> > worked OK and seemed sensible given that we may want to sample counters
> > at different frequencies.
> > 
> > Is this what you had in mind with the 'group_fd' paramter, Peter? That
> > there'd be only one hrtimer?
> 
> Right. So sys_perf_event_open() creates a stand alone event, but if you
> supply the group_fd param it will attach the newly created one to the
> leader indicated by group_fd.
> 
> Groups have the properly that they will always be scheduled together,
> and read/sample can access/provide data of all of them.

Ah, I think I see. The benefit of scheduling these events together is
that we can then sample the counters at roughly the same time? As
opposed to the case where we have multiple hrtimers firing and the
counter values will be from vastly different times? Yeah, that makes
sense now.

> So you can have a sampling leader report the counts of its siblings.
> 
> You can make multiple groups, like {hrtimer, cycles} and {hrtimer,
> instructions} and {hrtimer, dcache-miss} and perf will schedule the
> stuff. If you'd try to do {hrtimer, cycles, insns, dcache-miss} but only
> have 2 hardware counters the group creation should fail because the
> implementation should dis-allow creating groups that cannot be
> scheduled 

Right, so the hrtimer is an event all by itself: the parent event?

[PATCH 3/3] perf: Fix the bug which fails to put memory mapping to record file

[Gui Jianfeng]

If we execute perf record without -t or -p option, it fails to load
process memory map to record file. Here fix it.

Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
---
 tools/perf/builtin-record.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 84d58b7..f834d21 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -685,9 +685,19 @@ static int __cmd_record(int argc, const char **argv)
 	if (perf_guest)
 		perf_session__process_machines(session, event__synthesize_guest_os);
 
-	if (!system_wide && profile_cpu == -1)
-		event__synthesize_thread(target_tid, process_synthesized_event,
-					 session);
+	if (!system_wide && profile_cpu == -1) {
+		int ret;
+
+		if (target_tid != -1) {
+			ret = event__synthesize_thread(target_tid,
+						 process_synthesized_event,
+						 session);
+		} else {
+			ret = event__synthesize_thread(all_tids[0],
+						 process_synthesized_event,
+						 session);
+		}
+	}
 	else
 		event__synthesize_threads(process_synthesized_event, session);
 
-- 1.6.5.2 

[tip:perf/core] perf kvm: Get rid of unused guest_kallsyms

[tip-bot for Gui Jianfeng]

Commit-ID:  830f4c803196eec181e209110885c4ac130f3805
Gitweb:     http://git.kernel.org/tip/830f4c803196eec181e209110885c4ac130f3805
Author:     Gui Jianfeng <guijianfeng@cn.fujitsu.com>
AuthorDate: Fri, 25 Jun 2010 10:15:28 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 25 Jun 2010 07:28:21 -0300

perf kvm: Get rid of unused guest_kallsyms

guest_kallsyms is redundant here, remove it.

Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Yanmin Zhang <yanmin_zhang@linux.intel.com>
LKML-Reference: <4C241140.9090008@cn.fujitsu.com>
Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 86b1c3b..0df6408 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -445,8 +445,6 @@ static void atexit_header(void)
 static void event__synthesize_guest_os(struct machine *machine, void *data)
 {
 	int err;
-	char *guest_kallsyms;
-	char path[PATH_MAX];
 	struct perf_session *psession = data;
 
 	if (machine__is_host(machine))
@@ -466,13 +464,6 @@ static void event__synthesize_guest_os(struct machine *machine, void *data)
 		pr_err("Couldn't record guest kernel [%d]'s reference"
 		       " relocation symbol.\n", machine->pid);
 
-	if (machine__is_default_guest(machine))
-		guest_kallsyms = (char *) symbol_conf.default_guest_kallsyms;
-	else {
-		sprintf(path, "%s/proc/kallsyms", machine->root_dir);
-		guest_kallsyms = path;
-	}
-
 	/*
 	 * We use _stext for guest kernel because guest kernel's /proc/kallsyms
 	 * have no _text sometimes.