Messages in this thread Patch in this message |  | | Subject | [PATCH 2/5] OProfile update | From | John Levon <> | Date | Mon, 26 May 2003 05:27:29 +0100 |
| |
The code that attempts to reset last_task and in_kernel has a race against samples appearing during the handling of the buffers, that causes a small number of mis-attribution of samples. Closing the window is non-obvious, and not worth it, so we just make it smaller. Even without the patch, there seem to be few such "bad" samples because its effects are mitigated on a switch into userspace or a task switch.
diff -Naur -X dontdiff linux-cvs/drivers/oprofile/buffer_sync.c linux-me/drivers/oprofile/buffer_sync.c --- linux-cvs/drivers/oprofile/buffer_sync.c 2003-05-26 03:20:20.000000000 +0100 +++ linux-me/drivers/oprofile/buffer_sync.c 2003-05-26 04:25:15.000000000 +0100 @@ -366,12 +377,26 @@ } -/* compute number of filled slots in cpu_buffer queue */ -static unsigned long nr_filled_slots(struct oprofile_cpu_buffer * b) +/* "acquire" as many cpu buffer slots as we can */ +static unsigned long get_slots(struct oprofile_cpu_buffer * b) { unsigned long head = b->head_pos; unsigned long tail = b->tail_pos; + /* + * Subtle. This resets the persistent last_task + * and in_kernel values used for switching notes. + * BUT, there is a small window between reading + * head_pos, and this call, that means samples + * can appear at the new head position, but not + * be prefixed with the notes for switching + * kernel mode or a task switch. This small hole + * can lead to mis-attribution or samples where + * we don't know if it's in the kernel or not, + * at the start of an event buffer. + */ + cpu_buffer_reset(b); + if (head >= tail) return head - tail; @@ -408,9 +433,9 @@ /* Remember, only we can modify tail_pos */ - unsigned long const available_elements = nr_filled_slots(cpu_buf); + unsigned long const available = get_slots(cpu_buf); - for (i=0; i < available_elements; ++i) { + for (i=0; i < available; ++i) { struct op_sample * s = &cpu_buf->buffer[cpu_buf->tail_pos]; if (is_ctx_switch(s->eip)) { @@ -435,8 +460,6 @@ increment_tail(cpu_buf); } release_mm(mm); - - cpu_buffer_reset(cpu_buf); } - 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/
|  |