Dave Hansen [haveblue at us.ibm.com] wrote: | On Thu, 2007-01-11 at 07:58 -0800, Sukadev Bhattiprolu wrote: | > =================================================================== | > --- lx26-20-rc2-mm1.orig/kernel/fork.c 2007-01-11 07:18:03.383853328 -0800 | > +++ lx26-20-rc2-mm1/kernel/fork.c 2007-01-11 07:19:55.550801360 -0800 | > @@ -1248,8 +1248,13 @@ static struct task_struct *copy_process( | > p->signal->tty = current->signal->tty; | > p->signal->pgrp = process_group(current); | > set_signal_session(p->signal, process_session(current)); | > - find_attach_pid(p, PIDTYPE_PGID, process_group(p)); | > - find_attach_pid(p, PIDTYPE_SID, process_session(p)); | > + if (current->pid) { | > + attach_pid(p, PIDTYPE_PGID, task_pgrp(current)); | > + attach_pid(p, PIDTYPE_SID, task_session(current)); | > + } else { | > + find_attach_pid(p, PIDTYPE_PGID, process_group(current)); | > + find_attach_pid(p, PIDTYPE_SID, process_session(current)); | > + } | > | > list_add_tail_rcu(&p->tasks, &init_task.tasks); | > __get_cpu_var(process_counts)++; | | I know I've asked this before (and I know I'm going to ask it again), | but why do we need both task_pgrp() and process_group() to both have | similar-sounding names and both take the same kind of argument? :) This | stuff _really_ needs to get cleaned up. It makes reviewing these | patches much harder. We are phasing out process_group(), process_session() which return a pid_t. I guess it also points to not having a special case for swapper. | | In general, you should keep the hacks (which this is) to boot and | init-time stuff. If you can initialize a structure so that it plays | nicely for the rest of its life, do that. Don't put special cases in | common code that everybody will have to look at. | | > Since task_pid() task_pgrp(), task_session() for the swapper are NULL, I | > had to treat swapper as special in this patch and would like some comments. | | Can you do some research and find out _why_ these are NULL, and why they | need to be kept NULL? task_struct for swapper is initialized by hand (INIT_TASK, INIT_SIGNALS etc) but no struct pid is ever allocated and attached to the swapper. This is normally done in copy_process() and so is done for all other processes starting with pid_t = 1 (/sbin/init). I am trying to understand if there is a history to it and if they need to be kept NULL. | | -- Dave