[PATCH] ntb_netdev: set the net_device's parent

[PATCH] ntb_netdev: set the net_device's parent

[Logan Gunthorpe]

At present, ntb_netdev devices end up under /sys/devices/virtual/net
completely unconnected to the ntb trees below them. This patch sets the
parent of the net_device (using SET_NETDEV_DEV) to the client_dev
device. This results in a better connected sysfs path for the network
device:

/sys/devices/pci0000:00/0000:00:03.0/0000:03:00.1/0000:03:00.1/ntb_netdev0/net/eth2

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Jon Mason <jdmason@kudzu.us>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Allen Hubbe <Allen.Hubbe@emc.com>
---
 drivers/net/ntb_netdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
index 4daf3d0926a8..0250aa9ae2cb 100644
--- a/drivers/net/ntb_netdev.c
+++ b/drivers/net/ntb_netdev.c
@@ -418,6 +418,8 @@ static int ntb_netdev_probe(struct device *client_dev)
 	if (!ndev)
 		return -ENOMEM;
 
+	SET_NETDEV_DEV(ndev, client_dev);
+
 	dev = netdev_priv(ndev);
 	dev->ndev = ndev;
 	dev->pdev = pdev;
-- 
2.11.0

[PATCH] ntb_transport: fix qp count bug

[Logan Gunthorpe]

In cases where there are more mw's than spads/2-2, the mw count gets
reduced to match the limitation. ntb_transport also tries to ensure that
there are fewer qps than mws but uses the full mw count instead of
the reduced one. When this happens, the math in
'ntb_transport_setup_qp_mw' will get confused and result in a kernel
paging request bug.

This patch fixes the bug by reducing qp_count to the reduced mw count
instead of the full mw count.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Jon Mason <jdmason@kudzu.us>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Allen Hubbe <Allen.Hubbe@emc.com>
---
 drivers/ntb/ntb_transport.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 02ca45fdd892..0a778d2cab94 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -1128,8 +1128,8 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
 	qp_count = ilog2(qp_bitmap);
 	if (max_num_clients && max_num_clients < qp_count)
 		qp_count = max_num_clients;
-	else if (mw_count < qp_count)
-		qp_count = mw_count;
+	else if (nt->mw_count < qp_count)
+		qp_count = nt->mw_count;
 
 	qp_bitmap &= BIT_ULL(qp_count) - 1;
 
-- 
2.11.0

RE: [PATCH] ntb_transport: fix qp count bug

[Allen Hubbe]

From: Logan Gunthorpe
> In cases where there are more mw's than spads/2-2, the mw count gets
> reduced to match the limitation. ntb_transport also tries to ensure that
> there are fewer qps than mws but uses the full mw count instead of
> the reduced one. When this happens, the math in
> 'ntb_transport_setup_qp_mw' will get confused and result in a kernel
> paging request bug.
> 
> This patch fixes the bug by reducing qp_count to the reduced mw count
> instead of the full mw count.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Jon Mason <jdmason@kudzu.us>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Allen Hubbe <Allen.Hubbe@emc.com>

Acked-by: Allen Hubbe <Allen.Hubbe@dell.com>

> ---
>  drivers/ntb/ntb_transport.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 02ca45fdd892..0a778d2cab94 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -1128,8 +1128,8 @@ static int ntb_transport_probe(struct ntb_client *self, struct
> ntb_dev *ndev)
>  	qp_count = ilog2(qp_bitmap);
>  	if (max_num_clients && max_num_clients < qp_count)
>  		qp_count = max_num_clients;
> -	else if (mw_count < qp_count)
> -		qp_count = mw_count;
> +	else if (nt->mw_count < qp_count)
> +		qp_count = nt->mw_count;
> 
>  	qp_bitmap &= BIT_ULL(qp_count) - 1;
> 
> --
> 2.11.0

Re: [PATCH] ntb_transport: fix qp count bug

[Jon Mason]

