perf: fix broken perf inject -b

PATCH In review Stephane Eranian 3 participants 14 messages Jan 13, 2012 – Feb 3, 2012 perf events

Share & tools

Stephane Eranian

author

2012-01-13 14:47 UTC #0 lore raw

PATCH 1/1 perf: fix broken perf inject -b

perf inject -b was broken. It would not inject any build_ids into the stream. Furthermore, it would strip samples from the stream.

The reason was a missing initialization of the event attribute structures. The perf_tool.tool.attr() callback was pointing to a simple repipe. But there was no initialization of internal data structures to keep track of events and event ids. That later caused event id lookups to fail, and samples would get dropped.

The patch simply adds back the call to perf_event__process_attr() to initialize the evlist structure. Build_ids are again injected. The PID is also inject for PERF_RECORD_HEADER_BUILD_ID.

Signed-off-by Stephane Eranian <[email protected]>

---

tools/perf/builtin-inject.c +6 -0

index 09c1061..f38f370 100644
@@ -60,6 +60,11 @@ static int perf_event__repipe_tracing_data_synth(union perf_event *event,
 static int perf_event__repipe_attr(union perf_event *event,
 				   struct perf_evlist **pevlist __used)
 {
+	int ret;
+	ret = perf_event__process_attr(event, pevlist);
+	if (ret)
+		return ret;
+
 	return perf_event__repipe_synth(NULL, event, NULL);
 }

@@ -173,6 +178,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
 		       event->header.type);
 		goto repipe;
 	}
+	machine->pid = event->ip.pid;
 
 	thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION,
 			      event->ip.ip, &al);

Arnaldo Carvalho de Melo

2012-01-13 16:53 UTC #1 lore raw

Em Fri, Jan 13, 2012 at 03:47:31PM +0100, Stephane Eranian escreveu:

Quoted 4 lines

perf inject -b was broken. It would not inject any build_ids into the stream. Furthermore, it would strip samples from the stream.

Against which tree is this patch?

Quoted 37 lines

The reason was a missing initialization of the event attribute structures. The perf_tool.tool.attr() callback was pointing to a simple repipe. But there was no initialization of internal data structures to keep track of events and event ids. That later caused event id lookups to fail, and samples would get dropped.
The patch simply adds back the call to perf_event__process_attr() to initialize the evlist structure. Build_ids are again injected. The PID is also inject for PERF_RECORD_HEADER_BUILD_ID.
Signed-off-by Stephane Eranian <[email protected]>
---
tools/perf/builtin-inject.c +6 -0
index 09c1061..f38f370 100644
@@ -60,6 +60,11 @@ static int perf_event__repipe_tracing_data_synth(union perf_event *event,
 static int perf_event__repipe_attr(union perf_event *event,
 				   struct perf_evlist **pevlist __used)
 {
+	int ret;
+	ret = perf_event__process_attr(event, pevlist);
+	if (ret)
+		return ret;
+
 	return perf_event__repipe_synth(NULL, event, NULL);
 }
 
@@ -173,6 +178,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
 		       event->header.type);
 		goto repipe;
 	}
+	machine->pid = event->ip.pid;
 
 	thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION,
 			      event->ip.ip, &al);

Stephane Eranian

author

2012-01-13 16:54 UTC #2 lore raw

Quoted 8 lines · Arnaldo Carvalho de Melo

Em Fri, Jan 13, 2012 at 03:47:31PM +0100, Stephane Eranian escreveu:
Quoted 4 lines
perf inject -b was broken. It would not inject any build_ids into the stream. Furthermore, it would strip samples from the stream.
Against which tree is this patch?

3.2.0-rc7-tip.
Did that get fixed later on?

Quoted 37 lines

