On Fri, Nov 23, 2018 at 05:30:21PM +0800, oulijun wrote:
> 在 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.
The usage here is extremely simple, I would like it if you could help
Matthew's conversion by just starting out with xarray.
His conversion patches are here:
http://git.infradead.org/users/willy/linux-dax.git/shortlog/refs/heads/xarray-conv
Thanks,
Jason
