On Fri, Jan 04, 2019 at 04:24:24PM +0000, Jason Gunthorpe wrote:
> On Fri, Jan 04, 2019 at 08:12:50AM +0200, Leon Romanovsky wrote:
> > On Thu, Jan 03, 2019 at 10:27:52PM +0000, Jason Gunthorpe wrote:
> > > On Thu, Jan 03, 2019 at 10:05:57AM +0200, Leon Romanovsky wrote:
> > > > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > > >
> > > > Add new general helper to get restrack entry given by ID and
> > > > their respective type.
> > > >
> > > > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > > > drivers/infiniband/core/restrack.c | 13 +++++++++++++
> > > > include/rdma/restrack.h | 11 +++++++++++
> > > > 2 files changed, 24 insertions(+)
> > > >
> > > > diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
> > > > index 473422da0856..d65758a3a7b6 100644
> > > > +++ b/drivers/infiniband/core/restrack.c
> > > > @@ -249,6 +249,19 @@ int __must_check rdma_restrack_get(struct rdma_restrack_entry *res)
> > > > }
> > > > EXPORT_SYMBOL(rdma_restrack_get);
> > > >
> > > > +struct rdma_restrack_entry *
> > > > +rdma_restrack_get_byid(struct rdma_restrack_root *rt,
> > > > + enum rdma_restrack_type type, u32 id)
> > > > +{
> > > > + struct rdma_restrack_entry *res;
> > > > +
> > > > + res = xa_load(&rt->xa[type], id);
> > >
> > > the previous patch did:
> > >
> > > + id = res_to_id(res);
> > > + ret = xa_insert(&dev->res.xa[res->type], id, res, GFP_KERNEL);
> > >
> > > Where res_to_id() returns a pointer casted to an unsigned long - so
> > > the use of 'u32 id' is wrong here.
> >
> > I "removed" from this RFC patches which added software ID generation for
> > drivers without relevant HW/FW and their conversion to use that ID.
> >
> > At the time of final submission res_to_id() won't return pointers.
> >
> > >
> > > Also, it seems to expose kernel pointers to user space, which is a
> > > security no-no.
> >
> > We are not, see introduction of ".id" field in nldev.c. It is added once
> > all drivers are capable to return some unique ID and not pointer.
> >
> > >
> > > I think this needs actual device-unique IDs for all objects, no use of
> > > pointers.
> >
> > It exists, I didn't post it as part of RFC.
> >
> > >
> > > Probably better to use the xarray in the IDR mode and have it generate
> > > unique ID's for all the cases where a HW ID is not already present.
> >
> > I'm attaching this patch for your convenience.
> >
> > From c3cca57ebf246c268b60be9ad364a0275cedb1de Mon Sep 17 00:00:00 2001
> > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > Date: Thu, 3 Jan 2019 18:19:43 +0200
> > Subject: [PATCH 24/25] RDMA/restrack: Implement software ID interface
> >
> > Provide basic functionality to safely get and put software IDs
> > for devices which don't do it in HW.
> >
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > drivers/infiniband/core/restrack.c | 94 +++++++++++++++++++++++++++++-
> > include/rdma/restrack.h | 39 +++++++++++++
> > 2 files changed, 132 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
> > index bd3c37ca7829..f687dffb042a 100644
> > +++ b/drivers/infiniband/core/restrack.c
> > @@ -12,6 +12,24 @@
> >
> > #include "cma_priv.h"
> >
> > +struct sw_id {
> > + /**
> > + * @id: Bitmap to keep SW genrated IDs for devices which
> > + * don't have relevant hW/FW to generate them on their own.
> > + * It is used for types without other unique identification
> > + * available, for example QPs have unique QPN built-in.
> > + */
> > + unsigned long *bitmap;
>
> Why? Just use the xarray to generate IDs.
I wanted, but didn't find proper API for that in include/linux/xarray.h.
Also, we are talking about objects which anyway backed by HW and limited
in size, at least for PD, almost all drivers implemented it with
bitmap, so IDR was overkill for them.
Thanks
>
> Jason
Attachment:
signature.asc
Description: PGP signature
