From owner-cvs-all Mon Jan 15 23: 8:40 2001 Delivered-To: cvs-all@freebsd.org Received: from moby.geekhouse.net (moby.geekhouse.net [64.81.6.36]) by hub.freebsd.org (Postfix) with ESMTP id A144037B400; Mon, 15 Jan 2001 23:08:16 -0800 (PST) Received: from laptop.baldwin.cx (john@dhcp150.geekhouse.net [192.168.1.150]) by moby.geekhouse.net (8.11.0/8.9.3) with ESMTP id f0G7C3s53094; Mon, 15 Jan 2001 23:12:03 -0800 (PST) (envelope-from jhb@FreeBSD.org) Message-ID: X-Mailer: XFMail 1.4.0 on FreeBSD X-Priority: 3 (Normal) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit MIME-Version: 1.0 In-Reply-To: <20010115224043.M7240@fw.wintelcom.net> Date: Mon, 15 Jan 2001 23:08:22 -0800 (PST) From: John Baldwin To: Alfred Perlstein Subject: Re: cvs commit: src/sys/kern uipc_mbuf.c Cc: cvs-all@FreeBSD.org, cvs-committers@FreeBSD.org, Bosko Milekic Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG 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. -- John Baldwin -- http://www.FreeBSD.org/~jhb/ PGP Key: http://www.baldwin.cx/~john/pgpkey.asc "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/ To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message