On Fri, Feb 01, 2019 at 12:10:55AM +0200, Leon Romanovsky wrote:
> > > > > > > - rt = dev->res;
> > > > > > > + rt = &dev->res[res->type];
> > > > > > > xa = rdma_dev_to_xa(dev, res->type);
> > > > > >
> > > > > > This whole rdma_dev_to_xa thing looks pretty pointless now that xa is
> > > > > > just rt->xa
> > > > > >
> > > > > > I feel like this patch should be pushed down to be right
> > > > > > after/before/or part of the xarray introduction patch
> > > > >
> > > > > I prefer to keep it as is, except painful rebase your proposal
> > > > > won't give us much, the outcome will be the same.
> > > >
> > > > It means you won't add a function in one patch and then just delete
> > > > the entire function in the next patch, which is fairly bad
> > > > practice. All the patch ordering in this series can be improved to
> > > > make it more understandable.
> > >
> > > I didn't delete rdma_dev_to_xa(), that function was introduced in
> > > "RDMA/restrack: Hide restrack DB from IB/core" patch.
> >
> > My remark is that you should delete it now that everything trivially
> > has rt->xa
>
> Is this "must"? I really like this helper and it is easier
> for me than "&dev->res[type].xa"
You should use this approach everywhere:
rt = &dev->res[res->type];
down_read(&rt->rwsem);
xa_load(&rt->xa);
Instead of building a whole bunch of wakky wrapper APIs that take type
as an argument.
Jason
