Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 20 Sep 2006 11:09:12 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        Martin Blapp <mb@imp.ch>
Cc:        cvs-src@freebsd.org, Martin Blapp <mbr@freebsd.org>, src-committers@freebsd.org, cvs-all@freebsd.org
Subject:   Re: cvs commit: src/sys/kern kern_proc.c
Message-ID:  <200609201109.13271.jhb@freebsd.org>
In-Reply-To: <20060920012110.P1494@godot.imp.ch>
References:  <200609191925.k8JJPBaH091145@repoman.freebsd.org> <200609191714.46864.jhb@freebsd.org> <20060920012110.P1494@godot.imp.ch>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200609201109.13271.jhb>