Messages in this thread Patch in this message | | | Date | Fri, 12 Mar 2010 00:50:16 +0300 | From | Cyrill Gorcunov <> | Subject | Re: [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)
| |