From 0d22b578d5e540aa4d046d8f9eebfe5166d3f4a4 Mon Sep 17 00:00:00 2001 From: John Levon Date: Sun, 25 May 2003 07:42:35 -0700 Subject: [PATCH] [PATCH] OProfile: minimize sample error 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. --- drivers/oprofile/buffer_sync.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c index a5bcb23929e6..59d6fc778039 100644 --- a/drivers/oprofile/buffer_sync.c +++ b/drivers/oprofile/buffer_sync.c @@ -372,12 +372,26 @@ static inline int is_ctx_switch(unsigned long val) } -/* 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; @@ -414,9 +428,9 @@ static void sync_buffer(struct oprofile_cpu_buffer * cpu_buf) /* 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)) { @@ -441,8 +455,6 @@ static void sync_buffer(struct oprofile_cpu_buffer * cpu_buf) increment_tail(cpu_buf); } release_mm(mm); - - cpu_buffer_reset(cpu_buf); } -- 2.39.5