> On Aug 9, 2021, at 9:38 AM, Haakon Bugge <haakon.bugge@xxxxxxxxxx> wrote: > > > >> On 29 Jul 2021, at 20:28, Marciniszyn, Mike <mike.marciniszyn@xxxxxxxxxxxxxxxxxxxx> wrote: >> >>>> A test of 5.15-rc3 + a revert tested clean. >>>> >>>> Jason, do you need a patch to revert or should I send one. >>> >>> Please, I would like to hear from Haakon as well >>> >> The revert is https://lore.kernel.org/linux-rdma/1627583182-81330-1-git-send-email-mike.marciniszyn@xxxxxxxxxxxxxxxxxxxx/T/#u. > > Hi Mike/Chuck/Jason/Leon, > > > Thanks so much for filling in for me while I was on vacation. > > Short term I agree on the revert. But I actually think commit dc70f7c3ed34 ("RDMA/cma: Remove unnecessary INIT->INIT transition") is correcting an erroneous behaviour in the CMA. But it needs more. > > 1. An API that has a call that has to be made twice (modify_qp --> INIT) is either incorrectly designed or incorrectly used IMHO. > > 2. IBTA is quite clear, that the transition to Initialized shall happen on the Active side when a REQ is sent (section 12.9.7.1 ACTIVE STATES) and the CM state transitions to the "REQ Sent" state. Similar on the Passive side, the transition to Initialized shall happen when you are in the CMA LISTEN state and you receive a REQ and the CM state is transitioned to "REQ Rcvd" state (section 12.9.7.2 PASSIVE STATES). > > 3. Performance-wise, the WR posting _before_ sending a REQ is sent on the Active side (rdma_connect()) or before calling rdma_listen() on the passive side, prolongs the time before said REQ is sent or the server is ready to accept. Doing the WR posting as depicted by IBTA above, the time spent filling the recv queues are hidden because we're waiting for a communication response anyway. Not saying this is pronounced, but worth to mention. > > The problem here seems to be, CMA does incorrectly return a QP in the INIT state after rdma_create_qp() and some ULPs take advantage of it, it does not transition the QP to INIT state when REQ is sent or received as per IBTA, but has the (second) transition to INIT just before the transition to RTR. > > Should this be changed such that the QP is transitioned to the INIT state during rdma_connect() and rdma_accept()? After the respective calls, the ULP is allowed to post recvs. This also aligns nicely with the use of rdma_set_ack_timeout() and rdma_set_min_rnr_timer(). I don't have a philosophical position on exactly how rdma_create_qp() should work going forward, but I agree that ULPs have depended on the current behavior and will need to be updated if the QPs returned by rdma_create_qp() are not in INIT state. I stand ready to fix things up in the RPC/RDMA consumers should that be needed. In fact it looks like some consumers might already assume the corrected CMA behavior. Maybe the RPC/RDMA consumers can safely be modified now? Let me know how to proceed if this is the case. -- Chuck Lever
