[RFC][PATCH] Cleanup the new thread's creation
xemul at openvz.org
Sun Aug 26 23:43:24 PDT 2007
Oleg Nesterov wrote:
> On 08/24, Pavel Emelyanov wrote:
>> The major differences of creating a new thread from creating a
>> new process is that
>> 1. newbie's tgid is set to leader's
>> 2. newbie's leader is set to leader
>> 3. newbie is added to leader's thread_list
> (Surely, the are many other major differences, but from the pids virtualization
> POV - yes ;)
>> +static void setup_new_thread(struct task_struct *thr, struct task_struct
>> + thr->tgid = leader->tgid;
>> + thr->group_leader = leader;
>> + list_add_tail_rcu(&thr->thread_group, &leader->thread_group);
> Imho, this name is a bit "too generic". Not that I can suggest something
> better... copy_sub_thread/copy_group_leader ?
>> @@ -1147,9 +1161,6 @@ static struct task_struct *copy_process(
>> p->pid = pid_nr(pid);
>> - p->tgid = p->pid;
>> - if (clone_flags & CLONE_THREAD)
>> - p->tgid = current->tgid;
> I agree, it is absoulutely not clear why should we set ->tgid here, and it
> would be nice to consolidate "if (CLONE_THREAD)" checks, but do we really
> need the helpers above? There are very simple, and have the only one caller.
> Sometimes it is good to see what's going on without pressing C-]
> Not that I against this patch, just I'm not sure it really simplifies things.
> Perhaps I missed something else you have in mind.
Me too, but while cleaning up the pid_t usage over the kernel I found
this place to be one of the most difficult from "how to make it better"
point of view. We need to hide the pid/tgid explicit usage somehow, but
the problem is that pid and tgid are set in this place and de_thread()
only and making helpers like set_task_tgid() doesn't sound reasonable.
More information about the Containers