Messages in this thread Patch in this message | | | Date | Thu, 26 Aug 2010 20:36:35 +0800 | From | DDD <> | Subject | Re: [PATCH] perf: fix possible divide-by-zero in perf_swevent_overflow() |
| |
Peter Zijlstra wrote: > On Thu, 2010-08-26 at 20:07 +0800, Dongdong Deng wrote: >> The event->hw.last_period is possible to zero, thus it will >> cause divide_by_zero later in perf_swevent_set_period(). > > How can it be zero?
When I am running the kgdbts to test the hw_breakpoint_layer with kgdb, I get a call trace as following and this problem is hardly to reproduce.
Maybe the root cause was from kgdb/hw_breakpoint_layer, but add a checking is good to us and harmless. :-)
Thanks, Dongdong
------------[ cut here ]------------ WARNING: at /buildarea/ddeng/build/linux/drivers/misc/kgdbts.c:703 run_simple_test+0x1fc/0x2a0() Hardware name: Moon Creek platform Modules linked in: Pid: 668, comm: sh Tainted: G W 2.6.36-rc1-00133-g47ade1d #43 Call Trace: [<c103b1fd>] warn_slowpath_common+0x6d/0xa0 [<c124e15c>] ? run_simple_test+0x1fc/0x2a0 [<c124e15c>] ? run_simple_test+0x1fc/0x2a0 [<c103b245>] warn_slowpath_null+0x15/0x20 [<c124e15c>] run_simple_test+0x1fc/0x2a0 [<c124daf4>] kgdbts_put_char+0x14/0x20 [<c107bab8>] gdb_serial_stub+0x678/0xbd0 [<c101aced>] ? default_send_IPI_allbutself+0x7d/0x90 [<c107a487>] kgdb_cpu_enter+0x197/0x470 [<c107a8e7>] kgdb_handle_exception+0x47/0x140 [<c13be9f0>] ? _raw_read_unlock+0x10/0x30 [<c101eb48>] __kgdb_notify+0x38/0x170 [<c130f1e0>] ? sock_queue_rcv_skb+0xe0/0x130 [<c101ec8e>] kgdb_notify+0xe/0x20 [<c105b815>] notifier_call_chain+0x35/0x70 [<c1003910>] ? do_divide_error+0x0/0xa0 [<c105bce8>] __atomic_notifier_call_chain+0x28/0x50 [<c105bd2a>] atomic_notifier_call_chain+0x1a/0x20 [<c105bd7d>] notify_die+0x2d/0x30 [<c1003964>] do_divide_error+0x54/0xa0 [<c11cf3d5>] ? div64_u64+0x55/0x80 [<c1361c22>] ? udp_rcv+0x12/0x20 [<c133e6e7>] ? ip_local_deliver_finish+0x127/0x240 [<c1361c22>] ? udp_rcv+0x12/0x20 [<c133e887>] ? ip_local_deliver+0x87/0x90 [<c109bb8e>] ? perf_output_begin+0x1fe/0x230 [<c133e0c3>] ? ip_rcv_finish+0xc3/0x320 [<c13bf822>] error_code+0x66/0x6c [<c1003910>] ? do_divide_error+0x0/0xa0 [<c11cf3d5>] ? div64_u64+0x55/0x80 [<c1097cac>] perf_swevent_set_period+0x7c/0x100 [<c109c797>] perf_swevent_overflow+0x87/0x90 [<c109c85f>] perf_swevent_add+0xbf/0xd0 [<c109c8ce>] perf_bp_event+0x5e/0x80 [<c1289be7>] ? e1000_clean+0xf7/0x260 [<c1008941>] hw_breakpoint_exceptions_notify+0x111/0x170 [<c105b815>] notifier_call_chain+0x35/0x70 [<c105bce8>] __atomic_notifier_call_chain+0x28/0x50 [<c105bd2a>] atomic_notifier_call_chain+0x1a/0x20 [<c105bd7d>] notify_die+0x2d/0x30 [<c1004088>] do_debug+0x68/0x270 [<c101a4d4>] ? smp_apic_timer_interrupt+0x74/0x140 [<c13bf871>] debug_stack_correct+0x29/0x30 [<c101007b>] ? hw_perf_event_init+0x1bb/0x820 [<c124de60>] ? kgdbts_break_test+0x0/0x30 [<c124e870>] ? configure_kgdbts+0x1d0/0x4c0 [<c10544f0>] ? param_attr_store+0x0/0x20 [<c124ebad>] param_set_kgdbts_var+0x4d/0xb0 [<c124eb60>] ? param_set_kgdbts_var+0x0/0xb0 [<c1054507>] param_attr_store+0x17/0x20 [<c105458c>] module_attr_store+0x2c/0x40 [<c111fe54>] sysfs_write_file+0x94/0xf0 [<c10d42c6>] vfs_write+0x96/0x130 [<c111fdc0>] ? sysfs_write_file+0x0/0xf0 [<c10d44a6>] sys_write+0x46/0xd0 [<c13bf209>] system_call_done+0x0/0x4 ---[ end trace e0a820ecc2c65c24 ]---
> >> This patch checks event->hw.last_period before invoke >> perf_swevent_set_period() and replaces "event->hw" with "hwc". >> >> Signed-off-by: Dongdong Deng <dongdong.deng@windriver.com> >> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> >> Cc: Frederic Weisbecker <fweisbec@gmail.com> >> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> >> Cc: Paul Mackerras <paulus@samba.org> >> --- >> kernel/perf_event.c | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/perf_event.c b/kernel/perf_event.c >> index 403d180..ccba741 100644 >> --- a/kernel/perf_event.c >> +++ b/kernel/perf_event.c >> @@ -4050,8 +4050,8 @@ static void perf_swevent_overflow(struct perf_event *event, u64 overflow, >> struct hw_perf_event *hwc = &event->hw; >> int throttle = 0; >> >> - data->period = event->hw.last_period; >> - if (!overflow) >> + data->period = hwc->last_period; >> + if (!overflow && hwc->last_period) >> overflow = perf_swevent_set_period(event); >> >> if (hwc->interrupts == MAX_INTERRUPTS) > > >
| |