Re: [PATCH] perf: fix perf_event_context->time

From: Song Liu
Date: Thu Mar 02 2023 - 20:22:00 EST




> On Mar 2, 2023, at 1:15 PM, Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
>
> On Wed, Mar 1, 2023 at 3:16 PM Song Liu <songliubraving@xxxxxxxx> wrote:
>>
>>
>>
>>> On Mar 1, 2023, at 2:29 PM, Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
>>>
>>> Hi Song,
>>>
>>> On Tue, Feb 28, 2023 at 11:22 AM Song Liu <song@xxxxxxxxxx> 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?
>
> I can see ctx_sched_out(ctx, EVENT_TIME) in some places.
> Also it'd reset EVENT_TIME if both _PINNED and _FLEXIBLE is
> cleared.

Yeah, you are right. I missed this part.

Hi Peter,

Does this fix look good do you?

Thanks,
Song