On Mon, Aug 05, 2002 at 03:22:44AM +0200, Frank van der Linden wrote:
> Greg Oster sent me the following trace of a LOCKDEBUG warning regarding
> holding a spinlock while (possibly) going to sleep:
> 
<snip>
> 
> The first pool_get (from getnewvnode()) is PR_NOWAIT, and it has to be,
> since it's holding a spinlock at that time.

It would be trivial to move the 'simple_lock(&vnode_free_list_slock)'
inside the 'else' part of the if.  That way pool_get wouldn't
be called with the spin lock held.
This would fix this case...

> A NOWAIT call of any kind
> into UVM shouldn't lead to the process sleeping, but in this case it might.
> 
> The problem is the (inline) function uvm_mapent_alloc in uvm_map.c. It
> always uses PR_WAITOK for non-static entries. Also, callers to
> uvm_mapent_alloc don't check the return value.

Yes - a horrid (inline) function!  That code ought to be able to pull
a 'free' map_ent of any of the pool free lists in order to satisfy
and call that can't block......
(but beware that it is very nearly a recursive loop...)
> 
> The quickest solution I see is to pass a WAIT/NOWAIT flag to
> uvm_mapent_alloc, have it use PR_NOWAIT/PR_WAITOK depending on that, and
> return NULL properly if it fails. Callers will always pass in WAIT,
> because that is the current default. For cases where a WAIT/NOWAIT flag
> is passed down from above, it can be used appropriately (uvm_map() is
> such a case), and the return value will be checked.

You also need a FAIL_OK flag....

Should the various nowait/waitok (etc) flags be unified throughout
the kernel.  I've spotted a few cases where one value has to be
converted to another.

	David

-- 
David Laight: david@l8s.co.uk