Re: [PATCH 26/76] tty: Add a kref count

[Louis Rilling]

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

Attachment: signature.asc
Description: Digital signature