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

lizhijian
2022-11-28T10:23:45Z

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
>