-----"Stefan Metzmacher" <metze@xxxxxxxxx> wrote: -----
>To: "Bernard Metzler" <bmt@xxxxxxxxxxxxxx>
>From: "Stefan Metzmacher" <metze@xxxxxxxxx>
>Date: 05/07/2021 01:39AM
>Cc: linux-rdma@xxxxxxxxxxxxxxx, "Stefan Metzmacher" <metze@xxxxxxxxx>
>Subject: [EXTERNAL] [PATCH 24/31] rdma/siw: do the full
>disassociation of cep and qp in siw_qp_llp_close()
>
>It's much clearer to drop the references on both sides and reset the
>cross referencing pointers in one place. I makes the caller much
>saner
>and understandable.
I think it is cleaner if the qp code does not alter
cep private pointers as it is in the current code.
>
>Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
>Signed-off-by: Stefan Metzmacher <metze@xxxxxxxxx>
>Cc: Bernard Metzler <bmt@xxxxxxxxxxxxxx>
>Cc: linux-rdma@xxxxxxxxxxxxxxx
>---
> drivers/infiniband/sw/siw/siw_cm.c | 2 --
> drivers/infiniband/sw/siw/siw_qp.c | 3 +++
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/infiniband/sw/siw/siw_cm.c
>b/drivers/infiniband/sw/siw/siw_cm.c
>index 7fd67499f1d3..31135d877d41 100644
>--- a/drivers/infiniband/sw/siw/siw_cm.c
>+++ b/drivers/infiniband/sw/siw/siw_cm.c
>@@ -1240,10 +1240,8 @@ static void siw_cm_work_handler(struct
>work_struct *w)
> siw_cep_set_free(cep);
>
> siw_qp_llp_close(qp);
>- siw_qp_put(qp);
>
> siw_cep_set_inuse(cep);
>- cep->qp = NULL;
> siw_qp_put(qp);
> }
> if (cep->sock) {
>diff --git a/drivers/infiniband/sw/siw/siw_qp.c
>b/drivers/infiniband/sw/siw/siw_qp.c
>index ddb2e66f9f13..badb065eb9b1 100644
>--- a/drivers/infiniband/sw/siw/siw_qp.c
>+++ b/drivers/infiniband/sw/siw/siw_qp.c
>@@ -166,8 +166,11 @@ void siw_qp_llp_close(struct siw_qp *qp)
> * Dereference closing CEP
> */
> if (qp->cep) {
>+ BUG_ON(qp->cep->qp != qp);
Don't introduce BUG()
>+ qp->cep->qp = NULL;
Only the CM code should change that pointer.
> siw_cep_put(qp->cep);
> qp->cep = NULL;
>+ siw_qp_put(qp);
> }
>
> up_write(&qp->state_lock);
>--
>2.25.1
>
>
