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.
And I don't see any calls to kfree_rcu or synchronize_rcu in this
driver, so I'm very skeptical this code is OK. Please go over the RCU
check list and make sure RCU is used properly.
Perhaps the spinlock should be held here instead of RCU.
Also.. Use of the radix tree is being phased out with xarray as the
preferred API now. I don't see anything complicated about the use of
the radix tree API here, so could you please respin these patches to use
xarray? It should be straightforward, and I'd prefer not to add new
users while Matthew is trying to get rid of it.
Thanks,
Jason
