Re: [for-next PATCH v6 09/10] RDMA/cm: Make QP FLUSHABLE

[lizhijian@xxxxxxxxxxx]

On 25/11/2022 22:16, Jason Gunthorpe wrote:
>>>>>> ---
>>>>>>     drivers/infiniband/core/cm.c | 3 ++-
>>>>>>     1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
>>>>>> index 1f9938a2c475..58837aac980b 100644
>>>>>> --- a/drivers/infiniband/core/cm.c
>>>>>> +++ b/drivers/infiniband/core/cm.c
>>>>>> @@ -4096,7 +4096,8 @@ static int cm_init_qp_init_attr(struct cm_id_private *cm_id_priv,
>>>>>>     		qp_attr->qp_access_flags = IB_ACCESS_REMOTE_WRITE;
>>>>>>     		if (cm_id_priv->responder_resources)
>>>>>>     			qp_attr->qp_access_flags |= IB_ACCESS_REMOTE_READ |
>>>>>> -						    IB_ACCESS_REMOTE_ATOMIC;
>>>>>> +						    IB_ACCESS_REMOTE_ATOMIC |
>>>>>> +						    IB_ACCESS_FLUSHABLE;
>>>>> What is the point of this? Nothing checks IB_ACCESS_FLUSHABLE ?

>> I'm fine to expand it in next version.
> OIC, that is why it escaped grep
> 
> But this is back to my original question - why is it OK to do this
> here in CMA? Shouldn't this cause other drivers to refuse to create
> the QP because they don't support the flag?
> 

Jason,

My flush example got wrong since V5 where responder side does 
qp_access_flags check. so i added this patch.

I also agreed it's a good idea that we should only add this flush flag 
to the supported drivers. But i haven't figured out how to achieve it in 
current RDMA.

After more digging into rdma-core, i found that this flag can be also 
set from usespace by calling ibv_modify_qp().
For server side(responder), ibv_modify_qp() must be called after 
rdma_accept(). rdma_accept() inside will modify qp_access_flags 
again(rdma_get_request is the first place to modify qp_access_flags).

Back to the original question, IIUC, current rdma-core have no API to 
set qp_access_flags during qp creating.

FLUSH operation in responder side will check both mr->access_flags and 
qp_access_flags. So unsupported device/driver are not able to do flush 
at all though qp_access_flags apply to all drivers.


------------------------------------------
(gdb) bt
#0  __ibv_modify_qp_1_1 (qp=0x40fbf0, attr=0x7fffffffd920, attr_mask=57)
     at /home/lizhijian/rdma-core/libibverbs/verbs.c:715
#1  0x00007ffff7faa1db in ucma_init_conn_qp (id_priv=0x40f6d0, qp=0x40fbf0)
     at /home/lizhijian/rdma-core/librdmacm/cma.c:1380
#2  0x00007ffff7faada2 in rdma_create_qp_ex (id=0x40f6d0, 
attr=0x7fffffffda30)
     at /home/lizhijian/rdma-core/librdmacm/cma.c:1676
#3  0x00007ffff7faae94 in rdma_create_qp (id=0x40f6d0, pd=0x407710, 
qp_init_attr=0x7fffffffdae0)
     at /home/lizhijian/rdma-core/librdmacm/cma.c:1702
#4  0x00007ffff7fab5d3 in rdma_get_request (listen=0x40ede0, id=0x4051a8 
<id>)
     at /home/lizhijian/rdma-core/librdmacm/cma.c:1883
#5  0x0000000000401af9 in run () at 
/home/lizhijian/rdma-core/librdmacm/examples/rdma_flush_server.c:91
#6  0x00000000004021ea in main (argc=7, argv=0x7fffffffe228)
     at /home/lizhijian/rdma-core/librdmacm/examples/rdma_flush_server.c:282

(gdb) bt
#0  __ibv_modify_qp_1_1 (qp=0x40fbf0, attr=0x7fffffffd930, 
attr_mask=1216897)
     at /home/lizhijian/rdma-core/libibverbs/verbs.c:715
#1  0x00007ffff7fa9f16 in ucma_modify_qp_rtr (id=0x40f6d0, resp_res=128 
'\200')
     at /home/lizhijian/rdma-core/librdmacm/cma.c:1304
#2  0x00007ffff7fab80a in rdma_accept (id=0x40f6d0, conn_param=0x0)
     at /home/lizhijian/rdma-core/librdmacm/cma.c:1932
#3  0x0000000000401c8a in run () at 
/home/lizhijian/rdma-core/librdmacm/examples/rdma_flush_server.c:132
#4  0x00000000004021ea in main (argc=7, argv=0x7fffffffe228)
     at /home/lizhijian/rdma-core/librdmacm/examples/rdma_flush_server.c:282


> Jason
>