Messages in this thread Patch in this message | | | Date | Sun, 21 Oct 2007 14:08:42 +0200 | From | Philippe Elie <> | Subject | [patch 1/2] oProfile: oops when profile_pc() return ~0LU |
| |
From: Philippe Elie <phil.el@wanadoo.fr>
Instruction pointer returned by profile_pc() can be a random value. This break the assumption than we can safely set struct op_sample.eip field to a magic value to signal to the per-cpu buffer reader side special event like task switch ending up in a segfault in get_task_mm() when profile_pc() return ~0UL. Fixed by exchanging the meaning of eip/event field for these specials events.
Special thanks to Sami Farin who reported the oops and helped patiently to narrow down the problem.
CC: Andrew Morton <akpm@linux-foundation.org> CC: Linus Torvalds <torvalds@linux-foundation.org> CC: Sami Farin <safari-kernel@safari.iki.fi>
--- drivers/oprofile/buffer_sync.c | 12 ++++++------ drivers/oprofile/cpu_buffer.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-)
Beside Sami, Jesse Barnes already reported this bug two years ago but I've been unable to understand it at this time.
diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c index 8134c7e..7a4ccc7 100644 --- a/drivers/oprofile/buffer_sync.c +++ b/drivers/oprofile/buffer_sync.c @@ -514,21 +514,21 @@ void sync_buffer(int cpu) for (i = 0; i < available; ++i) { struct op_sample * s = &cpu_buf->buffer[cpu_buf->tail_pos]; - if (is_code(s->eip)) { - if (s->event <= CPU_IS_KERNEL) { + if (is_code(s->event)) { + if (s->eip <= CPU_IS_KERNEL) { /* kernel/userspace switch */ - in_kernel = s->event; + in_kernel = s->eip; if (state == sb_buffer_start) state = sb_sample_start; - add_kernel_ctx_switch(s->event); - } else if (s->event == CPU_TRACE_BEGIN) { + add_kernel_ctx_switch(s->eip); + } else if (s->eip == CPU_TRACE_BEGIN) { state = sb_bt_start; add_trace_begin(); } else { struct mm_struct * oldmm = mm; /* userspace context switch */ - new = (struct task_struct *)s->event; + new = (struct task_struct *)s->eip; release_mm(oldmm); mm = take_tasks_mm(new); diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c index a83c3db..c856530 100644 --- a/drivers/oprofile/cpu_buffer.c +++ b/drivers/oprofile/cpu_buffer.c @@ -156,7 +156,7 @@ add_sample(struct oprofile_cpu_buffer * cpu_buf, static inline void add_code(struct oprofile_cpu_buffer * buffer, unsigned long value) { - add_sample(buffer, ESCAPE_CODE, value); + add_sample(buffer, value, ESCAPE_CODE); } /* This must be safe from any context. It's safe writing here - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |