From: John Levon <levon@movementarian.org>
To: torvalds@transmeta.com, linux-kernel@vger.kernel.org
Subject: [PATCH 2/5] OProfile update
Date: Mon, 26 May 2003 05:27:29 +0100 [thread overview]
Message-ID: <10539232492885@movementarian.org> (raw)
In-Reply-To: <1053923249281@movementarian.org>
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);
}
next prev parent reply other threads:[~2003-05-26 4:14 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2003-05-26 4:27 [PATCH 1/5] OProfile update John Levon
2003-05-26 4:27 ` John Levon [this message]
2003-05-26 4:27 ` [PATCH 3/5] " John Levon
2003-05-26 4:27 ` [PATCH 4/5] " John Levon
2003-05-26 4:27 ` [PATCH 5/5] " John Levon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=10539232492885@movementarian.org \
--to=levon@movementarian.org \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@transmeta.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.