Quoted 37 lines
The reason was a missing initialization of the event attribute structures. The perf_tool.tool.attr() callback was pointing to a simple repipe. But there was no initialization of internal data structures to keep track of events and event ids. That later caused event id lookups to fail, and samples would get dropped.
The patch simply adds back the call to perf_event__process_attr() to initialize the evlist structure. Build_ids are again injected. The PID is also inject for PERF_RECORD_HEADER_BUILD_ID.
Signed-off-by Stephane Eranian <[email protected]>
---
tools/perf/builtin-inject.c +6 -0
index 09c1061..f38f370 100644
@@ -60,6 +60,11 @@ static int perf_event__repipe_tracing_data_synth(union perf_event *event,
  static int perf_event__repipe_attr(union perf_event *event,
                                  struct perf_evlist **pevlist __used)
  {
+     int ret;
+     ret = perf_event__process_attr(event, pevlist);
+     if (ret)
+             return ret;
+
       return perf_event__repipe_synth(NULL, event, NULL);
  }
 
@@ -173,6 +178,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
                      event->header.type);
               goto repipe;
       }
+     machine->pid = event->ip.pid;
 
       thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION,
                             event->ip.ip, &al);

Stephane Eranian

author

2012-01-16 18:53 UTC #3 lore raw

Arnaldo,

Quoted 42 lines · Stephane Eranian

Quoted 8 lines · Arnaldo Carvalho de Melo
Em Fri, Jan 13, 2012 at 03:47:31PM +0100, Stephane Eranian escreveu:
Quoted 4 lines
perf inject -b was broken. It would not inject any build_ids into the stream. Furthermore, it would strip samples from the stream.
Against which tree is this patch?
3.2.0-rc7-tip.
Did that get fixed later on?
Quoted 29 lines
Quoted 29 lines
The reason was a missing initialization of the event attribute structures. The perf_tool.tool.attr() callback was pointing to a simple repipe. But there was no initialization of internal data structures to keep track of events and event ids. That later caused event id lookups to fail, and samples would get dropped.
The patch simply adds back the call to perf_event__process_attr() to initialize the evlist structure. Build_ids are again injected. The PID is also inject for PERF_RECORD_HEADER_BUILD_ID.
Signed-off-by Stephane Eranian <[email protected]>
---
tools/perf/builtin-inject.c +5 -0
index 09c1061..f38f370 100644
@@ -60,6 +60,11 @@ static int perf_event__repipe_tracing_data_synth(union perf_event *event,
  static int perf_event__repipe_attr(union perf_event *event,
                                  struct perf_evlist **pevlist __used)
  {
+     int ret;
+     ret = perf_event__process_attr(event, pevlist);
+     if (ret)
+             return ret;
+
       return perf_event__repipe_synth(NULL, event, NULL);
  }
 

Quoted 6 lines

tools/perf/builtin-inject.c static int perf_event__inject_buildid(struct perf_tool *tool, · line 182

Quoted 6 lines

tools/perf/builtin-inject.c +1 -0

@@ -173,6 +178,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
                      event->header.type);
               goto repipe;
       }
+     machine->pid = event->ip.pid;

I noticed that this statement conflicts with perf buildid-list (which I am also fixing for pipe mode).
I don't quite understand why, though.

Arnaldo Carvalho de Melo

2012-01-30 19:00 UTC #4 lore raw

Em Mon, Jan 16, 2012 at 07:53:28PM +0100, Stephane Eranian escreveu:

Quoted 6 lines · Stephane Eranian

Quoted 5 lines · Arnaldo Carvalho de Melo
Quoted 4 lines
Em Fri, Jan 13, 2012 at 03:47:31PM +0100, Stephane Eranian escreveu:
Quoted 3 lines
perf inject -b was broken. It would not inject any build_ids into the stream. Furthermore, it would strip samples from the stream.

<SNIP>

Quoted 9 lines

Quoted 4 lines
Quoted 4 lines
tools/perf/builtin-inject.c static int perf_event__inject_buildid(struct perf_tool *tool, · line 180
Quoted 4 lines
tools/perf/builtin-inject.c +1 -0
@@ -173,6 +178,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
               goto repipe;
       }
+     machine->pid = event->ip.pid;
I noticed that this statement conflicts with perf buildid-list (which I am also fixing for pipe mode).
I don't quite understand why, though.

Have you reached any conclusion about this problem? I haven't looked at it in detail, could you please elaborate more?

- Arnaldo

Stephane Eranian

author

2012-01-30 19:53 UTC #5 lore raw

Quoted 17 lines · Arnaldo Carvalho de Melo

