| Message ID | 505a18c83a283963174c9e567ce73d055e26ec7b.1655248086.git.metze@samba.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | rdma/siw: implement non-blocking connect | expand |
On 6/15/22 4:40 PM, Stefan Metzmacher wrote: > The mpa timer will also span the non-blocking connect > in the final patch. > > Fixes: 6c52fdc244b5 ("rdma/siw: connection management") > Signed-off-by: Stefan Metzmacher <metze@samba.org> > Cc: Bernard Metzler <bmt@zurich.ibm.com> > Cc: linux-rdma@vger.kernel.org > --- > drivers/infiniband/sw/siw/siw_cm.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c > index b19a2b777814..3fee1d4ef252 100644 > --- a/drivers/infiniband/sw/siw/siw_cm.c > +++ b/drivers/infiniband/sw/siw/siw_cm.c > @@ -1476,6 +1476,11 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params) > cep->mpa.hdr.params.pd_len = pd_len; > } > > + rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT); > + if (rv != 0) { > + goto error; > + } > + > cep->state = SIW_EPSTATE_AWAIT_MPAREP; > Here starts the MPA timer, but the cep->state is SIW_EPSTATE_CONNECTING. Consider the case when the connection timeout: the MPA timeout handler will release resources if cep->state is SIW_EPSTATE_AWAIT_MPAREP and SIW_EPSTATE_AWAIT_MPAREQ, not including SIW_EPSTATE_CONNECTING. I think you should handle this case in the MPA timeout handler: report a cm event and set release_cep with 1. Otherwise it will cause resource leak. Thanks, Cheng Xu
Hi Cheng, > On 6/15/22 4:40 PM, Stefan Metzmacher wrote: >> The mpa timer will also span the non-blocking connect >> in the final patch. >> >> Fixes: 6c52fdc244b5 ("rdma/siw: connection management") >> Signed-off-by: Stefan Metzmacher <metze@samba.org> >> Cc: Bernard Metzler <bmt@zurich.ibm.com> >> Cc: linux-rdma@vger.kernel.org >> --- >> drivers/infiniband/sw/siw/siw_cm.c | 12 +++++++----- >> 1 file changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c >> index b19a2b777814..3fee1d4ef252 100644 >> --- a/drivers/infiniband/sw/siw/siw_cm.c >> +++ b/drivers/infiniband/sw/siw/siw_cm.c >> @@ -1476,6 +1476,11 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params) >> cep->mpa.hdr.params.pd_len = pd_len; >> } >> + rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT); >> + if (rv != 0) { >> + goto error; >> + } >> + >> cep->state = SIW_EPSTATE_AWAIT_MPAREP; > > Here starts the MPA timer, but the cep->state is SIW_EPSTATE_CONNECTING. > > Consider the case when the connection timeout: the MPA timeout handler > will release resources if cep->state is SIW_EPSTATE_AWAIT_MPAREP and > SIW_EPSTATE_AWAIT_MPAREQ, not including SIW_EPSTATE_CONNECTING. > > I think you should handle this case in the MPA timeout handler: report > a cm event and set release_cep with 1. Otherwise it will cause resource > leak. Yes, you're right. I've actually fixed it in the rest of the patchset I have. It seems I need to pull in a few more patches into the first chunk. metze
diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c index b19a2b777814..3fee1d4ef252 100644 --- a/drivers/infiniband/sw/siw/siw_cm.c +++ b/drivers/infiniband/sw/siw/siw_cm.c @@ -1476,6 +1476,11 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params) cep->mpa.hdr.params.pd_len = pd_len; } + rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT); + if (rv != 0) { + goto error; + } + cep->state = SIW_EPSTATE_AWAIT_MPAREP; rv = siw_send_mpareqrep(cep, cep->mpa.pdata, @@ -1493,11 +1498,6 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params) goto error; } - rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT); - if (rv != 0) { - goto error; - } - siw_dbg_cep(cep, "[QP %u]: exit\n", qp_id(qp)); siw_cep_set_free(cep); return 0; @@ -1506,6 +1506,8 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params) siw_dbg(id->device, "failed: %d\n", rv); if (cep) { + siw_cancel_mpatimer(cep); + siw_socket_disassoc(s); sock_release(s); cep->sock = NULL;
The mpa timer will also span the non-blocking connect in the final patch. Fixes: 6c52fdc244b5 ("rdma/siw: connection management") Signed-off-by: Stefan Metzmacher <metze@samba.org> Cc: Bernard Metzler <bmt@zurich.ibm.com> Cc: linux-rdma@vger.kernel.org --- drivers/infiniband/sw/siw/siw_cm.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)