From owner-cvs-all@FreeBSD.ORG Fri Sep 12 15:36:27 2008 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 32A011065678; Fri, 12 Sep 2008 15:36:27 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.terabit.net.ua (mail.terabit.net.ua [195.137.202.147]) by mx1.freebsd.org (Postfix) with ESMTP id C71E78FC14; Fri, 12 Sep 2008 15:36:26 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from skuns.zoral.com.ua ([91.193.166.194] helo=mail.zoral.com.ua) by mail.terabit.net.ua with esmtp (Exim 4.63 (FreeBSD)) (envelope-from ) id 1KeAh6-0003wh-BO; Fri, 12 Sep 2008 18:36:24 +0300 Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id m8CFaKMB028481 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 12 Sep 2008 18:36:20 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.2/8.14.2) with ESMTP id m8CFaKuM026440; Fri, 12 Sep 2008 18:36:20 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.3/8.14.3/Submit) id m8CFaJS2026439; Fri, 12 Sep 2008 18:36:19 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 12 Sep 2008 18:36:19 +0300 From: Kostik Belousov To: John Baldwin Message-ID: <20080912153619.GK39652@deviant.kiev.zoral.com.ua> References: <200809120951.m8C9pOZj037333@repoman.freebsd.org> <200809121022.36441.jhb@freebsd.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Bina0ufSB9dLMnVr" Content-Disposition: inline In-Reply-To: <200809121022.36441.jhb@freebsd.org> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: ClamAV version 0.93.3, clamav-milter version 0.93.3 on skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua X-Virus-Scanned: mail.terabit.net.ua 1KeAh6-0003wh-BO c6d050357d18032cb6a55571583aaba1 X-Terabit: YES Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/i386/i386 sys_machdep.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Sep 2008 15:36:27 -0000 --Bina0ufSB9dLMnVr Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 12, 2008 at 10:22:35AM -0400, John Baldwin wrote: > On Friday 12 September 2008 05:51:11 am Konstantin Belousov wrote: > > kib 2008-09-12 09:51:11 UTC > >=20 > > FreeBSD src repository > >=20 > > Modified files: > > sys/i386/i386 sys_machdep.c=20 > > Log: > > SVN rev 182960 on 2008-09-12 09:51:11Z by kib > > =20 > > The user_ldt_alloc() function shall return with dt_lock locked. > > The user_ldt_free() function shall return with dt_lock unlocked. > > Error handling code in both functions do not handle this, fix it by > > doing necessary lock/unlock. > > =20 > > While there, fix minor style nits. >=20 > Hmm, I had actually thought it was intentional for user_ldt_alloc() to on= ly=20 > return with the lock held on success and depend on a later call to anothe= r=20 > method to drop the lock in the success case (so the locking isn't visible= to=20 > consumers of the API in theory). For example, i386_ldt_grow() depended o= n=20 > this feature and is now broken (it leaks a lock on failure). I missed th= is=20 > when looking at this yesterday. I probably miss something there. On failure of user_ldt_alloc(), i386_ldt_grow() does return (ENOMEM), without changing lock state for dt_lock. There are three call locations for the i386_ldt_grow(), all of them in i386_set_ldt(). On failure, each call location does mtx_unlock_spin(&dt_lock) immediately after call. So I assumed that protocol for i386_ldt_grow() is to always return with dt_lock locked. Two other callers of the user_ldt_alloc() in cpu_fork() do panic() immediately after the failed call to user_ldt_alloc(). Could you, please, point me to exact place where the lock would leak ? >=20 > Other notes: >=20 > - Since user_ldt_free() handles the case of there not being an LDT, the c= ode > in exec_setregs() on i386 can be simplified to just always call > user_ldt_free(). > - cpu_exit() could possibly do the same. I wonder if exec_setregs() need= s the > same fixup to %gs that cpu_exit() does. If so, that could possibly be = moved > into user_ldt_free(). Ah, exec_setregs() does it unconditionally. I t= hink > you could make cpu_exit() just do it unconditionally as well before cal= ling > user_ldt_free(). >=20 > --=20 > John Baldwin --Bina0ufSB9dLMnVr Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (FreeBSD) iEYEARECAAYFAkjKjHMACgkQC3+MBN1Mb4jFMwCeKwnd9Sw/+0yRnxck35kzBolg E7QAoMEQijLNzhGJBW4IlRsc0MAbfKv0 =Atvt -----END PGP SIGNATURE----- --Bina0ufSB9dLMnVr--