Em Mon, Jan 16, 2012 at 07:53:28PM +0100, Stephane Eranian escreveu:
Quoted 6 lines · Stephane Eranian
Quoted 5 lines · Arnaldo Carvalho de Melo
Quoted 4 lines
Em Fri, Jan 13, 2012 at 03:47:31PM +0100, Stephane Eranian escreveu:
Quoted 3 lines
perf inject -b was broken. It would not inject any build_ids into the stream. Furthermore, it would strip samples from the stream.
<SNIP>
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
tools/perf/builtin-inject.c static int perf_event__inject_buildid(struct perf_tool *tool, · line 180
Quoted 4 lines
tools/perf/builtin-inject.c +1 -0
@@ -173,6 +178,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
               goto repipe;
       }
+     machine->pid = event->ip.pid;
I noticed that this statement conflicts with perf buildid-list (which I am also fixing for pipe mode).

Quoted 5 lines

Quoted 1 line
I don't quite understand why, though.
Have you reached any conclusion about this problem? I haven't looked at it in detail, could you please elaborate more?

I ended up removing it. But I am not sure this is correct. Is the pid used in any way when processing buildids?

Quoted 2 lines

- Arnaldo

Arnaldo Carvalho de Melo

2012-01-30 20:04 UTC #6 lore raw

Em Mon, Jan 30, 2012 at 08:53:26PM +0100, Stephane Eranian escreveu:

Quoted 19 lines

Quoted 17 lines · Arnaldo Carvalho de Melo
Em Mon, Jan 16, 2012 at 07:53:28PM +0100, Stephane Eranian escreveu:
Quoted 6 lines · Stephane Eranian
Quoted 5 lines · Arnaldo Carvalho de Melo
Quoted 4 lines
Em Fri, Jan 13, 2012 at 03:47:31PM +0100, Stephane Eranian escreveu:
Quoted 3 lines
perf inject -b was broken. It would not inject any build_ids into the stream. Furthermore, it would strip samples from the stream.
<SNIP>
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
tools/perf/builtin-inject.c static int perf_event__inject_buildid(struct perf_tool *tool, · line 180
Quoted 4 lines
tools/perf/builtin-inject.c +1 -0
@@ -173,6 +178,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
               goto repipe;
       }
+     machine->pid = event->ip.pid;
I noticed that this statement conflicts with perf buildid-list (which I am also fixing for pipe mode).

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
I don't quite understand why, though.
Have you reached any conclusion about this problem? I haven't looked at it in detail, could you please elaborate more?
I ended up removing it. But I am not sure this is correct. Is the pid used in any way when processing buildids?

I can't think of any.

The same DSO could conceivably be present in the virtual machine, the host, and in the workstation used for perf report. We just use the build-id in the perf.data file to find the right symtab.

- Arnaldo

Stephane Eranian

author

2012-01-30 20:09 UTC #7 lore raw

Quoted 26 lines · Arnaldo Carvalho de Melo

Em Mon, Jan 30, 2012 at 08:53:26PM +0100, Stephane Eranian escreveu:
Quoted 25 lines
Quoted 22 lines · Arnaldo Carvalho de Melo
Em Mon, Jan 16, 2012 at 07:53:28PM +0100, Stephane Eranian escreveu:
Quoted 6 lines · Stephane Eranian
Quoted 5 lines · Arnaldo Carvalho de Melo
Quoted 4 lines
Em Fri, Jan 13, 2012 at 03:47:31PM +0100, Stephane Eranian escreveu:
Quoted 3 lines
perf inject -b was broken. It would not inject any build_ids into the stream. Furthermore, it would strip samples from the stream.
<SNIP>
Quoted 9 lines
Quoted 4 lines
Quoted 4 lines
Review static int perf_event__inject_buildid(struct perf_tool *tool, · line 180
Quoted 4 lines
(quoted diff) +1 -0
@@ -173,6 +178,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
               goto repipe;
       }
+     machine->pid = event->ip.pid;
I noticed that this statement conflicts with perf buildid-list (which I am also fixing for pipe mode).
I don't quite understand why, though.
Have you reached any conclusion about this problem? I haven't looked at it in detail, could you please elaborate more?

Quoted 9 lines

Quoted 2 lines
I ended up removing it. But I am not sure this is correct. Is the pid used in any way when processing buildids?
I can't think of any.
The same DSO could conceivably be present in the virtual machine, the host, and in the workstation used for perf report. We just use the build-id in the perf.data file to find the right symtab.

