> From: Leon Romanovsky <leon@xxxxxxxxxx>
> Sent: Wednesday, August 15, 2018 9:23 AM
> To: Bason, Yuval <Yuval.Bason@xxxxxxxxxx>
> Cc: jgg@xxxxxxxxxxxx; dledford@xxxxxxxxxx; linux-rdma@xxxxxxxxxxxxxxx;
> Kalderon, Michal <Michal.Kalderon@xxxxxxxxxx>
> Subject: Re: [PATCH v5 rdma-next 1/3] qedr: Add wrapping generic structure
> for qpidr and adjust idr routines.
>
> On Thu, Aug 09, 2018 at 05:29:36PM +0300, Yuval Bason wrote:
> > Today, we are using idr mechanism for QP's only.
> > This patch prepares the qedr_idr stuctures and the idr routines for
> > both QP's and SRQ's.
> >
> > Signed-off-by: Yuval Bason <yuval.bason@xxxxxxxxxx>
> > Signed-off-by: Michal Kalderon <michal.kalderon@xxxxxxxxxx>
> > ---
> > drivers/infiniband/hw/qedr/main.c | 4 ++--
> > drivers/infiniband/hw/qedr/qedr.h | 8 ++++++--
> > drivers/infiniband/hw/qedr/qedr_iw_cm.c | 12 +++++------
> > drivers/infiniband/hw/qedr/verbs.c | 35 +++++++++++++++--------------
> ----
> > 4 files changed, 30 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/qedr/main.c
> > b/drivers/infiniband/hw/qedr/main.c
> > index ad22b32..912a0ca 100644
> > --- a/drivers/infiniband/hw/qedr/main.c
> > +++ b/drivers/infiniband/hw/qedr/main.c
> > @@ -325,8 +325,8 @@ static int qedr_alloc_resources(struct qedr_dev
> *dev)
> > spin_lock_init(&dev->sgid_lock);
> >
> > if (IS_IWARP(dev)) {
> > - spin_lock_init(&dev->idr_lock);
> > - idr_init(&dev->qpidr);
> > + spin_lock_init(&dev->qpidr.idr_lock);
> > + idr_init(&dev->qpidr.idr);
> > dev->iwarp_wq =
> create_singlethread_workqueue("qedr_iwarpq");
> > }
> >
> > diff --git a/drivers/infiniband/hw/qedr/qedr.h
> > b/drivers/infiniband/hw/qedr/qedr.h
> > index 86d4511..5d0b75e 100644
> > --- a/drivers/infiniband/hw/qedr/qedr.h
> > +++ b/drivers/infiniband/hw/qedr/qedr.h
> > @@ -122,6 +122,11 @@ struct qedr_device_attr {
> >
> > #define QEDR_ENET_STATE_BIT (0)
> >
> > +struct qedr_idr {
> > + spinlock_t idr_lock; /* Protect idr data-structure */
> > + struct idr idr;
> > +};
> > +
> > struct qedr_dev {
> > struct ib_device ibdev;
> > struct qed_dev *cdev;
> > @@ -165,8 +170,7 @@ struct qedr_dev {
> > struct qedr_cq *gsi_rqcq;
> > struct qedr_qp *gsi_qp;
> > enum qed_rdma_type rdma_type;
> > - spinlock_t idr_lock; /* Protect qpidr data-structure */
> > - struct idr qpidr;
> > + struct qedr_idr qpidr;
> > struct workqueue_struct *iwarp_wq;
> > u16 iwarp_max_mtu;
> >
> > diff --git a/drivers/infiniband/hw/qedr/qedr_iw_cm.c
> > b/drivers/infiniband/hw/qedr/qedr_iw_cm.c
> > index 26dc374..505fa36 100644
> > --- a/drivers/infiniband/hw/qedr/qedr_iw_cm.c
> > +++ 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)
> >
> > DP_DEBUG(dev, QEDR_MSG_IWARP, "Accept on qpid=%d\n",
> > conn_param->qpn);
> >
> > - qp = idr_find(&dev->qpidr, conn_param->qpn);
> > + qp = idr_find(&dev->qpidr.idr, conn_param->qpn);
>
> You choose spinlock over RCU, such decision requires from you to use
> spinlock over all IDR operations, including idr_find().
>
Hey Leon, we've discussed this point with Jason, as this is not related to srq we will
submit a fix related to rdma cm for iWARP that handles the qp refcnt and idr_find on the qpidr
In a separate patch.
Thanks,
Michal
> Thanks
