在 2018/11/23 2:13, Jason Gunthorpe 写道:
> 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.
ok, I analysis the rcu. The origin idea that it could reduce the frequency with getting lock when read srq
The spinlock may be more proper here. because it should keep the same with
qp event. Also, it use the spin lock in hns_roce_srq_alloc when insert the srq radix tree.
> 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.
I have learning the new API with xarray and need a more comprehensive understanding it .
https://lwn.net/Articles/745073/
I also found all the places in the kernel that use xarray and it is used less. Maybe the xarray will
be considered unified insteading of radix tree API in the hns driver in the following patch in future.
> Thanks,
> Jason
>
>
