Messages in this thread | | | Date | Mon, 6 Oct 2008 12:20:37 +0200 | From | Louis Rilling <> | Subject | Re: [PATCH 26/76] tty: Add a kref count |
| |
On Sun, Oct 05, 2008 at 05:08:15PM +0100, Alan Cox wrote: > Introduce a kref to the tty structure and use it to protect the tty->signal > tty references. For now we don't introduce it for anything else.
[...]
> diff --git a/kernel/fork.c b/kernel/fork.c > index 7ce2ebe..30de644 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -802,6 +802,7 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk) > > sig->leader = 0; /* session leadership doesn't inherit */ > sig->tty_old_pgrp = NULL; > + sig->tty = NULL; > > sig->utime = sig->stime = sig->cutime = sig->cstime = cputime_zero; > sig->gtime = cputime_zero; > @@ -838,6 +839,7 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk) > void __cleanup_signal(struct signal_struct *sig) > { > exit_thread_group_keys(sig); > + tty_kref_put(sig->tty); > kmem_cache_free(signal_cachep, sig); > } > > @@ -1227,7 +1229,8 @@ static struct task_struct *copy_process(unsigned long clone_flags, > p->nsproxy->pid_ns->child_reaper = p; > > p->signal->leader_pid = pid; > - p->signal->tty = current->signal->tty; > + tty_kref_put(p->signal->tty); > + p->signal->tty = tty_kref_get(current->signal->tty); > set_task_pgrp(p, task_pgrp_nr(current)); > set_task_session(p, task_session_nr(current)); > attach_pid(p, PIDTYPE_PGID, task_pgrp(current));
I'm a bit puzzled by this 'tty_kref_put(p->signal->tty)'. AFAICS, this is a noop since we are not in the CLONE_THREAD case and hence p->signal->tty == NULL. So, is it to make the code look more consistent? If so, is it worth adding extra code and cycles for this (I doubt that gcc is able to optimize this away)? What kind of future changes should this code protect against?
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 [unhandled content-type:application/pgp-signature] | |