[Devel] Re: [PATCH 5/6] Makes procs file writable to move all threads by tgid at once

[Louis Rilling]

On 28/07/09 17:23 -0700, Benjamin Blum wrote:
> On Fri, Jul 24, 2009 at 2:52 PM, Benjamin Blum<bblum at google.com> wrote:
> > On Fri, Jul 24, 2009 at 3:02 AM, Louis Rilling<Louis.Rilling at kerlabs.com> wrote:
> >> On 23/07/09 20:22 -0700, Ben Blum wrote:
> >>> @@ -408,6 +409,15 @@ You can attach the current shell task by echoing 0:
> >>>
> >>>  # echo 0 > tasks
> >>>
> >>> +The cgroup.procs file is useful for managing all tasks in a threadgroup at
> >>> +once. It works the same way as the tasks file, but moves all tasks in the
> >>> +threadgroup with the specified tgid.
> >>> +
> >>> +Writing the pid of a task that's not the threadgroup leader (i.e., a pid
> >>> +that isn't a tgid) is treated as invalid. Writing a '0' to cgroup.procs will
> >>> +attach the writing task and all tasks in its threadgroup, but is invalid if
> >>> +the writing task is not the leader of the threadgroup.
> >>> +
> >>
> >> This restriction sounds unfortunate and I'm not sure that there are good reasons
> >> for it (see below).
> >
> > Would it be preferable to allow any thread in a threadgroup to move
> > the whole group to a new cgroup? Would that undermine the desired
> > restriction of the tasks file in which a thread needs to have
> > sufficient perms to move a thread that's not itself, or is the task
> > file's restriction there undesirable in the case of threadgroups?
> 
> Still wondering on what to do about this. Noting that this restriction
> approximately parallels the behaviour you'd get from using the tasks
> file, I think it'd be safer to leave it like this then possibly in the
> future lift the restriction, rather than make it currently lifted then
> decide in the future that we want to re-impose it.

Sorry, I was on vacations last week.

To me it looks like all threads should be treated equally, since AFAIK POSIX has
no notion of thread group leader. Moreover, a thread group (process) can live an
arbitrary long time after the death of its leader (that remains as a zombie), so
this restriction just prevents such processes to user the procs file.

> 
> >>> +      * to move all tasks to the new cgroup. the only fail case henceforth
> >>> +      * is if the threadgroup leader has PF_EXITING set (in which case all
> >>> +      * the other threads get killed) - if other threads happen to be
> >>
> >> This statement is wrong. A thread group leader can have PF_EXITING (and even
> >> become zombie) while other sub-threads continue their execution. For instance it
> >> is perfectly valid for a thread group leader to call sys_exit(), and no other
> >> thread will be affected.
> >
> > Is that the case? Then, what becomes of his task_struct when he's
> > gone, since presumably all the sub-threads will need it. Does it just
> > hang around?

Yes. The thread leader remains in zombie state (and cannot be reaped) until all
other threads have exited.

> >
> > If so, would the desired behaviour be to proceed with moving all
> > sub-threads if the leader is exiting? (That's not a very big change
> > code-wise)
> >
> 
> I've fixed this, and also implemented the rwsem as threadgroup-local.
> If there's no more discussion on this version of the patch, I'll send
> out the revised series soon.

Ok. Thanks.

Louis

-- 
Dr Louis Rilling			Kerlabs
Skype: louis.rilling			Batiment Germanium
Phone: (+33|0) 6 80 89 08 23		80 avenue des Buttes de Coesmes
http://www.kerlabs.com/			35700 Rennes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.openvz.org/pipermail/devel/attachments/20090803/a655e240/attachment-0001.sig>
-------------- next part --------------
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers