> From: Leon Romanovsky [mailto:leon@xxxxxxxxxx] > Sent: Tuesday, July 24, 2018 8:18 PM > On Tue, Jul 24, 2018 at 04:31:49PM +0000, Bason, Yuval wrote: > > > From: Leon Romanovsky [mailto:leon@xxxxxxxxxx] > > > Sent: Tuesday, July 24, 2018 3:37 PM On Mon, Jul 23, 2018 at > > > 04:30:01PM +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(-) > > > > > > > > > > I afraid that all calls to "idr_find" in this patch are not correct. > > > They should be protected with RCU. > > Thanks for pointing this out (de-ref bug..) I'm afraid rcu read lock > > will not suffice, without managing some internal mechanism that requires > more modifications for this patch. > > We could use spin_lock_irqsave for protecting the idr_find, and address this > rcu issue in a future patch. > > What do you think? > > Without doubts, spin_lock_irqsave() will work and it is your driver, however I > don't see any reason why instead of places where you want to add > spin_lock_irqsave(), spin_unlock_irqsave() place rcu_read_lock() and > rcu_read_unlock(). As far as I understand, wrapping the idr_find with rcu_read_lock will not suffice. In addition, I need to address the free(srq) call that is triggered by destroy_srq. Otherwise, I might get NULL pointer de-ref.. > > Thanks > > > > > > > Thanks -- 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