From owner-cvs-all@FreeBSD.ORG Fri Sep 12 20:21:14 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 4FF0B1065673; Fri, 12 Sep 2008 20:21:14 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 933388FC1A; Fri, 12 Sep 2008 20:21:13 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from localhost.corp.yahoo.com (john@localhost [IPv6:::1]) (authenticated bits=0) by server.baldwin.cx (8.14.2/8.14.2) with ESMTP id m8CKL0Q1049591; Fri, 12 Sep 2008 16:21:06 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: Kostik Belousov Date: Fri, 12 Sep 2008 13:35:21 -0400 User-Agent: KMail/1.9.7 References: <200809120951.m8C9pOZj037333@repoman.freebsd.org> <200809121022.36441.jhb@freebsd.org> <20080912153619.GK39652@deviant.kiev.zoral.com.ua> In-Reply-To: <20080912153619.GK39652@deviant.kiev.zoral.com.ua> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200809121335.22070.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [IPv6:::1]); Fri, 12 Sep 2008 16:21:06 -0400 (EDT) X-Virus-Scanned: ClamAV 0.93.1/8227/Fri Sep 12 07:48:22 2008 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.6 required=4.2 tests=AWL,BAYES_00,NO_RELAYS autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx 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 20:21:14 -0000 On Friday 12 September 2008 11:36:19 am Kostik Belousov wrote: > 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 > > > > > > FreeBSD src repository > > > > > > Modified files: > > > sys/i386/i386 sys_machdep.c > > > Log: > > > SVN rev 182960 on 2008-09-12 09:51:11Z by kib > > > > > > 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. > > > > > > While there, fix minor style nits. > > > > Hmm, I had actually thought it was intentional for user_ldt_alloc() to only > > return with the lock held on success and depend on a later call to another > > method to drop the lock in the success case (so the locking isn't visible to > > consumers of the API in theory). For example, i386_ldt_grow() depended on > > this feature and is now broken (it leaks a lock on failure). I missed this > > 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 ? Gah, sorry, I had missed the callers of i386_ldt_grow(). I had recalled it being purposeful at one point that user_ldt_alloc() had this behavior. -- John Baldwin