On Wed, Aug 15, 2018 at 11:39:54AM +0000, Kalderon, Michal wrote:
> > 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,
> Michal
> > Thanks
Attachment:
signature.asc
Description: PGP signature
