All of lore.kernel.org
 help / color / mirror / Atom feed
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 = {


      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 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.