Re: [PATCH 12/35] perf tools: Add guest_cpu to hypervisor threads

Ian Rogers
2022-07-20T00:24:07Z

On Mon, Jul 11, 2022 at 2:33 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> It is possible to know which guest machine was running at a point in time
> based on the PID of the currently running host thread. That is, perf
> identifies guest machines by the PID of the hypervisor.
>
> To determine the guest CPU, put it on the hypervisor (QEMU) thread for
> that VCPU.
>
> This is done when processing the id_index which provides the necessary
> information.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> tools/perf/util/session.c | 18 ++++++++++++++++++
> tools/perf/util/thread.c | 1 +
> tools/perf/util/thread.h | 1 +
> 3 files changed, 20 insertions(+)
>
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 1af981d5ad3c..91a091c35945 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -2769,6 +2769,20 @@ static int perf_session__register_guest(struct perf_session *session, pid_t mach
> return 0;
> }
>
> +static int perf_session__set_guest_cpu(struct perf_session *session, pid_t pid,
> + pid_t tid, int guest_cpu)
> +{
> + struct machine *machine = &session->machines.host;
> + struct thread *thread = machine__findnew_thread(machine, pid, tid);
> +
> + if (!thread)
> + return -ENOMEM;
> + thread->guest_cpu = guest_cpu;
> + thread__put(thread);
> +
> + return 0;
> +}
> +
> int perf_event__process_id_index(struct perf_session *session,
> union perf_event *event)
> {
> @@ -2845,6 +2859,10 @@ int perf_event__process_id_index(struct perf_session *session,
> last_pid = sid->machine_pid;
> perf_guest = true;
> }
> +
> + ret = perf_session__set_guest_cpu(session, sid->machine_pid, e->tid, e2->vcpu);
> + if (ret)
> + return ret;
> }
> return 0;
> }
> diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> index 665e5c0618ed..e3e5427e1c3c 100644
> --- a/tools/perf/util/thread.c
> +++ b/tools/perf/util/thread.c
> @@ -47,6 +47,7 @@ struct thread *thread__new(pid_t pid, pid_t tid)
> thread->tid = tid;
> thread->ppid = -1;
> thread->cpu = -1;
> + thread->guest_cpu = -1;
> thread->lbr_stitch_enable = false;
> INIT_LIST_HEAD(&thread->namespaces_list);
> INIT_LIST_HEAD(&thread->comm_list);
> diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> index b066fb30d203..241f300d7d6e 100644
> --- a/tools/perf/util/thread.h
> +++ b/tools/perf/util/thread.h
> @@ -39,6 +39,7 @@ struct thread {
> pid_t tid;
> pid_t ppid;
> int cpu;
> + int guest_cpu; /* For QEMU thread */

Could we tweak the comments here to be something like:

int cpu; /* The CPU the thread is currently running on or the CPU of
the hypervisor thread. */
int guest_cpu; /* The CPU within a guest (QEMU) that's running. */

Does -1 convey meaning beyond uninitialized, like with the 'any' CPU
perf_event_open argument?

Thanks,
Ian

> refcount_t refcnt;
> bool comm_set;
> int comm_len;
> --
> 2.25.1
>