Right, so I don't know why it's there...

Arnaldo Carvalho de Melo

2012-01-30 20:37 UTC #8 lore raw

Em Mon, Jan 30, 2012 at 09:09:17PM +0100, Stephane Eranian escreveu:

Quoted 21 lines

Quoted 19 lines · Arnaldo Carvalho de Melo
Em Mon, Jan 30, 2012 at 08:53:26PM +0100, Stephane Eranian escreveu:
Quoted 17 lines
Quoted 8 lines · Arnaldo Carvalho de Melo
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
Review static int perf_event__inject_buildid(struct perf_tool *tool, · line 180
Quoted 4 lines
(quoted diff) +1 -0
@@ -173,6 +178,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
               goto repipe;
       }
+     machine->pid = event->ip.pid;
I noticed that this statement conflicts with perf buildid-list (which I am also fixing for pipe mode).
Quoted 4 lines
Quoted 1 line
I don't quite understand why, though.
Have you reached any conclusion about this problem? I haven't looked at it in detail, could you please elaborate more?
I ended up removing it. But I am not sure this is correct. Is the pid used in any way when processing buildids?

Quoted 7 lines

Quoted 5 lines
I can't think of any.
The same DSO could conceivably be present in the virtual machine, the host, and in the workstation used for perf report. We just use the build-id in the perf.data file to find the right symtab.
Right, so I don't know why it's there...

This comes from a1645ce1:

commit a1645ce12adb6c9cc9e19d7695466204e3f017fe

Author: Zhang, Yanmin <[email protected]>
Date:   Mon Apr 19 13:32:50 2010 +0800

perf: 'perf kvm' tool for monitoring guest performance from host

Here is the patch of userspace perf tool.

Signed-off-by Zhang Yanmin <[email protected]>

Signed-off-by Avi Kivity <[email protected]>

Zhang, what was the thinking about that pid in the buildid event?

Quoted 1 line

From what I can see we can just ignore it :-\

- Arnaldo

Yanmin Zhang

2012-01-31 05:58 UTC #9 lore raw

Quoted 38 lines · Arnaldo Carvalho de Melo

Em Mon, Jan 30, 2012 at 09:09:17PM +0100, Stephane Eranian escreveu:
Quoted 21 lines
Quoted 19 lines · Arnaldo Carvalho de Melo
Em Mon, Jan 30, 2012 at 08:53:26PM +0100, Stephane Eranian escreveu:
Quoted 17 lines
Quoted 8 lines · Arnaldo Carvalho de Melo
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
Review static int perf_event__inject_buildid(struct perf_tool *tool, · line 180
Quoted 4 lines
(quoted diff) +1 -0
@@ -173,6 +178,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
              goto repipe;
      }
+     machine->pid = event->ip.pid;
I noticed that this statement conflicts with perf buildid-list (which I am also fixing for pipe mode).
Quoted 4 lines
Quoted 1 line
I don't quite understand why, though.
Have you reached any conclusion about this problem? I haven't looked at it in detail, could you please elaborate more?
I ended up removing it. But I am not sure this is correct. Is the pid used in any way when processing buildids?
Quoted 7 lines
Quoted 5 lines
I can't think of any.
The same DSO could conceivably be present in the virtual machine, the host, and in the workstation used for perf report. We just use the build-id in the perf.data file to find the right symtab.
Right, so I don't know why it's there...
This comes from a1645ce1:
commit a1645ce12adb6c9cc9e19d7695466204e3f017fe
Author: Zhang, Yanmin <[email protected]>
Date:   Mon Apr 19 13:32:50 2010 +0800
perf: 'perf kvm' tool for monitoring guest performance from host

Quoted 8 lines

Here is the patch of userspace perf tool.
Signed-off-by Zhang Yanmin <[email protected]>
Signed-off-by Avi Kivity <[email protected]>
Zhang, what was the thinking about that pid in the buildid event?

I didn't work on it for a long time because of some special reasons. I check it quickly and below are some explanation.

machine->pid is to support KVM multiple guest os kernels.
1) The guest os kernels might be different version of kernels from host's;
2) The guest os might be Windows.

At host side, every guest os is a process of host although it's multi-threaded. machine->pid is to save its pid. The pid of host itself is HOST_KERNEL_ID.

In guest os, there are many processes. host os doesn't know them. So currently or when I enhanced perf to support KVM, perf filters out guest os user space detailed event samples while still keeping guest os user space simple counters.

event->ip.pid is equal to machine->pid only when
((event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK) == PERF_RECORD_MISC_GUEST_KERNEL).

In function perf_event__inject_buildid, we shouldn't reset machine->pid to event->ip.pid. They are equal to each other if it's a guest os event. Isn't it? If the event is host event, event->ip.pid points to a real process at host and it shouldn't be equal to machine->pid (HOST_KERNEL_ID).

Yanmin

Stephane Eranian

author

2012-02-02 11:22 UTC #10 lore raw

Quoted 46 lines · Yanmin Zhang

Quoted 45 lines · Arnaldo Carvalho de Melo
Em Mon, Jan 30, 2012 at 09:09:17PM +0100, Stephane Eranian escreveu:
Quoted 21 lines
Quoted 19 lines · Arnaldo Carvalho de Melo
Em Mon, Jan 30, 2012 at 08:53:26PM +0100, Stephane Eranian escreveu:
Quoted 17 lines
Quoted 8 lines · Arnaldo Carvalho de Melo
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
Review static int perf_event__inject_buildid(struct perf_tool *tool, · line 180
Quoted 4 lines
(quoted diff) +1 -0
@@ -173,6 +178,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
               goto repipe;
       }
+     machine->pid = event->ip.pid;
I noticed that this statement conflicts with perf buildid-list (which I am also fixing for pipe mode).
Quoted 4 lines
Quoted 1 line
I don't quite understand why, though.
Have you reached any conclusion about this problem? I haven't looked at it in detail, could you please elaborate more?
I ended up removing it. But I am not sure this is correct. Is the pid used in any way when processing buildids?
Quoted 7 lines
Quoted 5 lines
I can't think of any.
The same DSO could conceivably be present in the virtual machine, the host, and in the workstation used for perf report. We just use the build-id in the perf.data file to find the right symtab.
Right, so I don't know why it's there...
This comes from a1645ce1:
commit a1645ce12adb6c9cc9e19d7695466204e3f017fe
Author: Zhang, Yanmin <[email protected]>
Date: Mon Apr 19 13:32:50 2010 +0800
perf: 'perf kvm' tool for monitoring guest performance from host
Here is the patch of userspace perf tool.
Signed-off-by Zhang Yanmin <[email protected]>
Signed-off-by Avi Kivity <[email protected]>

Quoted 8 lines

Quoted 1 line
Zhang, what was the thinking about that pid in the buildid event?
I didn't work on it for a long time because of some special reasons. I check it quickly and below are some explanation.
machine->pid is to support KVM multiple guest os kernels.
1) The guest os kernels might be different version of kernels from host's;
2) The guest os might be Windows.

I understand that.

Quoted 1 line

At host side, every guest os is a process of host although it's multi-threaded.

Yes.

Quoted 2 lines

machine->pid is to save its pid. The pid of host itself is HOST_KERNEL_ID.

What do you mean by the 'pid of the host'? You always capture samples on the host on behalf of a host task. I see PID:-1 to simulate mmap of the kernel in the perf.data file. Is that what you are referring to?

Quoted 1 line

In guest os, there are many processes. host os doesn't know them. So currently or

Yes.

Quoted 3 lines

when I enhanced perf to support KVM, perf filters out guest os user space detailed event samples while still keeping guest os user space simple counters.

That is not clear to me. Are you saying, you have no visibility into the guest OS
user space processes, samples captured at that level are attributed to guest kernel?
Or are you simply dropping them?

Quoted 5 lines

event->ip.pid is equal to machine->pid only when
((event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK) == PERF_RECORD_MISC_GUEST_KERNEL).
In function perf_event__inject_buildid, we shouldn't reset machine->pid to event->ip.pid. They are equal to each other if it's a guest os event. Isn't it?

I have not tried capturing samples on a kvm process. So I don't know.

Quoted 3 lines

