On Sun, Jul 29, 2018 at 01:16:10PM +0300, Yuval Bason wrote:
> diff --git a/drivers/infiniband/hw/qedr/qedr_iw_cm.c b/drivers/infiniband/hw/qedr/qedr_iw_cm.c
> index 26dc374..505fa36 100644
> +++ b/drivers/infiniband/hw/qedr/qedr_iw_cm.c
> @@ -491,7 +491,7 @@ int qedr_iw_connect(struct iw_cm_id *cm_id, struct iw_cm_conn_param *conn_param)
> int rc = 0;
> int i;
>
> - qp = idr_find(&dev->qpidr, conn_param->qpn);
> + qp = idr_find(&dev->qpidr.idr, conn_param->qpn);
>
> laddr = (struct sockaddr_in *)&cm_id->m_local_addr;
> raddr = (struct sockaddr_in *)&cm_id->m_remote_addr;
> @@ -679,7 +679,7 @@ int qedr_iw_accept(struct iw_cm_id *cm_id, struct iw_cm_conn_param *conn_param)
[..]
> @@ -2279,8 +2275,9 @@ int qedr_destroy_qp(struct ib_qp *ibqp)
>
> qedr_free_qp_resources(dev, qp);
>
> - if (atomic_dec_and_test(&qp->refcnt)) {
> - qedr_idr_remove(dev, qp->qp_id);
> + if (atomic_dec_and_test(&qp->refcnt) &&
> + rdma_protocol_iwarp(&dev->ibdev, 1)) {
> + qedr_idr_remove(dev, &dev->qpidr, qp->qp_id);
> kfree(qp);
So what prevents kfree from racing with the idr_find above? Looks like noting.
And this code above is total garbage:
if (qp->ep) {
int wait_count = 1;
while (qp->ep->during_connect) {
DP_DEBUG(dev, QEDR_MSG_QP,
"Still in during connect/accept\n");
msleep(100);
if (wait_count++ > 200) {
DP_NOTICE(dev,
"during connect timeout\n");
break;
}
}
}
That is just not an acceptable way to manage concurrency, it is not
allowed to just read unlocked some random variable.
As is the unlocked use of qp->ep.
I think you need to send some kind of series to fix the races with
this idr before going ahead with anything else.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
