Date: Wed, 13 Nov 1996 20:49:56 -0700 (MST) From: Terry Lambert <terry@lambert.org> To: dg@root.com Cc: terry@lambert.org, michaelh@cet.co.jp, ponds!rivers@dg-rtp.dg.com, Hackers@FreeBSD.org Subject: Re: Even more info on daily panics... Message-ID: <199611140349.UAA23432@phaeton.artisoft.com> In-Reply-To: <199611140255.SAA09566@root.com> from "David Greenman" at Nov 13, 96 06:55:53 pm
next in thread | previous in thread | raw e-mail | index | archive | help
> Terry, the problem has nothing to do with functional abstractions, > automatons, layering errors, execution contexts, interface boundries, > race conditions, or little green men from Alpha Centauri. > Vnodes on the free list are not allowed to have non-zero v_usecount's. That's not what the code says. The code inserts them on list wrap. > Vnodes can not be used without first gaining a reference (v_usecount++). > Vnodes are removed from the freelist when this happens. I noticed last > night that there is a v_usecount++ in the vnode_pager that could cause > the problem that David is reporting if an object allocation was attempted > for a vnode without first gaining a reference to it. We've had bugs like > this before, so it should come as no surprise. > David, please apply the attached patch and see if your system trips > over it. Thanks. This is a "horse out of the barn" 'fix'. It will cause a panic, but the stack trace will *still* not include the cause of the panic in the call list. The problem is clearly a bogus list insertion. Equally as clearly, a bogus list insertion in possible in the current code because of the lock race potentially causing a list wrap during the allocation to extend sleeping through a valid vrele. This is possible because the lock is not a lock on list access, it is a lock across interface access... the lock is held in vclean above the VOP layer and in valloc below the VOP layer. The *correct* soloution is to panic the bogus list insertion at the time of the insertion attempt (in vrele) so that the stack reflects who is responsible for the bogus insertion. With an obviously bad parameter, it is apparent where the parameter orginated... from the insertion/list expansion race. It is a bogus layering abstraction that permits such code to be written in the first place. If the abstraction were correct, the race window would not exist. Bluntly: a panic occurs, therefore the code which is the distal cause of the panic is erroneous. Under no circumstances, short of real hardware failure, should it be possible to panic the system. If you look at the BSD4.4-Lite2 code, you will see they cleaned this up (using a fix just as kludgy as my one liner) by using kern_lock.c functions instead of smearing the lock state. Their error is in keeping the calldown interface, which leaves a potential for a deadly embrace deadlock across the interface boundry (at least after you factor in the cache unification -- the BSD4.4-Lite2 code can't itself suffer from this deadlock because it lacks the necessary concurrency). Going to a veto interface would fix this problem by unwinding the EWOULDBLOCK state by returning up the stack instead of panic'ing. Put in a retry loop, and your not as elegant as a computation of transitive closure, but at least you don't panic or lock up the damn machine when you hit a preventable resource conflict. Regards, Terry Lambert terry@lambert.org --- Any opinions in this posting are my own and not those of my present or previous employers.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199611140349.UAA23432>