lkml.org 
[lkml]   [2010]   [Mar]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [RFC] x86,perf: Implement minimal P4 PMU driver v14
    On Thu, Mar 11, 2010 at 10:38:00PM +0100, Peter Zijlstra wrote:
    > On Fri, 2010-03-12 at 00:31 +0300, Cyrill Gorcunov wrote:
    > > On Thu, Mar 11, 2010 at 10:24:22PM +0100, Peter Zijlstra wrote:
    > > > On Fri, 2010-03-12 at 00:15 +0300, Cyrill Gorcunov wrote:
    > > >
    > > > > Perhaps something like the patch below (tested with kvm)? With this patch
    > > > > we will actually waste ~4/8 bytes per PMU (intel,amd,p6) since this call
    > > > > hits on p4 only, so I think perhaps better to use one x86 scheduler hook
    > > > > instead of empty schedule_events() in PMU, hmm?
    > > > > ---
    > > > >
    > > > > x86,perf: Fix NULL deref on not assigned x86_pmu
    > > > >
    > > > > In case of not assigned x86_pmu and software events
    > > > > NULL dereference may being hit via x86_pmu::schedule_events
    > > > > method.
    > > > >
    > > > > Fix it by calling x86_pmu::schedule_events only if we
    > > > > have one. Otherwise use general scheduler.
    > > > >
    > > > > Also the former x86_schedule_events calls restored.
    > > >
    > > > Hrm,.. not sure that makes sense, sure it might not crash anymore, but
    > > > its not making much sense to compute anything if we don't have an
    > > > initialized x86_pmu.
    > > >
    > > > Doesn't adding something like:
    > > >
    > > > if (!x86_pmu_initialized())
    > > > return;
    > > >
    > > > to hw_perf_group_sched_in() make more sense? We seem to do that for all
    > > > these weak things except this one.
    > > >
    > >
    > > As far as I see it'll not update tstamp_running then (in x86_event_sched_in).
    > > Or I miss somethig?
    >
    > Have it return 0 and it will fallback to defaults. Since there is no
    > initialized x86_pmu there's no point in doing anything x86 specific.
    >

    I suppose you mean something like below.

    -- Cyrill
    ---
    x86,perf: Fix NULL deref on not assigned x86_pmu

    In case of not assigned x86_pmu and software events
    NULL dereference may being hit via x86_pmu::schedule_events
    method.

    Fix it by checking if x86_pmu is initialized at all.

    Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
    ---
    arch/x86/kernel/cpu/perf_event.c | 3 +++
    1 file changed, 3 insertions(+)

    Index: linux-2.6.git/arch/x86/kernel/cpu/perf_event.c
    =====================================================================
    --- linux-2.6.git.orig/arch/x86/kernel/cpu/perf_event.c
    +++ linux-2.6.git/arch/x86/kernel/cpu/perf_event.c
    @@ -1263,6 +1263,9 @@ int hw_perf_group_sched_in(struct perf_e
    int assign[X86_PMC_IDX_MAX];
    int n0, n1, ret;

    + if (!x86_pmu_initialized())
    + return 0;
    +
    /* n0 = total number of events */
    n0 = collect_events(cpuc, leader, true);
    if (n0 < 0)

    \
     
     \ /
      Last update: 2010-03-11 22:53    [W:0.026 / U:0.120 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site