On Sun, Oct 14, 2018 at 01:22:28PM -0600, Jason Gunthorpe wrote:
> On Sun, Oct 14, 2018 at 09:11:13AM +0300, Shamir Rabinovitch wrote:
> > initialize the context field in ib_udata created from uverbs commands
> > and extended commands path.
> >
> > Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@xxxxxxxxxx>
> > drivers/infiniband/core/uverbs.h | 9 ++++---
> > drivers/infiniband/core/uverbs_cmd.c | 35 ++++++++++++++-------------
> > drivers/infiniband/core/uverbs_main.c | 6 +++--
> > 3 files changed, 28 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
> > index c97935a0c7c6..3215f2f4e6d7 100644
> > +++ b/drivers/infiniband/core/uverbs.h
> > @@ -55,23 +55,26 @@ static inline void
> > ib_uverbs_init_udata(struct ib_udata *udata,
> > const void __user *ibuf,
> > void __user *obuf,
> > - size_t ilen, size_t olen)
> > + size_t ilen, size_t olen,
> > + struct ib_ucontext *context)
> > {
> > udata->inbuf = ibuf;
> > udata->outbuf = obuf;
> > udata->inlen = ilen;
> > udata->outlen = olen;
> > + udata->context = context;
> > }
> >
> > static inline void
> > ib_uverbs_init_udata_buf_or_null(struct ib_udata *udata,
> > const void __user *ibuf,
> > void __user *obuf,
> > - size_t ilen, size_t olen)
> > + size_t ilen, size_t olen,
> > + struct ib_ucontext *context)
> > {
> > ib_uverbs_init_udata(udata,
> > ilen ? ibuf : NULL, olen ? obuf : NULL,
> > - ilen, olen);
> > + ilen, olen, context);
> > }
> >
> > /*
> > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> > index d41497acb264..cfe35ba0b619 100644
> > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > @@ -100,7 +100,7 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
> > ib_uverbs_init_udata(&udata, buf + sizeof(cmd),
> > u64_to_user_ptr(cmd.response) + sizeof(resp),
> > in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
> > - out_len - sizeof(resp));
> > + out_len - sizeof(resp), ib_uverbs_get_ucontext(file));
>
> > ret = ib_rdmacg_try_charge(&cg_obj, ib_dev, RDMACG_RESOURCE_HCA_HANDLE);
> > if (ret)
> > @@ -358,7 +358,7 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
> > ib_uverbs_init_udata(&udata, buf + sizeof(cmd),
> > u64_to_user_ptr(cmd.response) + sizeof(resp),
> > in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
> > - out_len - sizeof(resp));
> > + out_len - sizeof(resp), ib_uverbs_get_ucontext(file));
>
> No, this has to come form the uobject, get_ucontext is expensive and
> can return NULL unexpectedly in this context..
>
> This is why I keep saying it must be set during uobj_get/etc.
>
> Jason
uobj_get_xx are used to get the sub objects of the prime object.
Using this to copy the context to the udata can result in copying
the context over and over for prime object that has couple of sub
objects. Each of the sub objects is expected to have same context
as the prime object.
I have series that take the context from the prime object context
and when we do not have such object we just initialize the udata
context to NULL.
If this is OK with you, I'd like to post the series.
The series fix the kbuild hns build issues and I can send it as one
long series (avoid kbuild detecting missing function on patch-set #2)
or as 3 patch-sets for easy review.
Thanks, Shamir
