Re: [PATCH v2 07/54] perf: Add generic exclude_guest support

Peter Zijlstra
2024-06-12T11:17:51Z

On Tue, Jun 11, 2024 at 09:27:46AM -0400, Liang, Kan wrote:
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index dd4920bf3d1b..68c8b93c4e5c 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -945,6 +945,7 @@ struct perf_event_context {
> u64 time;
> u64 timestamp;
> u64 timeoffset;
> + u64 timeguest;
>
> /*
> * These fields let us detect when two contexts have both

> @@ -651,10 +653,26 @@ __perf_update_times(struct perf_event *event, u64
> now, u64 *enabled, u64 *runnin
>
> static void perf_event_update_time(struct perf_event *event)
> {
> - u64 now = perf_event_time(event);
> + u64 now;
> +
> + /* Never count the time of an active guest into an exclude_guest event. */
> + if (event->ctx->timeguest &&
> + event->pmu->capabilities & PERF_PMU_CAP_PASSTHROUGH_VPMU) {
> + /*
> + * If a guest is running, use the timestamp while entering the guest.
> + * If the guest is leaving, reset the event timestamp.
> + */
> + if (__this_cpu_read(perf_in_guest))
> + event->tstamp = event->ctx->timeguest;
> + else
> + event->tstamp = event->ctx->time;
> + return;
> + }
>
> + now = perf_event_time(event);
> __perf_update_times(event, now, &event->total_time_enabled,
> &event->total_time_running);
> +
> event->tstamp = now;
> }

So I really don't like this much, and AFAICT this is broken. At the very
least this doesn't work right for cgroup events, because they have their
own timeline.

Let me have a poke...