Date: Tue, 8 Aug 1995 05:55:50 -0700 (PDT) From: John Dyson <dyson> To: bde@zeta.org.au (Bruce Evans) Cc: CVS-commiters@freefall.cdrom.com, cvs-sys@freefall.cdrom.com Subject: Re: cvs commit: src/sys/i386/isa syscons.c Message-ID: <199508081255.FAA27304@freefall.cdrom.com> In-Reply-To: <199508081122.VAA04961@godzilla.zeta.org.au> from "Bruce Evans" at Aug 8, 95 09:22:21 pm
next in thread | previous in thread | raw e-mail | index | archive | help
> > > Modified: sys/i386/isa syscons.c > > Log: > > Fixed a problem that malloc(..,..,M_NOWAIT) was being called without checking > > for return values. It just so happens that in the cases where it is likely > > to fail, it is okay to change the M_NOWAIT to M_WAITOK -- and all will > > be well. This problem was manfest as a panic very regularly on a 4MB > > system right after bootup. > > Actually it isn't really OK to simply substitute M_NOWAIT with M_WAITOK. > If one of the malloc()s in scioctl() sleeps, then another process may > run and use the half-allocated resources. If one of the malloc()s in in > scioctl() or scopen() sleeps, then another process may run and repeat the > ioctl and (at best) allocate the resources twice. > You are right -- I'll fix it soon, but at least the system should not crash now :-). At least in the case of a failure we dont create a NULL pointer to set syscons up as a time-bomb.. I was recently *very embarassed* in front of one of my customers because of this bug :-(. I had 3/4 boot failures until these changes. > I think M_WAITOK is no good for general use. Using it safely seems to > _always_ require fairly tricky locking like we recently added to > ffs_vget(). If there is any chance that a subroutine of a syscall calls > malloc(..., M_WAIT_OK), then must be done to prevent the syscall being > reentered or to support reentrancy, and all resource that might be used > or change while malloc() is sleeping have to be locked down and/or > checked after waking up. > > The unified buffer cache probably makes these races more common. > It is very tricky to get the allocations correct, but the merged cache code does lock vnodes and check memory allocations. Additionally, looking at the vfs_bio it does M_NOWAIT (or equiv vm_page_alloc). (In fact, I have a deadlock free nfs_bio locking mechanism now, that I am experimenting with.) John dyson@root.com
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199508081255.FAA27304>