lkml.org 
[lkml]   [2022]   [Aug]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC v2] perf: Rewrite core context handling
    On Tue, Aug 02, 2022 at 11:47:24AM +0530, Ravi Bangoria wrote:
    > [...]
    >
    > > static void
    > > -ctx_sched_in(struct perf_event_context *ctx,
    > > - struct perf_cpu_context *cpuctx,
    > > - enum event_type_t event_type,
    > > +ctx_sched_in(struct perf_event_context *ctx, enum event_type_t event_type,
    > > struct task_struct *task)
    > > {
    > > + struct perf_cpu_context *cpuctx = this_cpu_ptr(&cpu_context);
    > > int is_active = ctx->is_active;
    > > u64 now;
    > >
    > > @@ -3818,6 +3905,7 @@ ctx_sched_in(struct perf_event_context *ctx,
    > > /* start ctx time */
    > > now = perf_clock();
    > > ctx->timestamp = now;
    > > + // XXX ctx->task =? task
    >
    > Couldn't get this XXX, it's from your original patch. If you can recall, it
    > would be helpful.

    No memories at all; but looking at it; it seems to worry if ctx->task is
    up-to-date; in this context the only thing that relies on the task is
    the cgroup for which we update the timestamp in the next statement:

    > > perf_cgroup_set_timestamp(task, ctx);

    I suppose I should really write less cryptic notes; then again, I never
    imagined this would take that many years to complete :/

    > > }
    >
    > Also, this hunk is under if (is_active ^ EVENT_TIME), which effectively is
    > (is_active != EVENT_TIME). I'm assuming it should be (is_active & EVENT_TIME)?

    So that code is identical to what it currently is upstream; but yes that
    looks somewhat dodgy.

    So the code itself (does as the comment says) starts time. This should
    only be done if EVENT_TIME is not set. That is, I'm thinking it should
    be something like:

    !(is_active & EVENT_TIME)

    which happens to be the same as:

    is_active ^ EVENT_TIME

    under the assumption is_active contains no other bits -- which I don't
    think is a valid assumption.

    \
     
     \ /
      Last update: 2022-08-23 09:27    [W:2.181 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site