| Message ID | 1551981764-26557-3-git-send-email-maxg@mellanox.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce new API for T10-PI offload | expand |
On Thu, 2019-03-07 at 20:02 +0200, Max Gurtovoy wrote: > This is a preparation for the signature verbs API change. This change is > needed since the MR type will define, in the upcoming patches, the need > for allocating internal resources in LLD for signature handover related > operations. It will also help to make sure that signature related > functions are called with an appropriate MR type and fail otherwise. > > Signed-off-by: Max Gurtovoy <maxg@mellanox.com> > Signed-off-by: Israel Rukshin <israelr@mellanox.com> > Reviewed-by: Sagi Grimberg <sagi@grimberg.me> > --- > drivers/infiniband/core/uverbs_cmd.c | 1 + > drivers/infiniband/core/verbs.c | 1 + > include/rdma/ib_verbs.h | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c > index 3317300ab036..72c5a8daf558 100644 > --- a/drivers/infiniband/core/uverbs_cmd.c > +++ b/drivers/infiniband/core/uverbs_cmd.c > @@ -737,6 +737,7 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs) > > mr->device = pd->device; > mr->pd = pd; > + mr->type = IB_MR_TYPE_MEM_REG; > mr->dm = NULL; > mr->uobject = uobj; > atomic_inc(&pd->usecnt); > diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c > index ac011836bb54..4588b933d4b4 100644 > --- a/drivers/infiniband/core/verbs.c > +++ b/drivers/infiniband/core/verbs.c > @@ -1992,6 +1992,7 @@ struct ib_mr *ib_alloc_mr(struct ib_pd *pd, > mr->need_inval = false; > mr->res.type = RDMA_RESTRACK_MR; > rdma_restrack_kadd(&mr->res); > + mr->type = mr_type; > } > > return mr; > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > index f89b521245ec..42a9e297c21f 100644 > --- a/include/rdma/ib_verbs.h > +++ b/include/rdma/ib_verbs.h > @@ -1695,6 +1695,7 @@ struct ib_mr { > u64 iova; > u64 length; > unsigned int page_size; > + enum ib_mr_type type; > bool need_inval; > union { > struct ib_uobject *uobject; /* user */ How about __ib_alloc_pd() and UVERBS_HANDLER(UVERBS_METHOD_DM_MR_REG)()? I think those functions also create a memory region. Shouldn't mr->type be initialized from these functions too? Bart.
On Thu, Mar 7, 2019 at 2:27 PM Bart Van Assche <bvanassche@acm.org> wrote: > > On Thu, 2019-03-07 at 20:02 +0200, Max Gurtovoy wrote: > > This is a preparation for the signature verbs API change. This change is > > needed since the MR type will define, in the upcoming patches, the need > > for allocating internal resources in LLD for signature handover related > > operations. It will also help to make sure that signature related > > functions are called with an appropriate MR type and fail otherwise. > > > > Signed-off-by: Max Gurtovoy <maxg@mellanox.com> > > Signed-off-by: Israel Rukshin <israelr@mellanox.com> > > Reviewed-by: Sagi Grimberg <sagi@grimberg.me> > > --- > > drivers/infiniband/core/uverbs_cmd.c | 1 + > > drivers/infiniband/core/verbs.c | 1 + > > include/rdma/ib_verbs.h | 1 + > > 3 files changed, 3 insertions(+) > > > > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c > > index 3317300ab036..72c5a8daf558 100644 > > --- a/drivers/infiniband/core/uverbs_cmd.c > > +++ b/drivers/infiniband/core/uverbs_cmd.c > > @@ -737,6 +737,7 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs) > > > > mr->device = pd->device; > > mr->pd = pd; > > + mr->type = IB_MR_TYPE_MEM_REG; > > mr->dm = NULL; > > mr->uobject = uobj; > > atomic_inc(&pd->usecnt); > > diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c > > index ac011836bb54..4588b933d4b4 100644 > > --- a/drivers/infiniband/core/verbs.c > > +++ b/drivers/infiniband/core/verbs.c > > @@ -1992,6 +1992,7 @@ struct ib_mr *ib_alloc_mr(struct ib_pd *pd, > > mr->need_inval = false; > > mr->res.type = RDMA_RESTRACK_MR; > > rdma_restrack_kadd(&mr->res); > > + mr->type = mr_type; > > } > > > > return mr; > > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > > index f89b521245ec..42a9e297c21f 100644 > > --- a/include/rdma/ib_verbs.h > > +++ b/include/rdma/ib_verbs.h > > @@ -1695,6 +1695,7 @@ struct ib_mr { > > u64 iova; > > u64 length; > > unsigned int page_size; > > + enum ib_mr_type type; > > bool need_inval; > > union { > > struct ib_uobject *uobject; /* user */ > > How about __ib_alloc_pd() and UVERBS_HANDLER(UVERBS_METHOD_DM_MR_REG)()? > I think those functions also create a memory region. Shouldn't mr->type > be initialized from these functions too? Max?? Jason
On 3/26/2019 4:11 PM, Jason Gunthorpe wrote: > On Thu, Mar 7, 2019 at 2:27 PM Bart Van Assche <bvanassche@acm.org> wrote: >> On Thu, 2019-03-07 at 20:02 +0200, Max Gurtovoy wrote: >>> This is a preparation for the signature verbs API change. This change is >>> needed since the MR type will define, in the upcoming patches, the need >>> for allocating internal resources in LLD for signature handover related >>> operations. It will also help to make sure that signature related >>> functions are called with an appropriate MR type and fail otherwise. >>> >>> Signed-off-by: Max Gurtovoy <maxg@mellanox.com> >>> Signed-off-by: Israel Rukshin <israelr@mellanox.com> >>> Reviewed-by: Sagi Grimberg <sagi@grimberg.me> >>> --- >>> drivers/infiniband/core/uverbs_cmd.c | 1 + >>> drivers/infiniband/core/verbs.c | 1 + >>> include/rdma/ib_verbs.h | 1 + >>> 3 files changed, 3 insertions(+) >>> >>> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c >>> index 3317300ab036..72c5a8daf558 100644 >>> --- a/drivers/infiniband/core/uverbs_cmd.c >>> +++ b/drivers/infiniband/core/uverbs_cmd.c >>> @@ -737,6 +737,7 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs) >>> >>> mr->device = pd->device; >>> mr->pd = pd; >>> + mr->type = IB_MR_TYPE_MEM_REG; >>> mr->dm = NULL; >>> mr->uobject = uobj; >>> atomic_inc(&pd->usecnt); >>> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c >>> index ac011836bb54..4588b933d4b4 100644 >>> --- a/drivers/infiniband/core/verbs.c >>> +++ b/drivers/infiniband/core/verbs.c >>> @@ -1992,6 +1992,7 @@ struct ib_mr *ib_alloc_mr(struct ib_pd *pd, >>> mr->need_inval = false; >>> mr->res.type = RDMA_RESTRACK_MR; >>> rdma_restrack_kadd(&mr->res); >>> + mr->type = mr_type; >>> } >>> >>> return mr; >>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h >>> index f89b521245ec..42a9e297c21f 100644 >>> --- a/include/rdma/ib_verbs.h >>> +++ b/include/rdma/ib_verbs.h >>> @@ -1695,6 +1695,7 @@ struct ib_mr { >>> u64 iova; >>> u64 length; >>> unsigned int page_size; >>> + enum ib_mr_type type; >>> bool need_inval; >>> union { >>> struct ib_uobject *uobject; /* user */ >> How about __ib_alloc_pd() and UVERBS_HANDLER(UVERBS_METHOD_DM_MR_REG)()? >> I think those functions also create a memory region. Shouldn't mr->type >> be initialized from these functions too? > Max?? The way I see this MR type is to give a hint for LLD for resource allocations. Currently the only code that really uses these types is the kernel MR allocation functions ib_alloc_mr. Adding IB_MR_TYPE_MEM_REG to user space MRs just for saying we did it, not really helps us here. Maybe we can add a new type IB_MR_TYPE_USER for hinting LLD. Same for the dma_mr allocation in alloc_pd function. In the future we can maybe reuse code in the core layer and LLDs using these types, but for now it will help for readability. Thoughts ? Max. > > Jason
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c index 3317300ab036..72c5a8daf558 100644 --- a/drivers/infiniband/core/uverbs_cmd.c +++ b/drivers/infiniband/core/uverbs_cmd.c @@ -737,6 +737,7 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs) mr->device = pd->device; mr->pd = pd; + mr->type = IB_MR_TYPE_MEM_REG; mr->dm = NULL; mr->uobject = uobj; atomic_inc(&pd->usecnt); diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c index ac011836bb54..4588b933d4b4 100644 --- a/drivers/infiniband/core/verbs.c +++ b/drivers/infiniband/core/verbs.c @@ -1992,6 +1992,7 @@ struct ib_mr *ib_alloc_mr(struct ib_pd *pd, mr->need_inval = false; mr->res.type = RDMA_RESTRACK_MR; rdma_restrack_kadd(&mr->res); + mr->type = mr_type; } return mr; diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index f89b521245ec..42a9e297c21f 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -1695,6 +1695,7 @@ struct ib_mr { u64 iova; u64 length; unsigned int page_size; + enum ib_mr_type type; bool need_inval; union { struct ib_uobject *uobject; /* user */