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.
Jason