If the event is host event, event->ip.pid points to a real process at host and it shouldn't be equal to machine->pid (HOST_KERNEL_ID).

Yanmin Zhang

2012-02-03 03:30 UTC #11 lore raw

Quoted 58 lines · Stephane Eranian

Quoted 46 lines · Yanmin Zhang
Quoted 45 lines · Arnaldo Carvalho de Melo
Em Mon, Jan 30, 2012 at 09:09:17PM +0100, Stephane Eranian escreveu:
Quoted 21 lines
Quoted 19 lines · Arnaldo Carvalho de Melo
Em Mon, Jan 30, 2012 at 08:53:26PM +0100, Stephane Eranian escreveu:
Quoted 17 lines
Quoted 8 lines · Arnaldo Carvalho de Melo
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
Review static int perf_event__inject_buildid(struct perf_tool *tool, · line 180
Quoted 4 lines
(quoted diff) +1 -0
@@ -173,6 +178,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
              goto repipe;
      }
+     machine->pid = event->ip.pid;
I noticed that this statement conflicts with perf buildid-list (which I am also fixing for pipe mode).
Quoted 4 lines
Quoted 1 line
I don't quite understand why, though.
Have you reached any conclusion about this problem? I haven't looked at it in detail, could you please elaborate more?
I ended up removing it. But I am not sure this is correct. Is the pid used in any way when processing buildids?
Quoted 7 lines
Quoted 5 lines
I can't think of any.
The same DSO could conceivably be present in the virtual machine, the host, and in the workstation used for perf report. We just use the build-id in the perf.data file to find the right symtab.
Right, so I don't know why it's there...
This comes from a1645ce1:
commit a1645ce12adb6c9cc9e19d7695466204e3f017fe
Author: Zhang, Yanmin <[email protected]>
Date:   Mon Apr 19 13:32:50 2010 +0800
perf: 'perf kvm' tool for monitoring guest performance from host
Here is the patch of userspace perf tool.
Signed-off-by Zhang Yanmin <[email protected]>
Signed-off-by Avi Kivity <[email protected]>
Quoted 8 lines
Quoted 1 line
Zhang, what was the thinking about that pid in the buildid event?
I didn't work on it for a long time because of some special reasons. I check it quickly and below are some explanation.
machine->pid is to support KVM multiple guest os kernels.
1) The guest os kernels might be different version of kernels from host's;
2) The guest os might be Windows.
I understand that.

Quoted 7 lines

Quoted 1 line
At host side, every guest os is a process of host although it's multi-threaded.
Yes.
Quoted 2 lines
machine->pid is to save its pid. The pid of host itself is HOST_KERNEL_ID.
What do you mean by the 'pid of the host'? You always capture samples on the host on behalf of a host task. I see PID:-1 to simulate mmap of the kernel in the perf.data file. Is that what you are referring to?

Yes.

Quoted 5 lines

Quoted 2 lines
In guest os, there are many processes. host os doesn't know them. So currently or
Yes.

Quoted 8 lines

Quoted 3 lines
when I enhanced perf to support KVM, perf filters out guest os user space detailed event samples while still keeping guest os user space simple counters.
That is not clear to me. Are you saying, you have no visibility into the guest OS
user space processes, samples captured at that level are attributed to guest kernel?
Or are you simply dropping them?

It depends specific perf subcommand.
1) With per top, besides the specific function symbol statistics, it also shows the total percent of host_kernel/host_user/guest_kernel/guest_user. With the function symbol statistics, we drop guest user space data. With the total percent, we keep them into the calculation of guest user. static void perf_top__mmap_read_idx(struct perf_top *top, int idx)

{
	...
                case PERF_RECORD_MISC_GUEST_USER:
                        ++top->guest_us_samples;
                        /*
                         * TODO: we don't process guest user from host side
                         * except simple counting.
                         */
                        /* Fall thru */
                default:
                        continue;
                }
	...
}

2) With other subcommands, it seems perf would drop guest user space statistics. Sorry for my old memory.

Quoted 8 lines

Quoted 6 lines
event->ip.pid is equal to machine->pid only when
((event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK) == PERF_RECORD_MISC_GUEST_KERNEL).
In function perf_event__inject_buildid, we shouldn't reset machine->pid to event->ip.pid. They are equal to each other if it's a guest os event. Isn't it?
I have not tried capturing samples on a kvm process. So I don't know.

