On Tue, Apr 30, 2019 at 02:45:02PM -0400, Jason Gunthorpe wrote:
> On Tue., Apr. 30, 2019, 2:43 p.m. Shamir Rabinovitch, <
> shamir.rabinovitch@xxxxxxxxxx> wrote:
>
> > On Tue, Apr 30, 2019 at 02:54:08PM -0300, Jason Gunthorpe wrote:
> > > On Tue, Apr 30, 2019 at 05:23:22PM +0300, Shamir Rabinovitch wrote:
> > > > future patch will remove the ib_uobject pointer from the ib_x
> > > > objects. the uobj_get_obj_read and uobj_put_obj_read macros
> > > > were constructed with the ability to reach the ib_uobject from
> > > > ib_x in mind. this need to change now.
> > > >
> > > > Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@xxxxxxxxxx>
> > > > drivers/infiniband/core/uverbs_cmd.c | 165 +++++++++++++++++++++------
> > > > include/rdma/uverbs_std_types.h | 8 +-
> > > > 2 files changed, 137 insertions(+), 36 deletions(-)
> > > >
> > > > diff --git a/drivers/infiniband/core/uverbs_cmd.c
> > b/drivers/infiniband/core/uverbs_cmd.c
> > > > index 76ac113d1da5..93363c41e77e 100644
> > > > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > > > @@ -37,6 +37,7 @@
> > > > #include <linux/fs.h>
> > > > #include <linux/slab.h>
> > > > #include <linux/sched.h>
> > > > +#include <linux/list.h>
> > > >
> > > > #include <linux/uaccess.h>
> > > >
> > > > @@ -700,6 +701,7 @@ static int ib_uverbs_reg_mr(struct
> > uverbs_attr_bundle *attrs)
> > > > struct ib_uverbs_reg_mr cmd;
> > > > struct ib_uverbs_reg_mr_resp resp;
> > > > struct ib_uobject *uobj;
> > > > + struct ib_uobject *pduobj;
> > > > struct ib_pd *pd;
> > > > struct ib_mr *mr;
> > > > int ret;
> > > > @@ -720,7 +722,8 @@ static int ib_uverbs_reg_mr(struct
> > uverbs_attr_bundle *attrs)
> > > > if (IS_ERR(uobj))
> > > > return PTR_ERR(uobj);
> > > >
> > > > - pd = uobj_get_obj_read(pd, UVERBS_OBJECT_PD, cmd.pd_handle, attrs);
> > > > + pd = uobj_get_obj_read(pd, UVERBS_OBJECT_PD, cmd.pd_handle, attrs,
> > > > + pduobj);
> > >
> > > This should be &pduobj in all places so it reads sensibly..
> >
> > OK. Will change the macro.
> >
> > >
> > > > @@ -2009,6 +2034,12 @@ static int ib_uverbs_post_send(struct
> > uverbs_attr_bundle *attrs)
> > > > const struct ib_sge __user *sgls;
> > > > const void __user *wqes;
> > > > struct uverbs_req_iter iter;
> > > > + struct uobj_list_item {
> > > > + struct list_head list;
> > > > + struct ib_uobject *uobj;
> > > > + };
> > > > + struct uobj_list_item *item, *tmp;
> > > > + LIST_HEAD(ud_uobj_list);
> > >
> > > I'd rather not add this for AH's if we don't plan to drop the uobject
> > > pointer right away.. Same for the other place making a big logic
> > > change
> >
> > What's the concern? We drop the uobject in (almost) same line of code
> > where the current code do that. The uobjects are not held beyond the
> > function. Can you elaborate?
> >
>
> I don't like adding a memory allocation.
>
> Jason
Is it possible to drop the AH uobject ref in this case right after we
figure the ib_ah ? I thought no..
(1) I can move the uobject pointer to the ib_ud_wr if it make more sense
since the ud wr tie the AH uobject to the ib_ah in this case. This has
some benefit for object sharing.
(2) Or leave the uobject pointer in the ib_ah and just use it in this case while
still modifying the macro for the sake of the rest of the code. This leave us with
same situation with regard for object sharing.
As for WQ, (1) is not option AFAK. Only (2).
