Re: [Fwd: [-mm] Add an owner to the mm_struct (v9)]

[Oleg Nesterov]

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/