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.
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?
-- Dave
