cvs commit: src/sys/kern kern_proc.c

[John Baldwin]

On Tuesday 19 September 2006 19:29, Martin Blapp wrote:
> 
> Hi,
> 
> > Will you be able to revert 1.258 of tty.c now and still be safe from panics?
> 
> I guess so. I don't see another place which could be dangerous for us beside
> enterpgrp(). I don't understand the code there 100%.
> 
> sys/kern/kern_proc.c: line 308
> 
>          if (sess != NULL) {
>                   ^^^^^^
> Why only if a session already exists ? Could you explain that to me ?
> Anyway, we may need here Giant too. What do you think ?

This is because the callers pre-allocate session and process group objects
and then enterpgrp() initializes them and glues them together IIRC.

>                  /*
>                   * new session
>                   */
>                  mtx_init(&sess->s_mtx, "session", NULL, MTX_DEF);
>                  PROC_LOCK(p);
>                  p->p_flag &= ~P_CONTROLT;
>                  PROC_UNLOCK(p);
>                  PGRP_LOCK(pgrp);
>                  sess->s_leader = p;
>                  sess->s_sid = p->p_pid;
>                  sess->s_count = 1;
>                  sess->s_ttyvp = NULL;
>                  sess->s_ttyp = NULL;
> 
> 
> But I don't think we should revert v. 1.258 because the locks will
> be needed later. After NEED_GIANT has gone (from tty code) we will need the 
> same lock at the same place again to avoid races. There are some places more where we 
> need the locks then. I propose to keep rev. 1.258 (maybe not to MFC it) or to 
> add a comment instead.

Well, I'd rather use whatever lock we end up using for t_session instead
of assuming it's going to be proctree_lock, so I'd like to leave t_session
only under Giant for now until we really know what we are doing.

-- 
John Baldwin