Messages in this thread | | | From | "" <> | Subject | Re: [for-next PATCH v6 09/10] RDMA/cm: Make QP FLUSHABLE | Date | Wed, 23 Nov 2022 06:07:37 +0000 |
| |
On 22/11/2022 22:52, Jason Gunthorpe wrote: > On Wed, Nov 16, 2022 at 04:19:50PM +0800, Li Zhijian wrote: >> It enables flushable access flag for qp >> >> Reviewed-by: Zhu Yanjun <zyjzyj2000@gmail.com> >> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> >> --- >> V5: new patch, inspired by Bob >> --- >> 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 ?
Previous, responder of RXE will check qp_access_flags in check_op_valid(): 256 static enum resp_states check_op_valid(struct rxe_qp *qp,
257 struct rxe_pkt_info *pkt)
258 {
259 switch (qp_type(qp)) {
260 case IB_QPT_RC:
261 if (((pkt->mask & RXE_READ_MASK) &&
262 !(qp->attr.qp_access_flags & IB_ACCESS_REMOTE_READ)) ||
263 ((pkt->mask & RXE_WRITE_MASK) &&
264 !(qp->attr.qp_access_flags & IB_ACCESS_REMOTE_WRITE)) || 265 ((pkt->mask & RXE_ATOMIC_MASK) &&
266 !(qp->attr.qp_access_flags & IB_ACCESS_REMOTE_ATOMIC))) { 267 return RESPST_ERR_UNSUPPORTED_OPCODE;
268 }
based on this, additional IB_FLUSH_PERSISTENT and IB_ACCESS_FLUSH_GLOBAL were added in patch7 since V5 suggested by Bob. Because of this change, QP should become FLUSHABLE correspondingly.
> > Do flush ops require a responder resource?
Yes, i think so. See IBA spec, oA19-9: FLUSH shall consume a single responder...
> > Why should CM set it unconditionally? >
I had ever checked git history log of qp->qp_access_flags, they did as it's. So i also think qp_access_flags should accept all new IBA abilities unconditionally.
What do you think of this @Jason
Thanks Zhijian > Explain in the commit message > > Jason
| |