On Thu, Jan 31, 2019 at 03:29:00PM -0700, Jason Gunthorpe wrote: > 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. rdma_dev_to_xa() is used outside of restrack.c (e.g. nldev.c) and it is designed to hide restrack_root. Do you want to return to header rdma_restrack_root? Thanks > > Jason
Attachment:
signature.asc
Description: PGP signature
