cvs commit: src/sys/vm vm_fault.c

[Alan Cox]

On Sun, Aug 22, 2004 at 01:35:35AM -0400, Brian Fundakowski Feldman wrote:
> On Sun, Aug 22, 2004 at 12:27:31AM -0500, Alan Cox wrote:
> > On Sun, Aug 22, 2004 at 01:01:55AM -0400, Brian Fundakowski Feldman wrote:
> > > On Sat, Aug 21, 2004 at 11:49:36PM -0500, Alan Cox wrote:
> > > > On Sun, Aug 22, 2004 at 12:18:49AM -0400, Robert Watson wrote:
> > > > > 
> > > > > On Sun, 22 Aug 2004, Brian Fundakowski Feldman wrote:
> > > > > 
> > > > > > Also, it was the system_map lock, so it was a mutex, not an sx. 
> > > > > > 
> > > > > > /Goes back to trying to figure out wtf portupgrade -rR kde\* keeps
> > > > > > causing
> > > > > >  hangs, but only in X, not at the command line, and not seemingly
> > > > > >  dependant on AGP/X driver. 
> > > > > 
> > > > > I believe mutexes are alright also as long as you're careful about
> > > > > sleeping and lock order.  I.e., don't sleep while holding a
> > > > > non-funnel-like mutex, etc.
> > > > > 
> > > > 
> > > > We should never page fault on a system map.  If that happens to you,
> > > > it indicates an error.  (In-kernel maps on which we do page fault,
> > > > such as the pipe submap, are treated the same as user maps and utilize
> > > > an sx lock.)
> > > 
> > > vm_fault
> > > vm_fault_wire
> > > vm_map_wire
> > > kmem_alloc
> > > vm_ksubmap_init
> > > cpu_startup
> > > mi_startup
> > > 
> > 
> > This is a call to vm_fault(), but not a page fault.  This works
> > without error because kmem_alloc() has preallocated and wired all of
> > the required pages before calling vm_map_wire() (which calls
> > vm_fault()).  Specifically, the preallocation and wiring guarantees
> > that vm_fault() will not follow any of the code paths on which it
> > could sleep with the system map mutex held.  (In fact, I believe that
> > we are guaranteed that it will not sleep under any circumstances.)
> 
> I know, but that has nothing to do with the fact that it's locking the
> system map mutex, then Giant, which causes a lock order reversal.  Do
> you run WITNESS?  If you do, you should see this in your startup.
> 

Yes, I do.  And, there is no complaint from WITNESS.  Giant is already
held by the initialization code.  So, the acquisition of Giant inside
of vm_fault() is recursive and thus not a lock-order violation.  The
bug was, however, causing Giant to be acquired once and released twice
by vm_fault().  So, the first call to vm_fault() by vm_map_wire()
would release the initialization code's hold on Giant.  Once Giant
became unlocked, the next call to vm_fault() by vm_map_wire() would
tigger a lock-order reversal report.

Alan