Messages in this thread | | | From | Dave Peterson <> | Subject | Re: [PATCH 2/2] mm: fix mm_struct reference counting bugs in mm/oom_kill.c | Date | Fri, 14 Apr 2006 12:14:35 -0700 |
| |
On Friday 14 April 2006 00:26, Andrew Morton wrote: > task_lock() can be used to pin a task's ->mm. To use task_lock() in > badness() we'd need to either > > a) nest task_lock()s. I don't know if we're doing that anywhere else, > but the parent->child ordering is a natural one. or > > b) take a ref on the parent's mm_struct, drop the parent's task_lock() > while we walk the children, then do mmput() on the parent's mm outside > tasklist_lock. This is probably better.
Looking a bit more closely at the code, I see that select_bad_process() iterates over all tasks, repeatedly calling badness(). This would complicate option 'b' since the iteration is done while holding tasklist_lock. An alternative to option 'a' that avoids nesting task_lock()s would be to define a couple of new functions that might look something like this:
void mmput_atomic(struct mm_struct *mm) { if (atomic_dec_and_test(&mm->mm_users)) { add mm to a global list of expired mm_structs } }
void mmput_atomic_cleanup(void) { empty the global list of expired mm_structs and do cleanup stuff for each one }
Then you could call mmput_atomic() an arbitrary # of times in places where sleeping is not permitted, as long as mmput_atomic_cleanup() is later called in a place where sleeping is permissible. In the case of the OOM killer code, a call to mmput_atomic_cleanup() could be added to out_of_memory() in a place where we no longer hold tasklist_lock. Let me know if you have a preference for either of these options, or if you have other suggestions.
Thanks, Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |