Messages in this thread | | | From | Song Liu <> | Subject | Re: [PATCH] perf: fix perf_event_context->time | Date | Wed, 1 Mar 2023 23:16:32 +0000 |
| |
> On Mar 1, 2023, at 2:29 PM, Namhyung Kim <namhyung@kernel.org> wrote: > > Hi Song, > > On Tue, Feb 28, 2023 at 11:22 AM Song Liu <song@kernel.org> wrote: >> >> Time readers rely on perf_event_context->[time|timestamp|timeoffset] to get >> accurate time_enabled and time_running for an event. The difference between >> ctx->timestamp and ctx->time is the among of time when the context is not >> enabled. For cpuctx.ctx, time and timestamp should stay the same, however, > > I'm not sure if it's correct. The timestamp can go when the context is disabled > for example, in ctx_resched() even if the NMI watchdog is enabled, right?
I think we do not disable EVENT_TIME for per cpu ctx?
> >> it is not the case at the moment. To show this with drgn [1]: >> >> drgn 0.0.22 (using Python 3.8.6, elfutils 0.185, with libkdumpfile) >> For help, type help(drgn). >> ... >>>>> ctx = per_cpu_ptr(prog['pmu'].pmu_cpu_context, 0).ctx >>>>> ctx.timestamp * 1.0 / ctx.time >> (double)1.0385869134111765 >>>>> >> >> ctx->timestamp is about 4% larger than ctx.time. This issue causes time >> read by perf_event_read_local() goes back in some cases. > > I don't think the difference between the ctx.time and the timestamp > itself is a problem. I think the problem is it CAN update the timestamp > (and the timeoffset) while the context is enabled. Then depending on > the timing, event times can return smaller values than before.
For per cpu ctx, I think timeoffset should stay the same (may not be zero though).
Thanks, Song
| |