Re: [PATCH for-next] RDMA/cma: Remove unnecessary INIT->INIT transition

[Mark Zhang <markzhang@nvidia.com>]

On 6/21/2021 4:44 PM, Haakon Bugge wrote:
> External email: Use caution opening links or attachments
> 
> 
>> On 20 Jun 2021, at 14:19, Mark Zhang <markzhang@nvidia.com> wrote:
>>
>> On 6/17/2021 11:46 PM, Håkon Bugge wrote:
>>> External email: Use caution opening links or attachments
>>> In rdma_create_qp(), a connected QP will be transitioned to the INIT
>>> state.
>>> Afterwards, the QP will be transitioned to the RTR state by the
>>> cma_modify_qp_rtr() function. But this function starts by performing
>>> an ib_modify_qp() to the INIT state again, before another
>>> ib_modify_qp() is performed to transition the QP to the RTR state.
>>> Hence, there is no need to transition the QP to the INIT state in
>>> rdma_create_qp().
>>
>> The comment in cma_modify_qp_rtr() says:
>>     /* Need to update QP attributes from default values. */
>>
>> So maybe both are needed? E.g., qp_attr->qp_access_flags maybe updated in cm_init_qp_init_attr()?
> 
> I'll give you two reasons why that is not the case :-)
> 
> 1. In cm_init_qp_init_attr(), which sets the mask both places, the mask is set hard to
> 
>          IB_QP_STATE | IB_QP_ACCESS_FLAGS | IB_QP_PKEY_INDEX | IB_QP_PORT
> 
> If we do the old RESET -> INIT -> INIT, it is the last modify_qp to INIT which will persist. And therefore, the values will be the same if we skip RESET -> INIT.
> 
> 2. I think the rationale behind modifying the QP to INIT in rdma_create_qp() was to enable ULPs to tweak some of the values. But no ULP calls modify_qp on a QP created by rdma_create_qp(). And if one did, the values would be overwritten when the state transitions to RTR.
> 

Got it, thanks Haakon:)