[Openib-windows] races on __destrot_obj function

[Yossi Leybovich]

> -----Original Message-----
> From: ftillier.sst at gmail.com [mailto:ftillier.sst at gmail.com] 
> On Behalf Of Fabian Tillier
> Sent: Tuesday, July 11, 2006 6:16 PM
> To: Yossi Leybovich
> Cc: openib-windows at openib.org
> Subject: Re: [Openib-windows] races on __destrot_obj function
> 
> Hi Yossi,
> 
> On 7/11/06, Yossi Leybovich <sleybo at mellanox.co.il> wrote:
> > Attached is patch that solve this problem
> >
> > It includes:
> >
> > 1. fix to cl_obj to return value if the parent port is not in 
> > CL_INITIALIZED
> 
> I think this should be an assertion - the client should be 
> able to check its state before trying to make this call - the 
> assertion would trap improper usage.
> 

Why just ASSERT ? Look at the al_obj it uses similar mechanism but al
obj does not allowed to add child to parent obj that is not in
CL_INITIALIZED state.
Why we need to remember to check the state in all passes of child
creation let the code do that for us (same as attched_al..)

> A state check in ipoib_port_resume should be all that's 
> needed to prevent the MC join from being kicked off.

I want general solution not just to the ipoib_port, but to all clients
that use cl_obj (adapter,endpt).
This way we can keep resume similar to send so that the code try to send
but fail because the QP is in error state or because ethere is no endpt,
no need to check port/adapter state. 

> 
> > 2. fix to insert/insert_locked function to check return value
> 
> Same as above.

Even if we move the check from cl_obj
the client (ipoib) still need to check its state before trying to add
itself to the father list.
So insert/insert_lock might fail so they should be change to return
value.

> 
> > 3. add ref count mechanism to help debug ref count problems
> 
> This is good.
> 
> > 4. not call port_resume in endpt cleanup but in port destroy
> 
> I think this is needed for the case where the adapter was 
> part of an MC group, had some sends posted, and then the MC 
> group is being explicitly left.  If the sends to that MC 
> group where posted before the MC group is resolved, they 
> would block the send queue until some other code path calls 
> ipoib_port_resume.

But ipoib_port_resume does not guaranty that you flush all the sends to
that MC group. 
The send to the MC group might be at the end of the pending_list, so if
the sq_depth is full you still end up destroy the endpt while having
outstanding sends to that MC.
Any way if IPoIB come to MC send with out matching endpt :
1. if its 10005E.. MAC ,it will create new endpt and try to send - so
its OK
2. other MC MAC drop the send - OK.

> 
> If we check the port state in ipoib_port_resume we should be 
> able to leave the call to port_resume in the endpoint.

Why we need to call it on each endpt cleanup?Why not to call it on port
destroy (after we remove all endpnt from all lists)
It will have the same effect.

> 
> - Fab
>