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>