| Message ID | 20180314203942.GA6563@ziepe.ca (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
On Wed, Mar 14, 2018 at 02:39:42PM -0600, Jason Gunthorpe wrote: > Open coding a loose value size is not acceptable for describing > the uABI in RDMA. Provide the missing struct. > > Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> > --- > drivers/infiniband/hw/hns/hns_roce_pd.c | 5 ++++- > include/uapi/rdma/hns-abi.h | 5 +++++ > 2 files changed, 9 insertions(+), 1 deletion(-) > > hns team: This patch goes along with the remarks in my prior email, > structs should always be used with hns_roce_ib_alloc_pd_resp. Please > ensure whatever you come up with follows that principle. > > Thanks, > Jason > > diff --git a/drivers/infiniband/hw/hns/hns_roce_pd.c b/drivers/infiniband/hw/hns/hns_roce_pd.c > index bdab2188c04a9a..4b41e041799cae 100644 > --- a/drivers/infiniband/hw/hns/hns_roce_pd.c > +++ b/drivers/infiniband/hw/hns/hns_roce_pd.c > @@ -32,6 +32,7 @@ > > #include <linux/platform_device.h> > #include <linux/pci.h> > +#include <uapi/rdma/hns-abi.h> > #include "hns_roce_device.h" > > static int hns_roce_pd_alloc(struct hns_roce_dev *hr_dev, unsigned long *pdn) > @@ -77,7 +78,9 @@ struct ib_pd *hns_roce_alloc_pd(struct ib_device *ib_dev, > } > > if (context) { > - if (ib_copy_to_udata(udata, &pd->pdn, sizeof(u64))) { > + struct hns_roce_ib_alloc_pd_resp uresp = {.pdn = pd->pdn}; > + > + if (ib_copy_to_udata(udata, &uresp, sizeof(uresp))) { > hns_roce_pd_free(to_hr_dev(ib_dev), pd->pdn); > dev_err(dev, "[alloc_pd]ib_copy_to_udata failed!\n"); > kfree(pd); > diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h > index 4df5b79c9e11a3..a464d7945ebb48 100644 > --- a/include/uapi/rdma/hns-abi.h > +++ b/include/uapi/rdma/hns-abi.h > @@ -64,4 +64,9 @@ struct hns_roce_ib_alloc_ucontext_resp { > __u32 qp_tab_size; > __u32 reserved; > }; > + > +struct hns_roce_ib_alloc_pd_resp { > + __u32 pdn; > +}; Don't we want to pad this struct to 64bits? Thanks > + > #endif /* HNS_ABI_USER_H */ > -- > 2.16.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 15, 2018 at 12:46:55PM +0200, Leon Romanovsky wrote: > On Wed, Mar 14, 2018 at 02:39:42PM -0600, Jason Gunthorpe wrote: > > Open coding a loose value size is not acceptable for describing > > the uABI in RDMA. Provide the missing struct. > > > > Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> > > drivers/infiniband/hw/hns/hns_roce_pd.c | 5 ++++- > > include/uapi/rdma/hns-abi.h | 5 +++++ > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > hns team: This patch goes along with the remarks in my prior email, > > structs should always be used with hns_roce_ib_alloc_pd_resp. Please > > ensure whatever you come up with follows that principle. > > > > Thanks, > > Jason > > > > diff --git a/drivers/infiniband/hw/hns/hns_roce_pd.c b/drivers/infiniband/hw/hns/hns_roce_pd.c > > index bdab2188c04a9a..4b41e041799cae 100644 > > +++ b/drivers/infiniband/hw/hns/hns_roce_pd.c > > @@ -32,6 +32,7 @@ > > > > #include <linux/platform_device.h> > > #include <linux/pci.h> > > +#include <uapi/rdma/hns-abi.h> > > #include "hns_roce_device.h" > > > > static int hns_roce_pd_alloc(struct hns_roce_dev *hr_dev, unsigned long *pdn) > > @@ -77,7 +78,9 @@ struct ib_pd *hns_roce_alloc_pd(struct ib_device *ib_dev, > > } > > > > if (context) { > > - if (ib_copy_to_udata(udata, &pd->pdn, sizeof(u64))) { > > + struct hns_roce_ib_alloc_pd_resp uresp = {.pdn = pd->pdn}; > > + > > + if (ib_copy_to_udata(udata, &uresp, sizeof(uresp))) { > > hns_roce_pd_free(to_hr_dev(ib_dev), pd->pdn); > > dev_err(dev, "[alloc_pd]ib_copy_to_udata failed!\n"); > > kfree(pd); > > diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h > > index 4df5b79c9e11a3..a464d7945ebb48 100644 > > +++ b/include/uapi/rdma/hns-abi.h > > @@ -64,4 +64,9 @@ struct hns_roce_ib_alloc_ucontext_resp { > > __u32 qp_tab_size; > > __u32 reserved; > > }; > > + > > +struct hns_roce_ib_alloc_pd_resp { > > + __u32 pdn; > > +}; > > Don't we want to pad this struct to 64bits? It isn't padded right now.. I have another series that adds the missing padding. Can't make that series until we actually have a properly described uapi. If I recall, it turns out the padding is not technically required here anyhow. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 14, 2018 at 02:39:42PM -0600, Jason Gunthorpe wrote: > Open coding a loose value size is not acceptable for describing > the uABI in RDMA. Provide the missing struct. > > Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> > drivers/infiniband/hw/hns/hns_roce_pd.c | 5 ++++- > include/uapi/rdma/hns-abi.h | 5 +++++ > 2 files changed, 9 insertions(+), 1 deletion(-) Applied to for-next Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/hw/hns/hns_roce_pd.c b/drivers/infiniband/hw/hns/hns_roce_pd.c index bdab2188c04a9a..4b41e041799cae 100644 --- a/drivers/infiniband/hw/hns/hns_roce_pd.c +++ b/drivers/infiniband/hw/hns/hns_roce_pd.c @@ -32,6 +32,7 @@ #include <linux/platform_device.h> #include <linux/pci.h> +#include <uapi/rdma/hns-abi.h> #include "hns_roce_device.h" static int hns_roce_pd_alloc(struct hns_roce_dev *hr_dev, unsigned long *pdn) @@ -77,7 +78,9 @@ struct ib_pd *hns_roce_alloc_pd(struct ib_device *ib_dev, } if (context) { - if (ib_copy_to_udata(udata, &pd->pdn, sizeof(u64))) { + struct hns_roce_ib_alloc_pd_resp uresp = {.pdn = pd->pdn}; + + if (ib_copy_to_udata(udata, &uresp, sizeof(uresp))) { hns_roce_pd_free(to_hr_dev(ib_dev), pd->pdn); dev_err(dev, "[alloc_pd]ib_copy_to_udata failed!\n"); kfree(pd); diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h index 4df5b79c9e11a3..a464d7945ebb48 100644 --- a/include/uapi/rdma/hns-abi.h +++ b/include/uapi/rdma/hns-abi.h @@ -64,4 +64,9 @@ struct hns_roce_ib_alloc_ucontext_resp { __u32 qp_tab_size; __u32 reserved; }; + +struct hns_roce_ib_alloc_pd_resp { + __u32 pdn; +}; + #endif /* HNS_ABI_USER_H */
Open coding a loose value size is not acceptable for describing the uABI in RDMA. Provide the missing struct. Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> --- drivers/infiniband/hw/hns/hns_roce_pd.c | 5 ++++- include/uapi/rdma/hns-abi.h | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) hns team: This patch goes along with the remarks in my prior email, structs should always be used with hns_roce_ib_alloc_pd_resp. Please ensure whatever you come up with follows that principle. Thanks, Jason