There is a local team here working on performance tuning/benchmarking. They use 'perf kvm' to collect/analyze their guest os performance.

Stephane Eranian

author

2012-01-26 14:16 UTC #12 lore raw

In reply to Stephane Eranian

Quoted 4 lines · Stephane Eranian

Quoted 2 lines · Arnaldo Carvalho de Melo
Em Fri, Jan 13, 2012 at 03:47:31PM +0100, Stephane Eranian escreveu:
Quoted 1 line

Quoted 9 lines

Quoted 6 lines
Quoted 3 lines
perf inject -b was broken. It would not inject any build_ids into the stream. Furthermore, it would strip samples from the stream.
Against which tree is this patch?
3.2.0-rc7-tip.
Did that get fixed later on?

Did you look at this patch?

Quoted 37 lines

Quoted 37 lines
Quoted 37 lines
The reason was a missing initialization of the event attribute structures. The perf_tool.tool.attr() callback was pointing to a simple repipe. But there was no initialization of internal data structures to keep track of events and event ids. That later caused event id lookups to fail, and samples would get dropped.
The patch simply adds back the call to perf_event__process_attr() to initialize the evlist structure. Build_ids are again injected. The PID is also inject for PERF_RECORD_HEADER_BUILD_ID.
Signed-off-by Stephane Eranian <[email protected]>
---
tools/perf/builtin-inject.c +6 -0
index 09c1061..f38f370 100644
@@ -60,6 +60,11 @@ static int perf_event__repipe_tracing_data_synth(union perf_event *event,
  static int perf_event__repipe_attr(union perf_event *event,
                                  struct perf_evlist **pevlist __used)
  {
+     int ret;
+     ret = perf_event__process_attr(event, pevlist);
+     if (ret)
+             return ret;
+
       return perf_event__repipe_synth(NULL, event, NULL);
  }
 
@@ -173,6 +178,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
                      event->header.type);
               goto repipe;
       }
+     machine->pid = event->ip.pid;
 
       thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION,
                             event->ip.ip, &al);

Arnaldo Carvalho de Melo

2012-01-26 14:24 UTC #13 lore raw

Em Thu, Jan 26, 2012 at 03:16:15PM +0100, Stephane Eranian escreveu:

Quoted 9 lines · Stephane Eranian

Quoted 8 lines
Quoted 6 lines · Arnaldo Carvalho de Melo
Em Fri, Jan 13, 2012 at 03:47:31PM +0100, Stephane Eranian escreveu:
Quoted 4 lines
perf inject -b was broken. It would not inject any build_ids into the stream. Furthermore, it would strip samples from the stream.

Quoted 6 lines

Quoted 5 lines
Quoted 2 lines
Against which tree is this patch?
3.2.0-rc7-tip.
Did that get fixed later on?
Did you look at this patch?

Looking at it now.

- Arnaldo

Quoted 37 lines

Quoted 37 lines
Quoted 37 lines
Quoted 37 lines
The reason was a missing initialization of the event attribute structures. The perf_tool.tool.attr() callback was pointing to a simple repipe. But there was no initialization of internal data structures to keep track of events and event ids. That later caused event id lookups to fail, and samples would get dropped.
The patch simply adds back the call to perf_event__process_attr() to initialize the evlist structure. Build_ids are again injected. The PID is also inject for PERF_RECORD_HEADER_BUILD_ID.
Signed-off-by Stephane Eranian <[email protected]>
---
tools/perf/builtin-inject.c +6 -0
index 09c1061..f38f370 100644
@@ -60,6 +60,11 @@ static int perf_event__repipe_tracing_data_synth(union perf_event *event,
  static int perf_event__repipe_attr(union perf_event *event,
                                  struct perf_evlist **pevlist __used)
  {
+     int ret;
+     ret = perf_event__process_attr(event, pevlist);
+     if (ret)
+             return ret;
+
       return perf_event__repipe_synth(NULL, event, NULL);
  }
 
@@ -173,6 +178,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
                      event->header.type);
               goto repipe;
       }
+     machine->pid = event->ip.pid;
 
       thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION,
                             event->ip.ip, &al);