[ofw] [PATCH 3/7] ib/cm: export CM only interface

[Fab Tillier]

> Trying to trace through QP destruction, I think this is the code path
> that's followed:
>
> invokes qp->pfn_destroy()
> maps to async_destroy_obj()
> calls destroy_obj()
> this changes the QP state:
>         lock
>         state = destroying
>         unlock
> also calls qp->pfn_destroying
> maps to destroying_qp()
> calls al_destroy_cep()
>
> Finally everything unwinds, and the user's destroy callback is invoked
> after the reference count drops to zero.

Yep, that's pretty much right.

> Note that no lock is held between setting the QP state to destroying
> and calling al_destroy_cep().  This means that another thread can
> reference the QP before it is in the destroying state, the destroy
> thread can execute, then the QP's CID can be set.

The QP's handle is marked invalid before the destroying callback is called, so any IOCTLs that come between these two will fail.  If an IOCTL is already in progress it will have a reference on the QP.

Note also that the al_destroy_cep call in the destroying_qp marks the CID as not reusable (AL_RESERVED_CID), so any future assignments should fail, and any previous assignment cleaned up.

>  I think the
> synchronization that I mentioned in my previous mail handles this, and
> I'm not seeing a simpler solution at this point.  (I.e. Fixing just
> al_destroy_cep won't work.)

If you fix just al_destroy_cep you would be OK, but you don't resolve the lack of clarity in using the CEP's lock to protect the CID member of the QP.  You can change that, though I don't know if there are any potential pitfalls in holding the QP's lock when making CEP manager calls - you have to make sure when you destroy the CEP that the CEP's destroy callback doesn't try to take the same lock.  You might be able to just hold the lock after making the CEP calls, only long enough to check/set the CID member.  Or you might be able to work things out with interlocked operations.

-Fab