On Tue, Nov 28, 2006 at 12:47:32PM -0800, Bill Studenmund wrote:
> The patch does two things. The main thing it does is change the vget() in 
> layer_node_find() to a) not perform a lockmgr() op (I didn't know you 
> could do this, but it's exactly what I want) and b) set LK_NOWAIT. This 
> way, we either pull the vnode off the free list or we get an error 
> indicating that the node's being recycled.
> 
> The other thing the patch does is add layer_node_find1(), which now
> contains all the code. The caller must hold lmp->layerm_hashlock
> explicitly. The main reason for this is so layer_node_alloc() can hold it
> the whole time. This way we close a window in which, if the locking
> protocol were ever changed, two different upper nodes could be added (in
> the same layer) above one given lower node. Since we want to change the 
> locking protocol in the future, I want to make this change. Also, it makes 
> some of the cleaning case issues clear in my head. :-)

your patch allows multiple instances of the same vnode in the hash
if all but at most one of them is being recycled.  I don't think
that's a very good idea, I think we should maintain the usual rule
that there cannot be duplicates.  to fix the MP locking in this case
we just need to make sure that if layer_node_find() returns NULL then
we hold the hashlock from the time we started looking through the hash
bucket until after we add the new node to the hash bucket.  if
layer_node_find() returns a valid vp, then it's ok that we dropped the
hashlock while calling vget() on it.

-Chuck