Messages in this thread | | | Date | Thu, 17 Apr 2008 15:30:00 +0400 | From | Oleg Nesterov <> | Subject | Re: [Fwd: [-mm] Add an owner to the mm_struct (v9)] |
| |
On 04/17, Balbir Singh wrote: > > Oleg Nesterov wrote: > > > > I'd suggest to use ->real_parent though. And the third loop could be > > real_parent is for ptraced processes right?
No, please look at __ptrace_link(). ->real_parent is parent, ->parent is ptracer or it is equal to ->real_parent.
> > for_each_process(g) { > > c = g; > > do { > > if (!c->mm) > > continue; > > if (c->mm != mm) > > break; > > goto assign_new_owner; > > } while_each_thread(g, c); > > } > > > > I had this loop earlier (inspired from zap_threads()), is this loop more > efficient than what we have?
All sub-threads have the same ->mm. Once we see that c->mm != mm, we don't need to waste CPU iterating over the all other threads in the thread group.
> > Still. can't we make mm->mm_users_list ? > > I suspect that will be expensive to maintain. Specially with large number of > threads. I see a large space overhead and time overhead and additional > synchronization overhead.
Not sure... but I didn't really think about the implementation.
> Apart from finding the next owner is there any other > advantage?
it could be used by coredump.
OK, please forget. Even _if_ I am right, we can do this later.
Sadly, I don't have any time to read cgroup.c currently. Balbir, any chance you have the "for dummies" explanation what mm->owner is? I mean, I can't understand how it is possible that 2 CLONE_VM tasks are not equal wrt "ownering". When the old owner dies, we choose a random thread with the same mm. But we do nothing when the last user of ->mm dies. What is the point? (please feel free to ignore my q if it is not easy to explain).
Also, please let me remind,
> > + get_task_struct(c); > > + /* > > + * The task_lock protects c->mm from changing. > > + * We always want mm->owner->mm == mm > > + */ > > + task_lock(c); > > + /* > > + * Delay read_unlock() till we have the task_lock() > > + * to ensure that c does not slip away underneath us > > + */ > > + read_unlock(&tasklist_lock);
The commemt is misleading, tasklist_lock buys nothing and could be dropped right after get_task_struct(). tasklist can't prevent the task from exiting, but get_task_struct() pins task_struct, so it is safe to do task_lock() and re-check ->mm.
And we seem to have problems with use_mm(), no? Btw, what do you think about killing PF_BORROWED_MM ?
http://marc.info/?l=linux-kernel&m=120825843403378
Oleg.
| |