On Wed, Jul 25, 2018 at 02:04:56PM +0000, Bason, Yuval wrote: > > 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.. Those are two different problems, one is attempt to convert IDR to pointer (idr_find) without ensuring that IDR database is protected and second improper lifetime management of received pointer. First problem can be solved by RCU or spinlock, second problem is usually solved by kref. Thanks > > > > Thanks > > > > > > > > > > Thanks
Attachment:
signature.asc
Description: PGP signature
