From owner-cvs-all Mon Jan 15 23:23:18 2001 Delivered-To: cvs-all@freebsd.org Received: from fw.wintelcom.net (ns1.wintelcom.net [209.1.153.20]) by hub.freebsd.org (Postfix) with ESMTP id C3BA137B400; Mon, 15 Jan 2001 23:22:51 -0800 (PST) Received: (from bright@localhost) by fw.wintelcom.net (8.10.0/8.10.0) id f0G7MpZ01973; Mon, 15 Jan 2001 23:22:51 -0800 (PST) Date: Mon, 15 Jan 2001 23:22:51 -0800 From: Alfred Perlstein To: John Baldwin Cc: cvs-all@FreeBSD.org, cvs-committers@FreeBSD.org, Bosko Milekic Subject: Re: cvs commit: src/sys/kern uipc_mbuf.c Message-ID: <20010115232251.Q7240@fw.wintelcom.net> References: <20010115224043.M7240@fw.wintelcom.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: ; from jhb@FreeBSD.org on Mon, Jan 15, 2001 at 11:08:22PM -0800 Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG * John Baldwin [010115 23:08] wrote: > > On 16-Jan-01 Alfred Perlstein wrote: > > * Bosko Milekic [010115 17:53] wrote: > >> bmilekic 2001/01/15 17:53:13 PST > >> > >> Modified files: > >> sys/kern uipc_mbuf.c > >> Log: > >> Add some KASSERTs valid if WITNESS is defined to verify that the mbuf > >> allocation routines are being called safely. Since we drop our relevant > >> mbuf mutex and acquire Giant before we call kmem_malloc(), we have > >> to make sure that this does not pave the way for a fatal lock order > >> reversal. Check that either Giant is already held (in which case it's safe > >> to grab it again and recurse on it) or, if Giant is not held, that no > >> other locks are held before we try to acquire Giant. > >> > >> Similarily, add a KASSERT valid in the WITNESS case in m_reclaim() to > >> nail callers who end up in m_reclaim() and hold a lock. > > > > I think I may be missing something obvious here, but I'd like to > > understand what's going on here better. > > > > Doesn't this just guarantee a panic if we need to kmem_alloc while > > the driver lock is held and WITNESS is enabled? I don't see how > > this prevents/fixes anything ATM, if kmem_alloc is called (hence > > requiring Giant) we either panic, or have a chance of deadlocking. > > > > One of the ways to fix this would be to identify the entry points > > from the top half of the kernel and surround the function bodies > > with DROP_GIANT (ick) for the time being in order to fix the lock > > order before aquiring the driver lock. > > Right now all network drivers are not MP safe, so they all get Giant first > thing, so you won't hit this. Same for all syscalls (or all the ones hitting > network code at least) so this will always be a recursion. When drivers are > marked as MP safe, then the locking will have to be fixed such that they don't > call into mbuf allocation with their lock held if they can help it. Ok, then what were the panic's you and Jake were discussing on IRC related to? I thought it was directly related to the out of order locking on the driverlock. -- -Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org] "I have the heart of a child; I keep it in a jar on my desk." To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message