lkml.org 
[lkml]   [2013]   [Jul]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 3/7] perf, x86: Introduce x86 special perf event context
    On Mon, Jul 01, 2013 at 03:23:03PM +0800, Yan, Zheng wrote:
    > From: "Yan, Zheng" <zheng.z.yan@intel.com>
    >
    > The x86 special perf event context is named x86_perf_event_context,
    > We can enlarge it later to store PMU special data.

    This changelog is completely inadequate. It fails to state what and why
    we do things.

    I hate doing this; but I can't see another way around it either. That
    said:

    > @@ -274,6 +274,11 @@ struct pmu {
    > * flush branch stack on context-switches (needed in cpu-wide mode)
    > */
    > void (*flush_branch_stack) (void);
    > +
    > + /*
    > + * Allocate PMU special perf event context
    > + */
    > + void *(*event_context_alloc) (struct perf_event_context *parent_ctx);
    > };

    It should be *optional*, also wtf is that parent_ctx thing for?

    > +++ b/kernel/events/core.c
    > @@ -2961,13 +2961,20 @@ static void __perf_event_init_context(struct perf_event_context *ctx)
    > }
    >
    > static struct perf_event_context *
    > -alloc_perf_context(struct pmu *pmu, struct task_struct *task)
    > +alloc_perf_context(struct pmu *pmu, struct task_struct *task,
    > + struct perf_event_context *parent_ctx)
    > {
    > struct perf_event_context *ctx;
    >
    > - ctx = kzalloc(sizeof(struct perf_event_context), GFP_KERNEL);
    > - if (!ctx)
    > - return NULL;
    > + if (pmu->event_context_alloc) {
    > + ctx = pmu->event_context_alloc(parent_ctx);
    > + if (IS_ERR(ctx))
    > + return ctx;
    > + } else {
    > + ctx = kzalloc(sizeof(struct perf_event_context), GFP_KERNEL);
    > + if (!ctx)
    > + return ERR_PTR(-ENOMEM);
    > + }
    >
    > __perf_event_init_context(ctx);
    > if (task) {

    I'm not at all sure we want to do it like this; why not simply query the
    size. Something like:

    alloc_perf_context(struct pmu *pmu, struct task_struct *task)
    {
    size_t ctx_size = sizeof(struct perf_event_context);

    if (pmu->task_context_size)
    size = pmu->task_context_size();

    ctx = kzalloc(size, GFP_KERNEL);
    if (!ctx)
    return ERR_PTR(-ENOMEM);

    ...

    }




    \
     
     \ /
      Last update: 2013-07-04 15:21    [W:3.198 / U:0.584 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site