Messages in this thread | | | Date | Mon, 07 Nov 2005 13:57:51 +1100 | From | Nick Piggin <> | Subject | Re: [PATCH]: Clean up of __alloc_pages |
| |
Paul Jackson wrote: > Andi wrote: > >>>The current code in the kernel does the following: >>> 1) The cpuset_update_current_mems_allowed() calls in the >>> various alloc_page*() paths in mm/mempolicy.c: >>> * take the task_lock spinlock on the current task >> >>That needs to go imho. > > > The comment for refresh_mems(), where this is happening, explains > why this lock is needed: > > * The task_lock() is required to dereference current->cpuset safely. > * Without it, we could pick up the pointer value of current->cpuset > * in one instruction, and then attach_task could give us a different > * cpuset, and then the cpuset we had could be removed and freed, > * and then on our next instruction, we could dereference a no longer > * valid cpuset pointer to get its mems_generation field. > > Hmmm ... on second thought ... damn ... you're right. > > I can just flat out remove that task_lock - without penalty. > > It's *OK* if I dereference a no longer valid cpuset pointer to get > its (used to be) mems_generation field. Either that field will have > already changed, or it won't. >
I don't think so because if the cpuset can be freed, then its page might be unmapped from the kernel address space if use-after-free debugging is turned on. And this is a use after free :)
Also, it may be reused for something else far into the future without having its value changed - is this OK?
Anyway, I think the first problem is a showstopper. I'd look into Hugh's SLAB_DESTROY_BY_RCU for this, which sounds like a good fit if you need to go down this path (although I only had a quick skim over the cpusets code).
-- SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com
- 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/
| |