| Message ID | 20190207164511.21401-1-shamir.rabinovitch@oracle.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | IB/{hw,sw}: remove 'uobject->context' dependency APIs | expand |
On Thu, Feb 07, 2019 at 06:44:46PM +0200, Shamir Rabinovitch wrote: > This patch set continue the cleanup started with Jason RFC patch. This > patch set clean only the ib_xxx creation APIs because those APIs do have > ib_udata. > > The final goal of this cleanup is to remove the dependency in the IB > code in the ib_xxx->uobject pointer as step toward shared ib_xxx > objects. > > Changelog: > > v3: > - Jason: Modify helper rdma_udata_to_drv_context > - Jason: Remove helper rdma_get_ucontext > - Christoph Hellwig: Leave ib_udata as center of the user/kernel > control flow > > v2: > - Jason: Add the ib_ucontext to the attr bundle and make sure > rdma_get_ucontext can't fail > - Jason: Add helper macro to the the driver's context out of > ib_udata > - Leon: Un needed tests in mlx4_ib_db_unmap_user > > > Shamir Rabinovitch (3): > IB/uverbs: add ib_ucontext to uverbs_attr_bundle sent from ioctl and > cmd flows > IB/verbs: add helper function rdma_udata_to_drv_context > IB/{hw,sw}: remove 'uobject->context' dependency in object creation > APIs Applied to for-next, with a number of revisions - Needed rebasing - Fixes compilation failures in HNS. Enable COMPILE_TEST to get all drivers turned on in .config - Minor re-formatting - Few missed conversion places in mlx5 raw qp and devx Please check over the patches I pushed to wip/jgg-for-next The next logical series is to add udata parameters to all the destroy functions and get rid of their references to udata->context. I would like to get to a point where uobj->context is never seen in any drivers, it will be really close after the destroy change. Also, it would make sense to me to now clean up all the ops signatures that pass both a ucontext and a udata - we should never pass both, just pass the udata. Would you do that rationalization too? Thanks, Jason
On Fri, Feb 15, 2019 at 03:46:07PM -0700, Jason Gunthorpe wrote: > On Thu, Feb 07, 2019 at 06:44:46PM +0200, Shamir Rabinovitch wrote: > > This patch set continue the cleanup started with Jason RFC patch. This > > patch set clean only the ib_xxx creation APIs because those APIs do have > > ib_udata. > > > > The final goal of this cleanup is to remove the dependency in the IB > > code in the ib_xxx->uobject pointer as step toward shared ib_xxx > > objects. > > > > Changelog: > > > > v3: > > - Jason: Modify helper rdma_udata_to_drv_context > > - Jason: Remove helper rdma_get_ucontext > > - Christoph Hellwig: Leave ib_udata as center of the user/kernel > > control flow > > > > v2: > > - Jason: Add the ib_ucontext to the attr bundle and make sure > > rdma_get_ucontext can't fail > > - Jason: Add helper macro to the the driver's context out of > > ib_udata > > - Leon: Un needed tests in mlx4_ib_db_unmap_user > > > > > > Shamir Rabinovitch (3): > > IB/uverbs: add ib_ucontext to uverbs_attr_bundle sent from ioctl and > > cmd flows > > IB/verbs: add helper function rdma_udata_to_drv_context > > IB/{hw,sw}: remove 'uobject->context' dependency in object creation > > APIs > > Applied to for-next, with a number of revisions > - Needed rebasing > - Fixes compilation failures in HNS. Enable COMPILE_TEST to get all > drivers turned on in .config Thanks I did not know this. > - Minor re-formatting > - Few missed conversion places in mlx5 raw qp and devx > > Please check over the patches I pushed to wip/jgg-for-next Thanks will do. > > The next logical series is to add udata parameters to all the destroy > functions and get rid of their references to udata->context. I would > like to get to a point where uobj->context is never seen in any drivers, > it will be really close after the destroy change. I already have the patch set. Need some go-no-go test and it's ready. > > Also, it would make sense to me to now clean up all the ops signatures > that pass both a ucontext and a udata - we should never pass both, > just pass the udata. Would you do that rationalization too? Sure. I already did this. Will double check myself here. > > Thanks, > Jason After this patch set I would like to send RFC for a way to add the 'clone' function pointer for each ib_xx core object. I think it can be done by adding wrapper struct to each ib_xx core object via macro as you did for the type destroy in uverbs. Core objects are allocated by drivers so it's up to the driver to decide if it add the 'clone' function or not. If object is not shareable in that driver, 'clone' will be left as NULL. 'clone' purpose is to copy the driver specific information to user from existing ib_x object. Thanks, Shamir
On Fri, Feb 15, 2019 at 03:46:07PM -0700, Jason Gunthorpe wrote: > On Thu, Feb 07, 2019 at 06:44:46PM +0200, Shamir Rabinovitch wrote: > > This patch set continue the cleanup started with Jason RFC patch. This > > patch set clean only the ib_xxx creation APIs because those APIs do have > > ib_udata. > > > > The final goal of this cleanup is to remove the dependency in the IB > > code in the ib_xxx->uobject pointer as step toward shared ib_xxx > > objects. > > > > Changelog: > > > > v3: > > - Jason: Modify helper rdma_udata_to_drv_context > > - Jason: Remove helper rdma_get_ucontext > > - Christoph Hellwig: Leave ib_udata as center of the user/kernel > > control flow > > > > v2: > > - Jason: Add the ib_ucontext to the attr bundle and make sure > > rdma_get_ucontext can't fail > > - Jason: Add helper macro to the the driver's context out of > > ib_udata > > - Leon: Un needed tests in mlx4_ib_db_unmap_user > > > > > > Shamir Rabinovitch (3): > > IB/uverbs: add ib_ucontext to uverbs_attr_bundle sent from ioctl and > > cmd flows > > IB/verbs: add helper function rdma_udata_to_drv_context > > IB/{hw,sw}: remove 'uobject->context' dependency in object creation > > APIs > > Applied to for-next, with a number of revisions > - Needed rebasing > - Fixes compilation failures in HNS. Enable COMPILE_TEST to get all > drivers turned on in .config > - Minor re-formatting > - Few missed conversion places in mlx5 raw qp and devx > > Please check over the patches I pushed to wip/jgg-for-next Jason, I only see the patch 1/3 on this branch. Have I missed anything? Thanks > > The next logical series is to add udata parameters to all the destroy > functions and get rid of their references to udata->context. I would > like to get to a point where uobj->context is never seen in any drivers, > it will be really close after the destroy change. > > Also, it would make sense to me to now clean up all the ops signatures > that pass both a ucontext and a udata - we should never pass both, > just pass the udata. Would you do that rationalization too? > > Thanks, > Jason
On Mon, Feb 18, 2019 at 11:29:39AM +0200, Shamir Rabinovitch wrote: > On Fri, Feb 15, 2019 at 03:46:07PM -0700, Jason Gunthorpe wrote: > > On Thu, Feb 07, 2019 at 06:44:46PM +0200, Shamir Rabinovitch wrote: > > > This patch set continue the cleanup started with Jason RFC patch. This > > > patch set clean only the ib_xxx creation APIs because those APIs do have > > > ib_udata. > > > > > > The final goal of this cleanup is to remove the dependency in the IB > > > code in the ib_xxx->uobject pointer as step toward shared ib_xxx > > > objects. > > > > > > Changelog: > > > > > > v3: > > > - Jason: Modify helper rdma_udata_to_drv_context > > > - Jason: Remove helper rdma_get_ucontext > > > - Christoph Hellwig: Leave ib_udata as center of the user/kernel > > > control flow > > > > > > v2: > > > - Jason: Add the ib_ucontext to the attr bundle and make sure > > > rdma_get_ucontext can't fail > > > - Jason: Add helper macro to the the driver's context out of > > > ib_udata > > > - Leon: Un needed tests in mlx4_ib_db_unmap_user > > > > > > > > > Shamir Rabinovitch (3): > > > IB/uverbs: add ib_ucontext to uverbs_attr_bundle sent from ioctl and > > > cmd flows > > > IB/verbs: add helper function rdma_udata_to_drv_context > > > IB/{hw,sw}: remove 'uobject->context' dependency in object creation > > > APIs > > > > Applied to for-next, with a number of revisions > > - Needed rebasing > > - Fixes compilation failures in HNS. Enable COMPILE_TEST to get all > > drivers turned on in .config > > - Minor re-formatting > > - Few missed conversion places in mlx5 raw qp and devx > > > > Please check over the patches I pushed to wip/jgg-for-next > > Jason, > > I only see the patch 1/3 on this branch. > Have I missed anything? commit 89944450547334aa6655e0cd4aec8df1897a205a Author: Shamir Rabinovitch <shamir.rabinovitch@oracle.com> Date: Thu Feb 7 18:44:49 2019 +0200 IB/{hw,sw}: Remove 'uobject->context' dependency in object creation APIs Now when we have the udata passed to all the ib_xxx object creation APIs and the additional macro 'rdma_udata_to_drv_context' to get the ib_ucontext from ib_udata stored in uverbs_attr_bundle, we can finally start to remove the dependency of the drivers in the ib_xxx->uobject->context. Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@oracle.com> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> commit 730623f4a56fa42d4559715ff2f4a5c32b3ae8bf Author: Shamir Rabinovitch <shamir.rabinovitch@oracle.com> Date: Thu Feb 7 18:44:48 2019 +0200 IB/verbs: Add helper function rdma_udata_to_drv_context Helper function to get driver's context out of ib_udata wrapped in uverbs_attr_bundle for user objects or NULL for kernel objects. Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@oracle.com> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> commit 3d9dfd060391928bd615db62ecddea5e1255edfd Author: Shamir Rabinovitch <shamir.rabinovitch@oracle.com> Date: Thu Feb 7 18:44:47 2019 +0200 IB/uverbs: Add ib_ucontext to uverbs_attr_bundle sent from ioctl and cmd flows Add ib_ucontext to the uverbs_attr_bundle sent down the iocl and cmd flows as soon as the flow has ib_uobject. In addition, remove rdma_get_ucontext helper function that is only used by ib_umem_get. Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@oracle.com> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> Jason
On Sun, Feb 17, 2019 at 09:58:27AM +0200, Shamir Rabinovitch wrote: > After this patch set I would like to send RFC for a way to add the > 'clone' function pointer for each ib_xx core object. I think it can be > done by adding wrapper struct to each ib_xx core object via macro as you > did for the type destroy in uverbs. I'm not sure what you want to do here. Why do we need a callback? The HW object should not need to be touched. What you need is a new ioctl entry point that clones nearly any uobject between ufile's. This should just be some general core code routine using the ANY object stuff. The trick here will be to refcount the HW object and figure out what destroy means. > 'clone' purpose is to copy the driver specific information to user from > existing ib_x object. The driver involvement should be in a general 'query_udata' entry point that returns the driver specific data for the HW object. If the driver can't do that then rdma-core can fail the operation. Clone is the wrong entry point as I really don't want to comtemplate cloning the HW object. Jason
This patch set continue the cleanup started with Jason RFC patch. This patch set clean only the ib_xxx creation APIs because those APIs do have ib_udata. The final goal of this cleanup is to remove the dependency in the IB code in the ib_xxx->uobject pointer as step toward shared ib_xxx objects. Changelog: v3: - Jason: Modify helper rdma_udata_to_drv_context - Jason: Remove helper rdma_get_ucontext - Christoph Hellwig: Leave ib_udata as center of the user/kernel control flow v2: - Jason: Add the ib_ucontext to the attr bundle and make sure rdma_get_ucontext can't fail - Jason: Add helper macro to the the driver's context out of ib_udata - Leon: Un needed tests in mlx4_ib_db_unmap_user Shamir Rabinovitch (3): IB/uverbs: add ib_ucontext to uverbs_attr_bundle sent from ioctl and cmd flows IB/verbs: add helper function rdma_udata_to_drv_context IB/{hw,sw}: remove 'uobject->context' dependency in object creation APIs drivers/infiniband/core/rdma_core.c | 36 ++++++++++ drivers/infiniband/core/umem.c | 10 ++- drivers/infiniband/core/uverbs_cmd.c | 2 +- drivers/infiniband/core/uverbs_ioctl.c | 3 + drivers/infiniband/core/uverbs_main.c | 25 +------ drivers/infiniband/hw/bnxt_re/ib_verbs.c | 24 ++++--- drivers/infiniband/hw/cxgb3/iwch_provider.c | 4 +- drivers/infiniband/hw/cxgb4/qp.c | 10 +-- drivers/infiniband/hw/hns/hns_roce_qp.c | 22 +++--- drivers/infiniband/hw/i40iw/i40iw_verbs.c | 11 +-- drivers/infiniband/hw/mlx4/mr.c | 11 ++- drivers/infiniband/hw/mlx4/qp.c | 74 +++++++++++--------- drivers/infiniband/hw/mlx4/srq.c | 10 ++- drivers/infiniband/hw/mlx5/qp.c | 54 ++++++++------ drivers/infiniband/hw/mlx5/srq.c | 9 +-- drivers/infiniband/hw/mthca/mthca_provider.c | 23 +++--- drivers/infiniband/hw/mthca/mthca_qp.c | 14 ++-- drivers/infiniband/hw/mthca/mthca_srq.c | 23 +++--- drivers/infiniband/hw/nes/nes_verbs.c | 14 ++-- drivers/infiniband/hw/qedr/verbs.c | 7 +- drivers/infiniband/hw/usnic/usnic_ib_verbs.c | 6 +- drivers/infiniband/sw/rdmavt/qp.c | 10 ++- drivers/infiniband/sw/rdmavt/srq.c | 10 ++- drivers/infiniband/sw/rxe/rxe_qp.c | 8 ++- drivers/infiniband/sw/rxe/rxe_verbs.c | 6 +- include/rdma/ib_verbs.h | 16 ++++- include/rdma/uverbs_ioctl.h | 1 + include/rdma/uverbs_std_types.h | 18 +++-- 28 files changed, 285 insertions(+), 176 deletions(-)