On Tue, Jun 06, 2017 at 09:08:47AM -0400, Allen Hubbe wrote:
> From: Logan Gunthorpe
> > In cases where there are more mw's than spads/2-2, the mw count gets
> > reduced to match the limitation. ntb_transport also tries to ensure that
> > there are fewer qps than mws but uses the full mw count instead of
> > the reduced one. When this happens, the math in
> > 'ntb_transport_setup_qp_mw' will get confused and result in a kernel
> > paging request bug.
> > 
> > This patch fixes the bug by reducing qp_count to the reduced mw count
> > instead of the full mw count.
> > 
> > Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> > Cc: Jon Mason <jdmason@kudzu.us>
> > Cc: Dave Jiang <dave.jiang@intel.com>
> > Cc: Allen Hubbe <Allen.Hubbe@emc.com>
> 
> Acked-by: Allen Hubbe <Allen.Hubbe@dell.com>

Adding to my ntb branch with the following line:
Fixes: e26a5843f7f5 ("NTB: Split ntb_hw_intel and ntb_transport drivers")

This enables the stable kernels it is a bug fix and to pick it up.

Thanks,
Jon
> 
> > ---
> >  drivers/ntb/ntb_transport.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> > index 02ca45fdd892..0a778d2cab94 100644
> > --- a/drivers/ntb/ntb_transport.c
> > +++ b/drivers/ntb/ntb_transport.c
> > @@ -1128,8 +1128,8 @@ static int ntb_transport_probe(struct ntb_client *self, struct
> > ntb_dev *ndev)
> >  	qp_count = ilog2(qp_bitmap);
> >  	if (max_num_clients && max_num_clients < qp_count)
> >  		qp_count = max_num_clients;
> > -	else if (mw_count < qp_count)
> > -		qp_count = mw_count;
> > +	else if (nt->mw_count < qp_count)
> > +		qp_count = nt->mw_count;
> > 
> >  	qp_bitmap &= BIT_ULL(qp_count) - 1;
> > 
> > --
> > 2.11.0
> 
> 

[PATCH] ntb_transport: fix bug calculating num_qps_mw

[Logan Gunthorpe]

A divide by zero error occurs if qp_count is less than mw_count because
num_qps_mw is calculated to be zero. The calculation appears to be
incorrect.

The requirement is for num_qps_mw to be set to qp_count / mw_count
with any remainder divided among the earlier mws.

For example, if mw_count is 5 and qp_count is 12 then mws 0 and 1
will have 3 qps per window and mws 2 through 4 will have 2 qps per window.
Thus, when mw_num < qp_count % mw_count, num_qps_mw is 1 higher
than when mw_num >= qp_count.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Jon Mason <jdmason@kudzu.us>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Allen Hubbe <Allen.Hubbe@emc.com>
---
 drivers/ntb/ntb_transport.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 0a778d2cab94..5b6b00ea6ed9 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -623,7 +623,7 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
 	if (!mw->virt_addr)
 		return -ENOMEM;
 
-	if (qp_count % mw_count && mw_num + 1 < qp_count / mw_count)
+	if (mw_num < qp_count % mw_count)
 		num_qps_mw = qp_count / mw_count + 1;
 	else
 		num_qps_mw = qp_count / mw_count;
