Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 15 Jan 2001 23:32:33 -0800 (PST)
From:      John Baldwin <jhb@FreeBSD.org>
To:        Alfred Perlstein <bright@wintelcom.net>
Cc:        Bosko Milekic <bmilekic@FreeBSD.org>, cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/sys/kern uipc_mbuf.c
Message-ID:  <XFMail.010115233233.jhb@FreeBSD.org>
In-Reply-To: <20010115232251.Q7240@fw.wintelcom.net>

next in thread | previous in thread | raw e-mail | index | archive | help

On 16-Jan-01 Alfred Perlstein wrote:
> * John Baldwin <jhb@FreeBSD.org> [010115 23:08] wrote:
>> 
>> On 16-Jan-01 Alfred Perlstein wrote:
>> > * Bosko Milekic <bmilekic@FreeBSD.org> [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.

??  I need some context. :)  I had one lockup today involving some kind of
deadlock in the lockmgr (*blam* *blam* x 12 to the lockmgr) and Jake had
several panic's with the preemptive kernel stuff probably.  That and we mumbled
on about guaranteeing that Giant is always first in the lock order.  Other than
that I'm not sure what you are recalling.

-- 

John Baldwin <jhb@FreeBSD.org> -- 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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.010115233233.jhb>