From: John Levon <levon@movementarian.org>
To: torvalds@transmeta.com, linux-kernel@vger.kernel.org
Subject: [PATCH 8/8] OProfile update
Date: Sun, 4 May 2003 00:44:08 +0100 [thread overview]
Message-ID: <1052005448795@movementarian.org> (raw)
In-Reply-To: <10520054473121@movementarian.org>
Schedule work away on an unmap, instead of calling it directly. Should result
in less lost samples, and it fixes a lock ordering problem buffer_sem <-> mmap_sem
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-04-05 18:44:49.000000000 +0100
+++ linux-me/drivers/oprofile/buffer_sync.c 2003-05-03 20:10:44.000000000 +0100
@@ -58,8 +58,8 @@
* must concern ourselves with. First, when a task is about to
* exit (exit_mmap()), we should process the buffer to deal with
* any samples in the CPU buffer, before we lose the ->mmap information
- * we need. Second, a task may unmap (part of) an executable mmap,
- * so we want to process samples before that happens too
+ * we need. It is vital to get this case correct, otherwise we can
+ * end up trying to access a freed task_struct.
*/
static int mm_notify(struct notifier_block * self, unsigned long val, void * data)
{
@@ -67,6 +67,29 @@
return 0;
}
+
+/* Second, a task may unmap (part of) an executable mmap,
+ * so we want to process samples before that happens too. This is merely
+ * a QOI issue not a correctness one.
+ */
+static int munmap_notify(struct notifier_block * self, unsigned long val, void * data)
+{
+ /* Note that we cannot sync the buffers directly, because we might end up
+ * taking the the mmap_sem that we hold now inside of event_buffer_read()
+ * on a page fault, whilst holding buffer_sem - deadlock.
+ *
+ * This would mean a threaded reader of the event buffer, but we should
+ * prevent it anyway.
+ *
+ * Delaying the work in a context that doesn't hold the mmap_sem means
+ * that we won't lose samples from other mappings that current() may
+ * have. Note that either way, we lose any pending samples for what is
+ * being unmapped.
+ */
+ schedule_work(&sync_wq);
+ return 0;
+}
+
/* We need to be told about new modules so we don't attribute to a previously
* loaded module, or drop the samples on the floor.
@@ -92,7 +115,7 @@
};
static struct notifier_block exec_unmap_nb = {
- .notifier_call = mm_notify,
+ .notifier_call = munmap_notify,
};
static struct notifier_block exit_mmap_nb = {
prev parent reply other threads:[~2003-05-03 23:33 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2003-05-03 23:44 [PATCH 1/8] OProfile update John Levon
2003-05-03 23:44 ` [PATCH 2/8] " John Levon
2003-05-03 23:44 ` [PATCH 3/8] " John Levon
2003-05-03 23:44 ` [PATCH 4/8] " John Levon
2003-05-03 23:44 ` [PATCH 5/8] " John Levon
2003-05-03 23:44 ` [PATCH 6/8] " John Levon
2003-05-03 23:44 ` [PATCH 7/8] " John Levon
2003-05-03 23:44 ` John Levon [this message]
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=1052005448795@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox