| Message ID | 4a6b9d643f5298f047e3b62cad11a8c33f535529.1519688087.git.swise@opengridcomputing.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Mon, Feb 26, 2018 at 03:22:38PM -0800, Steve Wise wrote: > Setting iova, length, and page_size allows this information to be > seen via NLDEV netlink queries, which can aid in user rdma debugging. > > Signed-off-by: Steve Wise <swise@opengridcomputing.com> > --- > drivers/infiniband/hw/mlx4/mr.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/infiniband/hw/mlx4/mr.c b/drivers/infiniband/hw/mlx4/mr.c > index 4975f3e..17f4f15 100644 > --- a/drivers/infiniband/hw/mlx4/mr.c > +++ b/drivers/infiniband/hw/mlx4/mr.c > @@ -407,6 +407,9 @@ struct ib_mr *mlx4_ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length, > goto err_mr; > > mr->ibmr.rkey = mr->ibmr.lkey = mr->mmr.key; > + mr->ibmr.length = length; > + mr->ibmr.iova = virt_addr; > + mr->ibmr.page_size = 1U << shift; I wonder how many other drivers will have updates like this. > > return &mr->ibmr; > > -- > 1.8.3.1 >
> > On Mon, Feb 26, 2018 at 03:22:38PM -0800, Steve Wise wrote: > > Setting iova, length, and page_size allows this information to be > > seen via NLDEV netlink queries, which can aid in user rdma debugging. > > > > Signed-off-by: Steve Wise <swise@opengridcomputing.com> > > --- > > drivers/infiniband/hw/mlx4/mr.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/infiniband/hw/mlx4/mr.c > b/drivers/infiniband/hw/mlx4/mr.c > > index 4975f3e..17f4f15 100644 > > --- a/drivers/infiniband/hw/mlx4/mr.c > > +++ b/drivers/infiniband/hw/mlx4/mr.c > > @@ -407,6 +407,9 @@ struct ib_mr *mlx4_ib_reg_user_mr(struct ib_pd *pd, > u64 start, u64 length, > > goto err_mr; > > > > mr->ibmr.rkey = mr->ibmr.lkey = mr->mmr.key; > > + mr->ibmr.length = length; > > + mr->ibmr.iova = virt_addr; > > + mr->ibmr.page_size = 1U << shift; > > I wonder how many other drivers will have updates like this. I can't test the other drivers for lack of hardware. They can be fixed as the vendors discover them. OR, we don't dump any of this for MRs. And with the up and coming provider-specific resources, that will probably be dumped anyway. Should I just remove dumping of length/iova/page_size? -- 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, Feb 28, 2018 at 09:51:08AM -0600, Steve Wise wrote: > > > > On Mon, Feb 26, 2018 at 03:22:38PM -0800, Steve Wise wrote: > > > Setting iova, length, and page_size allows this information to be > > > seen via NLDEV netlink queries, which can aid in user rdma debugging. > > > > > > Signed-off-by: Steve Wise <swise@opengridcomputing.com> > > > drivers/infiniband/hw/mlx4/mr.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/infiniband/hw/mlx4/mr.c > > b/drivers/infiniband/hw/mlx4/mr.c > > > index 4975f3e..17f4f15 100644 > > > +++ b/drivers/infiniband/hw/mlx4/mr.c > > > @@ -407,6 +407,9 @@ struct ib_mr *mlx4_ib_reg_user_mr(struct ib_pd *pd, > > u64 start, u64 length, > > > goto err_mr; > > > > > > mr->ibmr.rkey = mr->ibmr.lkey = mr->mmr.key; > > > + mr->ibmr.length = length; > > > + mr->ibmr.iova = virt_addr; > > > + mr->ibmr.page_size = 1U << shift; > > > > I wonder how many other drivers will have updates like this. > > I can't test the other drivers for lack of hardware. They can be fixed as > the vendors discover them. OR, we don't dump any of this for MRs. And with > the up and coming provider-specific resources, that will probably be dumped > anyway. > > Should I just remove dumping of length/iova/page_size? We certainly have a problem if iova/length is uninited kernel memory in any driver. :( 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
> -----Original Message----- > From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma- > owner@vger.kernel.org] On Behalf Of Jason Gunthorpe > Sent: Wednesday, February 28, 2018 9:35 AM > To: Steve Wise <swise@opengridcomputing.com> > Cc: 'Leon Romanovsky' <leon@kernel.org>; dledford@redhat.com; Yishai Hadas > <yishaih@mellanox.com>; linux-rdma@vger.kernel.org > Subject: Re: [PATCH v3 rdma-next 07/10] mlx4_ib: set user mr attributes in struct > ib_mr > > On Wed, Feb 28, 2018 at 09:51:08AM -0600, Steve Wise wrote: > > > > > > On Mon, Feb 26, 2018 at 03:22:38PM -0800, Steve Wise wrote: > > > > Setting iova, length, and page_size allows this information to be > > > > seen via NLDEV netlink queries, which can aid in user rdma debugging. > > > > > > > > Signed-off-by: Steve Wise <swise@opengridcomputing.com> > > > > drivers/infiniband/hw/mlx4/mr.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/drivers/infiniband/hw/mlx4/mr.c > > > b/drivers/infiniband/hw/mlx4/mr.c > > > > index 4975f3e..17f4f15 100644 > > > > +++ b/drivers/infiniband/hw/mlx4/mr.c > > > > @@ -407,6 +407,9 @@ struct ib_mr *mlx4_ib_reg_user_mr(struct ib_pd > > > > *pd, > > > u64 start, u64 length, > > > > goto err_mr; > > > > > > > > mr->ibmr.rkey = mr->ibmr.lkey = mr->mmr.key; > > > > + mr->ibmr.length = length; > > > > + mr->ibmr.iova = virt_addr; > > > > + mr->ibmr.page_size = 1U << shift; > > > > > > I wonder how many other drivers will have updates like this. > > > > I can't test the other drivers for lack of hardware. They can be > > fixed as the vendors discover them. OR, we don't dump any of this for > > MRs. And with the up and coming provider-specific resources, that > > will probably be dumped anyway. > > > > Should I just remove dumping of length/iova/page_size? > > We certainly have a problem if iova/length is uninited kernel memory in any > driver. :( > Given that iova and length are user provided inputs to register a MR, shouldn't they be stored to mr->ibmr by ib_core and not by every provider drivers? -- 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 Mon, Feb 26, 2018 at 03:22:38PM -0800, Steve Wise wrote: > > > > > Setting iova, length, and page_size allows this information to be > > > > > seen via NLDEV netlink queries, which can aid in user rdma debugging. > > > > > > > > > > Signed-off-by: Steve Wise <swise@opengridcomputing.com> > > > > > drivers/infiniband/hw/mlx4/mr.c | 3 +++ > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > diff --git a/drivers/infiniband/hw/mlx4/mr.c > > > > b/drivers/infiniband/hw/mlx4/mr.c > > > > > index 4975f3e..17f4f15 100644 > > > > > +++ b/drivers/infiniband/hw/mlx4/mr.c > > > > > @@ -407,6 +407,9 @@ struct ib_mr *mlx4_ib_reg_user_mr(struct ib_pd > > > > > *pd, > > > > u64 start, u64 length, > > > > > goto err_mr; > > > > > > > > > > mr->ibmr.rkey = mr->ibmr.lkey = mr->mmr.key; > > > > > + mr->ibmr.length = length; > > > > > + mr->ibmr.iova = virt_addr; > > > > > + mr->ibmr.page_size = 1U << shift; > > > > > > > > I wonder how many other drivers will have updates like this. > > > > > > I can't test the other drivers for lack of hardware. They can be > > > fixed as the vendors discover them. OR, we don't dump any of this for > > > MRs. And with the up and coming provider-specific resources, that > > > will probably be dumped anyway. > > > > > > Should I just remove dumping of length/iova/page_size? > > > > We certainly have a problem if iova/length is uninited kernel memory in any > > driver. :( > > > Given that iova and length are user provided inputs to register a MR, shouldn't > they be stored to mr->ibmr by ib_core and not by every provider drivers? The page_size is computed by the provider, so that field, at least should be filled in by the provider. -- 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
> -----Original Message----- > From: Steve Wise [mailto:swise@opengridcomputing.com] > Sent: Wednesday, February 28, 2018 9:53 AM > To: Parav Pandit <parav@mellanox.com>; Jason Gunthorpe > <jgg@mellanox.com> > Cc: 'Leon Romanovsky' <leon@kernel.org>; dledford@redhat.com; Yishai Hadas > <yishaih@mellanox.com>; linux-rdma@vger.kernel.org > Subject: RE: [PATCH v3 rdma-next 07/10] mlx4_ib: set user mr attributes in struct > ib_mr > > > > > > > > > > > On Mon, Feb 26, 2018 at 03:22:38PM -0800, Steve Wise wrote: > > > > > > Setting iova, length, and page_size allows this information to > > > > > > be seen via NLDEV netlink queries, which can aid in user rdma > debugging. > > > > > > > > > > > > Signed-off-by: Steve Wise <swise@opengridcomputing.com> > > > > > > drivers/infiniband/hw/mlx4/mr.c | 3 +++ > > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > > > diff --git a/drivers/infiniband/hw/mlx4/mr.c > > > > > b/drivers/infiniband/hw/mlx4/mr.c > > > > > > index 4975f3e..17f4f15 100644 > > > > > > +++ b/drivers/infiniband/hw/mlx4/mr.c > > > > > > @@ -407,6 +407,9 @@ struct ib_mr *mlx4_ib_reg_user_mr(struct > > > > > > ib_pd *pd, > > > > > u64 start, u64 length, > > > > > > goto err_mr; > > > > > > > > > > > > mr->ibmr.rkey = mr->ibmr.lkey = mr->mmr.key; > > > > > > + mr->ibmr.length = length; > > > > > > + mr->ibmr.iova = virt_addr; > > > > > > + mr->ibmr.page_size = 1U << shift; > > > > > > > > > > I wonder how many other drivers will have updates like this. > > > > > > > > I can't test the other drivers for lack of hardware. They can be > > > > fixed as the vendors discover them. OR, we don't dump any of this > > > > for MRs. And with the up and coming provider-specific resources, > > > > that will probably be dumped anyway. > > > > > > > > Should I just remove dumping of length/iova/page_size? > > > > > > We certainly have a problem if iova/length is uninited kernel memory > > > in > any > > > driver. :( > > > > > Given that iova and length are user provided inputs to register a MR, > shouldn't > > they be stored to mr->ibmr by ib_core and not by every provider drivers? > > The page_size is computed by the provider, so that field, at least should be filled > in by the provider. > Yes, so that can be in ib_mr.provider.page_size field that provider driver can fill up. > -- 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 Mon, Feb 26, 2018 at 03:22:38PM -0800, Steve Wise wrote: > > > > > > > Setting iova, length, and page_size allows this information to > > > > > > > be seen via NLDEV netlink queries, which can aid in user rdma > > debugging. > > > > > > > > > > > > > > Signed-off-by: Steve Wise <swise@opengridcomputing.com> > > > > > > > drivers/infiniband/hw/mlx4/mr.c | 3 +++ > > > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > > > > > diff --git a/drivers/infiniband/hw/mlx4/mr.c > > > > > > b/drivers/infiniband/hw/mlx4/mr.c > > > > > > > index 4975f3e..17f4f15 100644 > > > > > > > +++ b/drivers/infiniband/hw/mlx4/mr.c > > > > > > > @@ -407,6 +407,9 @@ struct ib_mr *mlx4_ib_reg_user_mr(struct > > > > > > > ib_pd *pd, > > > > > > u64 start, u64 length, > > > > > > > goto err_mr; > > > > > > > > > > > > > > mr->ibmr.rkey = mr->ibmr.lkey = mr->mmr.key; > > > > > > > + mr->ibmr.length = length; > > > > > > > + mr->ibmr.iova = virt_addr; > > > > > > > + mr->ibmr.page_size = 1U << shift; > > > > > > > > > > > > I wonder how many other drivers will have updates like this. > > > > > > > > > > I can't test the other drivers for lack of hardware. They can be > > > > > fixed as the vendors discover them. OR, we don't dump any of this > > > > > for MRs. And with the up and coming provider-specific resources, > > > > > that will probably be dumped anyway. > > > > > > > > > > Should I just remove dumping of length/iova/page_size? > > > > > > > > We certainly have a problem if iova/length is uninited kernel memory > > > > in > > any > > > > driver. :( > > > > > > > Given that iova and length are user provided inputs to register a MR, > > shouldn't > > > they be stored to mr->ibmr by ib_core and not by every provider drivers? > > > > The page_size is computed by the provider, so that field, at least should be > filled > > in by the provider. > > > Yes, so that can be in ib_mr.provider.page_size field that provider driver can fill > up. > Are you proposing adding this to the ib_mr struct? The provider can simply set ib_mr.page_size just like it already sets rkey and lkey. Right? -- 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
> -----Original Message----- > From: Steve Wise [mailto:swise@opengridcomputing.com] > Sent: Wednesday, February 28, 2018 9:58 AM > To: Parav Pandit <parav@mellanox.com>; Jason Gunthorpe > <jgg@mellanox.com> > Cc: 'Leon Romanovsky' <leon@kernel.org>; dledford@redhat.com; Yishai Hadas > <yishaih@mellanox.com>; linux-rdma@vger.kernel.org > Subject: RE: [PATCH v3 rdma-next 07/10] mlx4_ib: set user mr attributes in struct > ib_mr > > > > > > > > > > > > > > > On Mon, Feb 26, 2018 at 03:22:38PM -0800, Steve Wise wrote: > > > > > > > > Setting iova, length, and page_size allows this > > > > > > > > information to be seen via NLDEV netlink queries, which > > > > > > > > can aid in user rdma > > > debugging. > > > > > > > > > > > > > > > > Signed-off-by: Steve Wise <swise@opengridcomputing.com> > > > > > > > > drivers/infiniband/hw/mlx4/mr.c | 3 +++ > > > > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > > > > > > > diff --git a/drivers/infiniband/hw/mlx4/mr.c > > > > > > > b/drivers/infiniband/hw/mlx4/mr.c > > > > > > > > index 4975f3e..17f4f15 100644 > > > > > > > > +++ b/drivers/infiniband/hw/mlx4/mr.c > > > > > > > > @@ -407,6 +407,9 @@ struct ib_mr > > > > > > > > *mlx4_ib_reg_user_mr(struct ib_pd *pd, > > > > > > > u64 start, u64 length, > > > > > > > > goto err_mr; > > > > > > > > > > > > > > > > mr->ibmr.rkey = mr->ibmr.lkey = mr->mmr.key; > > > > > > > > + mr->ibmr.length = length; > > > > > > > > + mr->ibmr.iova = virt_addr; > > > > > > > > + mr->ibmr.page_size = 1U << shift; > > > > > > > > > > > > > > I wonder how many other drivers will have updates like this. > > > > > > > > > > > > I can't test the other drivers for lack of hardware. They can > > > > > > be fixed as the vendors discover them. OR, we don't dump any > > > > > > of this for MRs. And with the up and coming provider-specific > > > > > > resources, that will probably be dumped anyway. > > > > > > > > > > > > Should I just remove dumping of length/iova/page_size? > > > > > > > > > > We certainly have a problem if iova/length is uninited kernel > > > > > memory in > > > any > > > > > driver. :( > > > > > > > > > Given that iova and length are user provided inputs to register a > > > > MR, > > > shouldn't > > > > they be stored to mr->ibmr by ib_core and not by every provider > drivers? > > > > > > The page_size is computed by the provider, so that field, at least > should be > > filled > > > in by the provider. > > > > > Yes, so that can be in ib_mr.provider.page_size field that provider > > driver > can fill > > up. > > > > Are you proposing adding this to the ib_mr struct? The provider can simply set > ib_mr.page_size just like it already sets rkey and lkey. Right? > Yes, because this makes code clear of what is owned/set by provider. Rkey, lkey is being used by ULPs too, so I am not up for that refactor. No gain there. But new fields like provider specific will be helpful in their own structure. -- 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
> > -----Original Message----- > > From: Steve Wise [mailto:swise@opengridcomputing.com] > > Sent: Wednesday, February 28, 2018 9:58 AM > > To: Parav Pandit <parav@mellanox.com>; Jason Gunthorpe > > <jgg@mellanox.com> > > Cc: 'Leon Romanovsky' <leon@kernel.org>; dledford@redhat.com; Yishai > Hadas > > <yishaih@mellanox.com>; linux-rdma@vger.kernel.org > > Subject: RE: [PATCH v3 rdma-next 07/10] mlx4_ib: set user mr attributes in > struct > > ib_mr > > > > > > > > > > > > > > > > > > On Mon, Feb 26, 2018 at 03:22:38PM -0800, Steve Wise wrote: > > > > > > > > > Setting iova, length, and page_size allows this > > > > > > > > > information to be seen via NLDEV netlink queries, which > > > > > > > > > can aid in user rdma > > > > debugging. > > > > > > > > > > > > > > > > > > Signed-off-by: Steve Wise <swise@opengridcomputing.com> > > > > > > > > > drivers/infiniband/hw/mlx4/mr.c | 3 +++ > > > > > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > > > > > > > > > diff --git a/drivers/infiniband/hw/mlx4/mr.c > > > > > > > > b/drivers/infiniband/hw/mlx4/mr.c > > > > > > > > > index 4975f3e..17f4f15 100644 > > > > > > > > > +++ b/drivers/infiniband/hw/mlx4/mr.c > > > > > > > > > @@ -407,6 +407,9 @@ struct ib_mr > > > > > > > > > *mlx4_ib_reg_user_mr(struct ib_pd *pd, > > > > > > > > u64 start, u64 length, > > > > > > > > > goto err_mr; > > > > > > > > > > > > > > > > > > mr->ibmr.rkey = mr->ibmr.lkey = mr->mmr.key; > > > > > > > > > + mr->ibmr.length = length; > > > > > > > > > + mr->ibmr.iova = virt_addr; > > > > > > > > > + mr->ibmr.page_size = 1U << shift; > > > > > > > > > > > > > > > > I wonder how many other drivers will have updates like this. > > > > > > > > > > > > > > I can't test the other drivers for lack of hardware. They can > > > > > > > be fixed as the vendors discover them. OR, we don't dump any > > > > > > > of this for MRs. And with the up and coming provider-specific > > > > > > > resources, that will probably be dumped anyway. > > > > > > > > > > > > > > Should I just remove dumping of length/iova/page_size? > > > > > > > > > > > > We certainly have a problem if iova/length is uninited kernel > > > > > > memory in > > > > any > > > > > > driver. :( > > > > > > > > > > > Given that iova and length are user provided inputs to register a > > > > > MR, > > > > shouldn't > > > > > they be stored to mr->ibmr by ib_core and not by every provider > > drivers? > > > > > > > > The page_size is computed by the provider, so that field, at least > > should be > > > filled > > > > in by the provider. > > > > > > > Yes, so that can be in ib_mr.provider.page_size field that provider > > > driver > > can fill > > > up. > > > > > > > Are you proposing adding this to the ib_mr struct? The provider can simply set > > ib_mr.page_size just like it already sets rkey and lkey. Right? > > > Yes, because this makes code clear of what is owned/set by provider. > Rkey, lkey is being used by ULPs too, so I am not up for that refactor. No gain > there. > But new fields like provider specific will be helpful in their own structure. These aren't new fields. They been in ib_mr since day one. I don't want to refactor all the rdma structs dividing up which bits are provider-dependent. Not in this patch series. Steve. -- 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
> -----Original Message----- > From: Steve Wise [mailto:swise@opengridcomputing.com] > Sent: Wednesday, February 28, 2018 10:19 AM > To: Parav Pandit <parav@mellanox.com>; Jason Gunthorpe > <jgg@mellanox.com> > Cc: 'Leon Romanovsky' <leon@kernel.org>; dledford@redhat.com; Yishai Hadas > <yishaih@mellanox.com>; linux-rdma@vger.kernel.org > Subject: RE: [PATCH v3 rdma-next 07/10] mlx4_ib: set user mr attributes in struct > ib_mr > > > > -----Original Message----- > > > From: Steve Wise [mailto:swise@opengridcomputing.com] > > > Sent: Wednesday, February 28, 2018 9:58 AM > > > To: Parav Pandit <parav@mellanox.com>; Jason Gunthorpe > > > <jgg@mellanox.com> > > > Cc: 'Leon Romanovsky' <leon@kernel.org>; dledford@redhat.com; Yishai > > Hadas > > > <yishaih@mellanox.com>; linux-rdma@vger.kernel.org > > > Subject: RE: [PATCH v3 rdma-next 07/10] mlx4_ib: set user mr > > > attributes > in > > struct > > > ib_mr > > > > > > > > > > > > > > > > > > > > > On Mon, Feb 26, 2018 at 03:22:38PM -0800, Steve Wise wrote: > > > > > > > > > > Setting iova, length, and page_size allows this > > > > > > > > > > information to be seen via NLDEV netlink queries, > > > > > > > > > > which can aid in user rdma > > > > > debugging. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Steve Wise > > > > > > > > > > <swise@opengridcomputing.com> > > > > > > > > > > drivers/infiniband/hw/mlx4/mr.c | 3 +++ > > > > > > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > > > > > > > > > > > diff --git a/drivers/infiniband/hw/mlx4/mr.c > > > > > > > > > b/drivers/infiniband/hw/mlx4/mr.c > > > > > > > > > > index 4975f3e..17f4f15 100644 > > > > > > > > > > +++ b/drivers/infiniband/hw/mlx4/mr.c > > > > > > > > > > @@ -407,6 +407,9 @@ struct ib_mr > > > > > > > > > > *mlx4_ib_reg_user_mr(struct ib_pd *pd, > > > > > > > > > u64 start, u64 length, > > > > > > > > > > goto err_mr; > > > > > > > > > > > > > > > > > > > > mr->ibmr.rkey = mr->ibmr.lkey = mr->mmr.key; > > > > > > > > > > + mr->ibmr.length = length; > > > > > > > > > > + mr->ibmr.iova = virt_addr; > > > > > > > > > > + mr->ibmr.page_size = 1U << shift; > > > > > > > > > > > > > > > > > > I wonder how many other drivers will have updates like this. > > > > > > > > > > > > > > > > I can't test the other drivers for lack of hardware. They > > > > > > > > can be fixed as the vendors discover them. OR, we don't > > > > > > > > dump any of this for MRs. And with the up and coming > > > > > > > > provider-specific resources, that will probably be dumped anyway. > > > > > > > > > > > > > > > > Should I just remove dumping of length/iova/page_size? > > > > > > > > > > > > > > We certainly have a problem if iova/length is uninited > > > > > > > kernel memory in > > > > > any > > > > > > > driver. :( > > > > > > > > > > > > > Given that iova and length are user provided inputs to > > > > > > register a MR, > > > > > shouldn't > > > > > > they be stored to mr->ibmr by ib_core and not by every > > > > > > provider > > > drivers? > > > > > > > > > > The page_size is computed by the provider, so that field, at > > > > > least > > > should be > > > > filled > > > > > in by the provider. > > > > > > > > > Yes, so that can be in ib_mr.provider.page_size field that > > > > provider driver > > > can fill > > > > up. > > > > > > > > > > Are you proposing adding this to the ib_mr struct? The provider can > simply set > > > ib_mr.page_size just like it already sets rkey and lkey. Right? > > > > > Yes, because this makes code clear of what is owned/set by provider. > > Rkey, lkey is being used by ULPs too, so I am not up for that > > refactor. No > gain > > there. > > But new fields like provider specific will be helpful in their own > structure. > > These aren't new fields. They been in ib_mr since day one. I don't want to > refactor all the rdma structs dividing up which bits are provider-dependent. > Not in this patch series. > I missed this part that page_size is already in ib_mr. Yes, no refactor in this series. page_size setting from provider looks fine. iova and length should be done by ib_core in this or other series. > Steve. -- 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, Feb 28, 2018 at 09:51:08AM -0600, Steve Wise wrote: > > > > On Mon, Feb 26, 2018 at 03:22:38PM -0800, Steve Wise wrote: > > > Setting iova, length, and page_size allows this information to be > > > seen via NLDEV netlink queries, which can aid in user rdma debugging. > > > > > > Signed-off-by: Steve Wise <swise@opengridcomputing.com> > > > --- > > > drivers/infiniband/hw/mlx4/mr.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/infiniband/hw/mlx4/mr.c > > b/drivers/infiniband/hw/mlx4/mr.c > > > index 4975f3e..17f4f15 100644 > > > --- a/drivers/infiniband/hw/mlx4/mr.c > > > +++ b/drivers/infiniband/hw/mlx4/mr.c > > > @@ -407,6 +407,9 @@ struct ib_mr *mlx4_ib_reg_user_mr(struct ib_pd *pd, > > u64 start, u64 length, > > > goto err_mr; > > > > > > mr->ibmr.rkey = mr->ibmr.lkey = mr->mmr.key; > > > + mr->ibmr.length = length; > > > + mr->ibmr.iova = virt_addr; > > > + mr->ibmr.page_size = 1U << shift; > > > > I wonder how many other drivers will have updates like this. > > I can't test the other drivers for lack of hardware. They can be fixed as > the vendors discover them. OR, we don't dump any of this for MRs. And with > the up and coming provider-specific resources, that will probably be dumped > anyway. > > Should I just remove dumping of length/iova/page_size? I don't think so, other drivers authors should fix it. Thanks > > > >
diff --git a/drivers/infiniband/hw/mlx4/mr.c b/drivers/infiniband/hw/mlx4/mr.c index 4975f3e..17f4f15 100644 --- a/drivers/infiniband/hw/mlx4/mr.c +++ b/drivers/infiniband/hw/mlx4/mr.c @@ -407,6 +407,9 @@ struct ib_mr *mlx4_ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length, goto err_mr; mr->ibmr.rkey = mr->ibmr.lkey = mr->mmr.key; + mr->ibmr.length = length; + mr->ibmr.iova = virt_addr; + mr->ibmr.page_size = 1U << shift; return &mr->ibmr;
Setting iova, length, and page_size allows this information to be seen via NLDEV netlink queries, which can aid in user rdma debugging. Signed-off-by: Steve Wise <swise@opengridcomputing.com> --- drivers/infiniband/hw/mlx4/mr.c | 3 +++ 1 file changed, 3 insertions(+)