guest_kallsyms is redundant here, remove it.
Signed-off-by: Gui Jianfeng <[email protected]>
---
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
group_fd related code is dead here, remove them.
Signed-off-by: Gui Jianfeng <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> > ---
> > 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
Commit-ID: 830f4c803196eec181e209110885c4ac130f3805
Gitweb: http://git.kernel.org/tip/830f4c803196eec181e209110885c4ac130f3805
Author: Gui Jianfeng <[email protected]>
AuthorDate: Fri, 25 Jun 2010 10:15:28 +0800
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Yanmin Zhang <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Gui Jianfeng <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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.
On Fri, 25 Jun 2010 16:48:05 +0200, Peter Zijlstra <[email protected]> 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.
Em Sat, Jun 26, 2010 at 04:14:46PM +0100, Matt Fleming escreveu:
> On Fri, 25 Jun 2010 16:48:05 +0200, Peter Zijlstra <[email protected]> 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
On Sat, 26 Jun 2010 16:14:46 +0100, Matt Fleming <[email protected]> wrote:
> On Fri, 25 Jun 2010 16:48:05 +0200, Peter Zijlstra <[email protected]> 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?
On Sun, 2010-07-18 at 21:06 +0100, Matt Fleming wrote:
> On Sat, 26 Jun 2010 16:14:46 +0100, Matt Fleming <[email protected]> wrote:
> > On Fri, 25 Jun 2010 16:48:05 +0200, Peter Zijlstra <[email protected]> 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
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?