From owner-cvs-sys  Tue Aug  8 05:55:56 1995
Return-Path: cvs-sys-owner
Received: (from majordom@localhost)
          by freefall.cdrom.com (8.6.11/8.6.6) id FAA27316
          for cvs-sys-outgoing; Tue, 8 Aug 1995 05:55:56 -0700
Received: (from dyson@localhost)
          by freefall.cdrom.com (8.6.11/8.6.6) id FAA27304
          ; Tue, 8 Aug 1995 05:55:50 -0700
From: John Dyson <dyson>
Message-Id: <199508081255.FAA27304@freefall.cdrom.com>
Subject: Re: cvs commit: src/sys/i386/isa syscons.c
To: bde@zeta.org.au (Bruce Evans)
Date: Tue, 8 Aug 1995 05:55:50 -0700 (PDT)
Cc: CVS-commiters@freefall.cdrom.com, cvs-sys@freefall.cdrom.com
In-Reply-To: <199508081122.VAA04961@godzilla.zeta.org.au> from "Bruce Evans" at Aug 8, 95 09:22:21 pm
X-Mailer: ELM [version 2.4 PL24]
Content-Type: text
Content-Length: 1935      
Sender: cvs-sys-owner@freebsd.org
Precedence: bulk

> 
> >  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