On Tue, 2010-06-08 at 11:05 -0400, Chas Williams (CONTRACTOR) wrote:

In message [ref],David Woodhouse 
writes:

quoted

On Mon, 2010-06-07 at 12:37 -0400, Chas Williams (CONTRACTOR) wrote:

quoted

i dont understand.  if you do a sock_hold() in find_vcc(), and then call
vcc->push() you should be able to call vcc->push() and then sock_put(). 

Holding the reference doesn't stop the problem. The problem is

vcc_release()
--> vcc_destroy_socket()
  --> br2684_push(vcc, NULL)
        sets vcc->user_back = NULL
        (which it what causes the oops when try try to feed it any
         subsequent packets).

Only _later_ does vcc_release() call sock_put().

hmm... perhaps this routine needs to take the vcc_sklist_lock because
it is going to modify the vcc.  or we need to use locking on the vcc
itself. 

Or move the ->push(vcc, NULL) and anything else which destroys the
state, so that it happens later. Use a real socket destructor function
which will be called from sk_free() after the last sock_put().

you took a reference to an object inside a hashed list and didnt do
anything to prevent the object from leaving the hashed list. that is
stil not correct IMHO.

Yeah yeah, but I fixed that already with the RCU-like approach of
synchronising with the tasklet on dev->ops->close(). So I don't _need_
the reference.

-- 
dwmw2