@@ -1000,7 +1000,7 @@ static int ntb_transport_init_queue(struct ntb_transport_ctx *nt,
 	qp->event_handler = NULL;
 	ntb_qp_link_down_reset(qp);
 
-	if (qp_count % mw_count && mw_num + 1 < qp_count / mw_count)
+	if (mw_num < qp_count % mw_count)
 		num_qps_mw = qp_count / mw_count + 1;
 	else
 		num_qps_mw = qp_count / mw_count;
-- 
2.11.0

RE: [PATCH] ntb_transport: fix bug calculating num_qps_mw

[Allen Hubbe]

From: Logan Gunthorpe
> A divide by zero error occurs if qp_count is less than mw_count because
> num_qps_mw is calculated to be zero. The calculation appears to be
> incorrect.
> 
> The requirement is for num_qps_mw to be set to qp_count / mw_count
> with any remainder divided among the earlier mws.
> 
> For example, if mw_count is 5 and qp_count is 12 then mws 0 and 1
> will have 3 qps per window and mws 2 through 4 will have 2 qps per window.
> Thus, when mw_num < qp_count % mw_count, num_qps_mw is 1 higher
> than when mw_num >= qp_count.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Jon Mason <jdmason@kudzu.us>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Allen Hubbe <Allen.Hubbe@emc.com>

Acked-by: Allen Hubbe <Allen.Hubbe@dell.com>

> ---
>  drivers/ntb/ntb_transport.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 0a778d2cab94..5b6b00ea6ed9 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -623,7 +623,7 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
>  	if (!mw->virt_addr)
>  		return -ENOMEM;
> 
> -	if (qp_count % mw_count && mw_num + 1 < qp_count / mw_count)
> +	if (mw_num < qp_count % mw_count)
>  		num_qps_mw = qp_count / mw_count + 1;
>  	else
>  		num_qps_mw = qp_count / mw_count;
> @@ -1000,7 +1000,7 @@ static int ntb_transport_init_queue(struct ntb_transport_ctx *nt,
>  	qp->event_handler = NULL;
>  	ntb_qp_link_down_reset(qp);
> 
> -	if (qp_count % mw_count && mw_num + 1 < qp_count / mw_count)
> +	if (mw_num < qp_count % mw_count)
>  		num_qps_mw = qp_count / mw_count + 1;
>  	else
>  		num_qps_mw = qp_count / mw_count;
> --
> 2.11.0

Re: [PATCH] ntb_transport: fix bug calculating num_qps_mw

[Jon Mason]

On Tue, Jun 06, 2017 at 09:08:43AM -0400, Allen Hubbe wrote:
> From: Logan Gunthorpe
> > A divide by zero error occurs if qp_count is less than mw_count because
> > num_qps_mw is calculated to be zero. The calculation appears to be
> > incorrect.
> > 
> > The requirement is for num_qps_mw to be set to qp_count / mw_count
> > with any remainder divided among the earlier mws.
> > 
> > For example, if mw_count is 5 and qp_count is 12 then mws 0 and 1
> > will have 3 qps per window and mws 2 through 4 will have 2 qps per window.
> > Thus, when mw_num < qp_count % mw_count, num_qps_mw is 1 higher
> > than when mw_num >= qp_count.
> > 
> > Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> > Cc: Jon Mason <jdmason@kudzu.us>
> > Cc: Dave Jiang <dave.jiang@intel.com>
> > Cc: Allen Hubbe <Allen.Hubbe@emc.com>
> 
> Acked-by: Allen Hubbe <Allen.Hubbe@dell.com>

Adding to my ntb branch with the following line:
Fixes: e26a5843f7f5 ("NTB: Split ntb_hw_intel and ntb_transport drivers")

This enables the stable kernels it is a bug fix and to pick it up.

Thanks,
Jon
> 
> > ---
> >  drivers/ntb/ntb_transport.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> > index 0a778d2cab94..5b6b00ea6ed9 100644
> > --- a/drivers/ntb/ntb_transport.c
> > +++ b/drivers/ntb/ntb_transport.c
> > @@ -623,7 +623,7 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
> >  	if (!mw->virt_addr)
> >  		return -ENOMEM;
> > 
> > -	if (qp_count % mw_count && mw_num + 1 < qp_count / mw_count)
> > +	if (mw_num < qp_count % mw_count)
> >  		num_qps_mw = qp_count / mw_count + 1;
> >  	else
> >  		num_qps_mw = qp_count / mw_count;
> > @@ -1000,7 +1000,7 @@ static int ntb_transport_init_queue(struct ntb_transport_ctx *nt,
> >  	qp->event_handler = NULL;
> >  	ntb_qp_link_down_reset(qp);
> > 
> > -	if (qp_count % mw_count && mw_num + 1 < qp_count / mw_count)
> > +	if (mw_num < qp_count % mw_count)
> >  		num_qps_mw = qp_count / mw_count + 1;
> >  	else
> >  		num_qps_mw = qp_count / mw_count;
> > --
> > 2.11.0
> 
> 

RE: [PATCH] ntb_netdev: set the net_device's parent

[Allen Hubbe]

From: Logan Gunthorpe
> At present, ntb_netdev devices end up under /sys/devices/virtual/net
> completely unconnected to the ntb trees below them. This patch sets the
> parent of the net_device (using SET_NETDEV_DEV) to the client_dev
> device. This results in a better connected sysfs path for the network
> device:
> 
> /sys/devices/pci0000:00/0000:00:03.0/0000:03:00.1/0000:03:00.1/ntb_netdev0/net/eth2
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Jon Mason <jdmason@kudzu.us>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Allen Hubbe <Allen.Hubbe@emc.com>

Acked-by: Allen Hubbe <Allen.Hubbe@dell.com>

> ---
>  drivers/net/ntb_netdev.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
> index 4daf3d0926a8..0250aa9ae2cb 100644
> --- a/drivers/net/ntb_netdev.c
> +++ b/drivers/net/ntb_netdev.c
> @@ -418,6 +418,8 @@ static int ntb_netdev_probe(struct device *client_dev)
>  	if (!ndev)
>  		return -ENOMEM;
> 
> +	SET_NETDEV_DEV(ndev, client_dev);
> +
>  	dev = netdev_priv(ndev);
>  	dev->ndev = ndev;
>  	dev->pdev = pdev;
> --
> 2.11.0

Re: [PATCH] ntb_netdev: set the net_device's parent

[Jon Mason]

On Tue, Jun 06, 2017 at 09:08:40AM -0400, Allen Hubbe wrote:
> From: Logan Gunthorpe
> > At present, ntb_netdev devices end up under /sys/devices/virtual/net
> > completely unconnected to the ntb trees below them. This patch sets the
> > parent of the net_device (using SET_NETDEV_DEV) to the client_dev
> > device. This results in a better connected sysfs path for the network
> > device:
> > 
> > /sys/devices/pci0000:00/0000:00:03.0/0000:03:00.1/0000:03:00.1/ntb_netdev0/net/eth2
> > 
> > Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> > Cc: Jon Mason <jdmason@kudzu.us>
> > Cc: Dave Jiang <dave.jiang@intel.com>
> > Cc: Allen Hubbe <Allen.Hubbe@emc.com>
> 
> Acked-by: Allen Hubbe <Allen.Hubbe@dell.com>

Adding to my ntb branch with the following line:
Fixes: a9c59ef77458 ("ntb_test: Add a selftest script for the NTB subsystem")

This enables the stable kernels it is a bug fix and to pick it up.

Thanks,
Jon
> 
> > ---
> >  drivers/net/ntb_netdev.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
> > index 4daf3d0926a8..0250aa9ae2cb 100644
> > --- a/drivers/net/ntb_netdev.c
> > +++ b/drivers/net/ntb_netdev.c
> > @@ -418,6 +418,8 @@ static int ntb_netdev_probe(struct device *client_dev)
> >  	if (!ndev)
> >  		return -ENOMEM;
> > 
> > +	SET_NETDEV_DEV(ndev, client_dev);
> > +
> >  	dev = netdev_priv(ndev);
> >  	dev->ndev = ndev;
> >  	dev->pdev = pdev;
> > --
> > 2.11.0
> 
> 

Re: [PATCH] ntb_netdev: set the net_device's parent

[Logan Gunthorpe]

On 09/06/17 03:12 PM, Jon Mason wrote:
> Adding to my ntb branch with the following line:
> Fixes: a9c59ef77458 ("ntb_test: Add a selftest script for the NTB subsystem")

Thanks Jon! But you might have gotten confused here... This particular
patch doesn't fix ntb_test, but I sent a different one that does which
you have not merged yet.

Logan

Re: [PATCH] ntb_netdev: set the net_device's parent

[Jon Mason]

On Fri, Jun 9, 2017 at 5:51 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
>
>
> On 09/06/17 03:12 PM, Jon Mason wrote:
>> Adding to my ntb branch with the following line:
>> Fixes: a9c59ef77458 ("ntb_test: Add a selftest script for the NTB subsystem")
>
> Thanks Jon! But you might have gotten confused here... This particular
> patch doesn't fix ntb_test, but I sent a different one that does which
> you have not merged yet.

Oops, I replied to the wrong one.  Sorry.  Everything of yours should
be moved in the ntb or ntb-next branches.  Please double check me and
resend if it is not present.

>
> Logan
>
> --
> You received this message because you are subscribed to the Google Groups "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-ntb+unsubscribe@googlegroups.com.
> To post to this group, send email to linux-ntb@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/9fa5c040-3e67-4f0b-accb-5905aa1de6f0%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] ntb_netdev: set the net_device's parent

[Logan Gunthorpe]

On 09/06/17 03:56 PM, Jon Mason wrote:
> Oops, I replied to the wrong one.  Sorry.  Everything of yours should
> be moved in the ntb or ntb-next branches.  Please double check me and
> resend if it is not present.

Yup, looks good. Thanks.


Logan