From: John Levon <levon@movementarian.org>
To: torvalds@transmeta.com, linux-kernel@vger.kernel.org
Subject: [PATCH 5/8] OProfile update
Date: Sun, 4 May 2003 00:44:07 +0100 [thread overview]
Message-ID: <10520054473909@movementarian.org> (raw)
In-Reply-To: <10520054463981@movementarian.org>
Now we don't do buffer syncs whilst holding current's mmap_sem ever,
we can wait to hold it. Also take task_lock and try to improve the docs a bit.
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
@@ -310,26 +337,23 @@
/* Take the task's mmap_sem to protect ourselves from
* races when we do lookup_dcookie().
*/
-static struct mm_struct * take_task_mm(struct task_struct * task)
+static struct mm_struct * take_tasks_mm(struct task_struct * task)
{
- struct mm_struct * mm = task->mm;
-
- /* if task->mm !NULL, mm_count must be at least 1. It cannot
- * drop to 0 without the task exiting, which will have to sleep
- * on buffer_sem first. So we do not need to mark mm_count
- * ourselves.
+ struct mm_struct * mm;
+
+ /* Subtle. We don't need to keep a reference to this task's mm,
+ * because, for the mm to be freed on another CPU, that would have
+ * to go through the task exit notifier, which ends up sleeping
+ * on the buffer_sem we hold, so we end up with mutual exclusion
+ * anyway.
*/
+ task_lock(task);
+ mm = task->mm;
+ task_unlock(task);
+
if (mm) {
- /* More ugliness. If a task took its mmap
- * sem then came to sleep on buffer_sem we
- * will deadlock waiting for it. So we can
- * but try. This will lose samples :/
- */
- if (!down_read_trylock(&mm->mmap_sem)) {
- /* FIXME: this underestimates samples lost */
- atomic_inc(&oprofile_stats.sample_lost_mmap_sem);
- mm = NULL;
- }
+ /* needed to walk the task's VMAs */
+ down_read(&mm->mmap_sem);
}
return mm;
@@ -399,7 +423,7 @@
new = (struct task_struct *)s->event;
release_mm(mm);
- mm = take_task_mm(new);
+ mm = take_tasks_mm(new);
cookie = get_exec_dcookie(mm);
add_user_ctx_switch(new->pid, cookie);
@@ -460,4 +484,3 @@
schedule_work(&sync_wq);
/* timer is re-added by the scheduled task */
}
-
next 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 ` John Levon [this message]
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 ` [PATCH 8/8] " 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=10520054473909@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.