On Thu, Jan 31, 2019 at 07:30:56PM +0200, Leon Romanovsky wrote:
> On Wed, Jan 30, 2019 at 09:24:56PM -0700, Jason Gunthorpe wrote:
> > On Wed, Jan 30, 2019 at 12:49:04PM +0200, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > >
> > > As a preparation to extension of rdma_restrack_root to provide
> > > software IDs, which will be per-type too. We convert the
> > > rdma_restrack_root from struct with arrays to array of structs.
> > >
> > > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > > drivers/infiniband/core/restrack.c | 35 +++++++++++++++---------------
> > > 1 file changed, 17 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
> > > index 284d087d7785..bcd6fc9d3225 100644
> > > +++ b/drivers/infiniband/core/restrack.c
> > > @@ -46,17 +46,13 @@ struct rdma_restrack_root {
> > > */
> > > struct rw_semaphore rwsem;
> > > /**
> > > - * @xa: Array of XArray structures to hold restrack entries.
> > > - * We want to use array of XArrays because insertion is type
> > > - * dependent. For types with xisiting unique ID (like QPN),
> > > - * we will insert to that unique index. For other types,
> > > - * we insert based on pointers and auto-allocate unique index.
> > > + * @xa: Array of XArray structure to hold restrack entries.
> > > */
> > > - struct xarray xa[RDMA_RESTRACK_MAX];
> > > + struct xarray xa;
> > > /**
> > > * #next_id: Next ID to support cyclic allocation
> > > */
> > > - u32 next_id[RDMA_RESTRACK_MAX];
> > > + u32 next_id;
> > > };
> > >
> > > /**
> > > @@ -70,15 +66,16 @@ int rdma_restrack_init(struct ib_device *dev)
> > > struct rdma_restrack_root *rt;
> > > int i;
> > >
> > > - dev->res = kzalloc(sizeof(*rt), GFP_KERNEL);
> > > + dev->res = kcalloc(RDMA_RESTRACK_MAX, sizeof(*rt), GFP_KERNEL);
> > > if (!dev->res)
> > > return -ENOMEM;
> > >
> > > rt = dev->res;
> > >
> > > - for (i = 0 ; i < RDMA_RESTRACK_MAX; i++)
> > > - xa_init_flags(&rt->xa[i], XA_FLAGS_ALLOC);
> > > - init_rwsem(&rt->rwsem);
> > > + for (i = 0 ; i < RDMA_RESTRACK_MAX; i++) {
> > > + init_rwsem(&rt[i].rwsem);
> > > + xa_init_flags(&rt[i].xa, XA_FLAGS_ALLOC);
> > > + }
> > >
> > > return 0;
> > > }
> > > @@ -107,7 +104,7 @@ static const char *type2str(enum rdma_restrack_type type)
> > > struct xarray *rdma_dev_to_xa(struct ib_device *dev,
> > > enum rdma_restrack_type type)
> > > {
> > > - return &dev->res->xa[type];
> > > + return &dev->res[type].xa;
> > >
> > > }
> > > EXPORT_SYMBOL(rdma_dev_to_xa);
> > > @@ -120,7 +117,7 @@ EXPORT_SYMBOL(rdma_dev_to_xa);
> > > */
> > > void rdma_rt_read_lock(struct ib_device *dev, enum rdma_restrack_type type)
> > > {
> > > - down_read(&dev->res->rwsem);
> > > + down_read(&dev->res[type].rwsem);
> > > }
> > > EXPORT_SYMBOL(rdma_rt_read_lock);
> > >
> > > @@ -132,7 +129,7 @@ EXPORT_SYMBOL(rdma_rt_read_lock);
> > > */
> > > void rdma_rt_read_unlock(struct ib_device *dev, enum rdma_restrack_type type)
> > > {
> > > - up_read(&dev->res->rwsem);
> > > + up_read(&dev->res[type].rwsem);
> > > }
> > > EXPORT_SYMBOL(rdma_rt_read_unlock);
> > >
> > > @@ -286,7 +283,7 @@ static void rdma_restrack_add(struct rdma_restrack_entry *res)
> > > if (!dev)
> > > return;
> > >
> > > - 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.
Jason
