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

[Ian Rogers <irogers@google.com>]

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
>