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

Sean Hefty sean.hefty at intel.com
Thu Jan 22 14:45:06 PST 2009


>> __ndi_send_req():
>>
>>         cl_spinlock_acquire( &h_qp->obj.lock ); if( h_qp->obj.state !=
>>         CL_DESTROYING && p_qp->cid == AL_INVALID_CID ) {
>>                 status = al_create_cep( qp_get_al( h_qp ),
>> __ndi_cm_handler,
>>                                                 &h_qp->obj,
>>                                                 deref_al_obj, &p_qp->cid
>>                                                 );
>>         }
>>         else
>>         {
>>                 status = IB_INVALID_HANDLE;
>
>Existing code returns IB_RESOURCE_BUSY here.

Invalid handle seems like a better choice, but I can change this.

>
>>         }
>>         cl_spinlock_release( &h_qp->obj.lock );
>>
>>
>> __ndi_send_rep():
>>
>>         /* prepare Passive CEP for connection */ p_qp =
>>         (al_conn_qp_t*)h_qp; cl_spinlock_acquire( &h_qp->obj.lock ); if(
>>         h_qp->obj.state != CL_DESTROYING && p_qp->cid == AL_INVALID_CID
>>         ) {
>>                 status = kal_config_cep(qp_get_al( h_qp ), p_rep->cid,
>>                                                 __ndi_cm_handler,
>>                                                 &h_qp->obj,
>> deref_al_obj);
>>                 if( status == IB_SUCCESS )
>>                 {
>>                         ref_al_obj( &h_qp->obj ); /* Take CEP reference.
>>                         */ status = kal_cep_pre_rep(qp_get_al( h_qp ),
>> p_rep->cid,
>>                                                          &cm_rep,
>>
>> QP_ATTRIB_RNR_NAK_TIMEOUT, &qp_mod );
>>                         if( status == IB_SUCCESS )
>>                         {
>>                                 p_qp->cid = p_rep->cid;
>>                         }
>
>Note that you could config the CEP and call kal_cep_pre_rep without the QP's
>lock, and then perform the checks in the success case and then perform the CID
>checks and make the assignment.  You can probably do this in other places too
>in order to avoid holding the lock over the CEP calls.

I considered this, but I'm not sure it works.  A call to kal_config_cep sets a
callback that ends up releasing a reference to the QP.  I want to lock around
verifying that it's okay to associate the CID with the QP and forming that
association.  It's not clear to me what happens if this is not done atomically.


>> The other area of the code that I would like to cleanup is getting
>> callers to invoke al_destroy_cep() only once for a given CID.  But
>> this looks much more difficult to achieve.
>
>I think if you manage the QP's CID assignment at a higher level you can do all
>the necessary checks on the CID before calling destroy, so only one call path
>should end up calling it.

Ok - it requires a lot more work to achieve.  :)

If we can get users above the new interface, then the CEP code could be modified
to take pointers in place of IDs, which could then be used to replace global
locking with per CEP object locks.  I think this is a longer term goal.

- Sean




More information about the ofw mailing list