Date: Fri, 12 Sep 2008 13:35:21 -0400 From: John Baldwin <jhb@freebsd.org> To: Kostik Belousov <kostikbel@gmail.com> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/i386/i386 sys_machdep.c Message-ID: <200809121335.22070.jhb@freebsd.org> In-Reply-To: <20080912153619.GK39652@deviant.kiev.zoral.com.ua> References: <200809120951.m8C9pOZj037333@repoman.freebsd.org> <200809121022.36441.jhb@freebsd.org> <20080912153619.GK39652@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200809121335.22070.jhb>