From owner-freebsd-current@FreeBSD.ORG Sat Jul 8 06:40:43 2006 Return-Path: X-Original-To: freebsd-current@freebsd.org Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id EE3FD16A4DA; Sat, 8 Jul 2006 06:40:43 +0000 (UTC) (envelope-from mb@imp.ch) Received: from smtp.imp.ch (mx2.imp.ch [157.161.9.17]) by mx1.FreeBSD.org (Postfix) with ESMTP id 679F243D45; Sat, 8 Jul 2006 06:40:42 +0000 (GMT) (envelope-from mb@imp.ch) Received: from godot.imp.ch (godot.imp.ch [157.161.4.8]) by smtp.imp.ch (8.13.7/8.13.7/Submit) with ESMTP id k686ecSd017359; Sat, 8 Jul 2006 08:40:40 +0200 (CEST) (envelope-from mb@imp.ch) Date: Sat, 8 Jul 2006 08:40:38 +0200 (CEST) From: Martin Blapp To: freebsd-current@freebsd.org In-Reply-To: <200607072236.55319.jhb@freebsd.org> Message-ID: <20060708081419.W14714@godot.imp.ch> References: <20060708020700.A14714@godot.imp.ch> <200607072236.55319.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Scanned-By: MIMEDefang 2.57 on 157.161.9.65 Cc: Subject: Re: Please help reviewing ! Got another crash because of the ttymodem() / ttyclose() race X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 08 Jul 2006 06:40:44 -0000 Hi, > Yes.. did you see my reply on the mailing list Ouch ? No I missed that ! Thanks. I just read it. > I'm not sure that the proctree_lock should protect t_pgrp as that is part > of the tty structure, not process groups or session structures. Hmm ? In /usr/src/sys/kern/tty.c:1215 we copy in ttioctl() the pointers of p->p_session and p->p_pgrp to the tty structure. 1215 tp->t_session = p->p_session; 1216 tp->t_pgrp = p->p_pgrp; 1217 SESS_LOCK(p->p_session); It looks to me that we have to protect it with the locks of the process groups or session structures since its only a copy of the pointer, and not an exact copy. Else I wouldn't had any crashes - this is the mess. Or do I understand something completly wrong ? >I think probably it should be protected by Giant for now until the tty subsystem >is locked. Also, the ttyinfo() part will not work since it tries to >acquire a mutex (PGRP_LOCK()) while holding a spin mutex (sched_lock). Ok, fixed. Does it look better now ? Btw. this version is now running for three days without any crashes on four SMP boxes. Rocking stable so far. >perhaps instead we need to expand Giant to cover it until the tty subsystem is >locked? How would you do that ? Martin