On Thu, Nov 22, 2018 at 08:50:00PM +0200, Leon Romanovsky wrote:
> On Thu, Nov 22, 2018 at 11:13:46AM -0700, Jason Gunthorpe wrote:
> > On Sat, Oct 27, 2018 at 03:15:18PM +0800, Lijun Ou wrote:
> > > +void hns_roce_srq_event(struct hns_roce_dev *hr_dev, u32 srqn, int event_type)
> > > +{
> > > + struct hns_roce_srq_table *srq_table = &hr_dev->srq_table;
> > > + struct hns_roce_srq *srq;
> > > +
> > > + rcu_read_lock();
> > > + srq = radix_tree_lookup(&srq_table->tree,
> > > + srqn & (hr_dev->caps.num_srqs - 1));
> > > + rcu_read_unlock();
> > > + if (srq) {
> > > + refcount_inc(&srq->refcount);
> >
> > This doesn't look right to me..
> >
> > The refcount_inc should be inside the RCU and it should almost
> > certainly be refcount_inc_not_zero.
>
> Are you sure that you can update the variable inside of struct which is
> protected by RCU?
Yes. Holding the rcu_read_lock() prevents the structure from being freed
after its refcount hits zero. refcount_inc_not_zero() will fail if the
structure is waiting to be freed.
