Commit 5422b467 authored by John Levon's avatar John Levon Committed by Linus Torvalds

[PATCH] OProfile update

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
parent 8eac5492
...@@ -58,8 +58,8 @@ static int exit_task_notify(struct notifier_block * self, unsigned long val, voi ...@@ -58,8 +58,8 @@ static int exit_task_notify(struct notifier_block * self, unsigned long val, voi
* must concern ourselves with. First, when a task is about to * must concern ourselves with. First, when a task is about to
* exit (exit_mmap()), we should process the buffer to deal with * exit (exit_mmap()), we should process the buffer to deal with
* any samples in the CPU buffer, before we lose the ->mmap information * any samples in the CPU buffer, before we lose the ->mmap information
* we need. Second, a task may unmap (part of) an executable mmap, * we need. It is vital to get this case correct, otherwise we can
* so we want to process samples before that happens too * end up trying to access a freed task_struct.
*/ */
static int mm_notify(struct notifier_block * self, unsigned long val, void * data) static int mm_notify(struct notifier_block * self, unsigned long val, void * data)
{ {
...@@ -67,6 +67,29 @@ static int mm_notify(struct notifier_block * self, unsigned long val, void * dat ...@@ -67,6 +67,29 @@ static int mm_notify(struct notifier_block * self, unsigned long val, void * dat
return 0; 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 /* 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. * loaded module, or drop the samples on the floor.
...@@ -92,7 +115,7 @@ static struct notifier_block exit_task_nb = { ...@@ -92,7 +115,7 @@ static struct notifier_block exit_task_nb = {
}; };
static struct notifier_block exec_unmap_nb = { static struct notifier_block exec_unmap_nb = {
.notifier_call = mm_notify, .notifier_call = munmap_notify,
}; };
static struct notifier_block exit_mmap_nb = { static struct notifier_block exit_mmap_nb = {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment