[PATCH 0/21] NTB and ntb_netdev patches

[PATCH 0/21] NTB and ntb_netdev patches

[Jon Mason]

Please apply these patches to Greg KH's char-misc-next tree.

Thanks,
Jon

[PATCH 01/21] NTB: correct missing readq/writeq errors

[Jon Mason]

Atomic readq and writeq do not exist by default on some 32bit
architectures, thus causing compile errors due to non-existent symbols.
In those cases, use the definitions of those symbols from
include/asm-generic/io-64-nonatomic-hi-lo.h

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_hw.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 4c71b17..0b46fef 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -45,6 +45,7 @@
  * Contact Information:
  * Jon Mason <jon.mason@intel.com>
  */
+#include <asm-generic/io-64-nonatomic-hi-lo.h>
 #include <linux/debugfs.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
-- 
1.7.9.5

Re: [PATCH 01/21] NTB: correct missing readq/writeq errors

[Greg KH]

On Sat, Jan 19, 2013 at 02:02:15AM -0700, Jon Mason wrote:
> Atomic readq and writeq do not exist by default on some 32bit
> architectures, thus causing compile errors due to non-existent symbols.
> In those cases, use the definitions of those symbols from
> include/asm-generic/io-64-nonatomic-hi-lo.h
> 
> Signed-off-by: Jon Mason <jon.mason@intel.com>
> ---
>  drivers/ntb/ntb_hw.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index 4c71b17..0b46fef 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -45,6 +45,7 @@
>   * Contact Information:
>   * Jon Mason <jon.mason@intel.com>
>   */
> +#include <asm-generic/io-64-nonatomic-hi-lo.h>

Really?  This seems really odd.  Usually we just don't build the code
for any platform that doesn't have readq/writeq.  Otherwise, shouldn't
those arches just include this file themselves to keep everything
working?  Individual drivers shouldn't have to do this.

So, I'm not going to take this one just yet, sorry.

greg k-h

Re: [PATCH 01/21] NTB: correct missing readq/writeq errors

[Jon Mason]

On Sun, Jan 20, 2013 at 03:40:05PM -0800, Greg KH wrote:
> On Sat, Jan 19, 2013 at 02:02:15AM -0700, Jon Mason wrote:
> > Atomic readq and writeq do not exist by default on some 32bit
> > architectures, thus causing compile errors due to non-existent symbols.
> > In those cases, use the definitions of those symbols from
> > include/asm-generic/io-64-nonatomic-hi-lo.h
> > 
> > Signed-off-by: Jon Mason <jon.mason@intel.com>
> > ---
> >  drivers/ntb/ntb_hw.c |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > index 4c71b17..0b46fef 100644
> > --- a/drivers/ntb/ntb_hw.c
> > +++ b/drivers/ntb/ntb_hw.c
> > @@ -45,6 +45,7 @@
> >   * Contact Information:
> >   * Jon Mason <jon.mason@intel.com>
> >   */
> > +#include <asm-generic/io-64-nonatomic-hi-lo.h>
> 
> Really?  This seems really odd.  Usually we just don't build the code
> for any platform that doesn't have readq/writeq.  Otherwise, shouldn't
> those arches just include this file themselves to keep everything
> working?  Individual drivers shouldn't have to do this.

There is no readq/writeq for 32bit x86.  I was using this header file
to get around that.  I freely admit that I have done 0 testing on
32bit, but it should work.  If you prefer I modify the Kconfig to only
enable this for x86_64 until such time as I test on x86_32, I will
happily submit that patch.

Thanks,
Jon

> 
> So, I'm not going to take this one just yet, sorry.
> 
> greg k-h

Re: [PATCH 01/21] NTB: correct missing readq/writeq errors

[Greg KH]

On Mon, Jan 21, 2013 at 10:38:18AM -0700, Jon Mason wrote:
> On Sun, Jan 20, 2013 at 03:40:05PM -0800, Greg KH wrote:
> > On Sat, Jan 19, 2013 at 02:02:15AM -0700, Jon Mason wrote:
> > > Atomic readq and writeq do not exist by default on some 32bit
> > > architectures, thus causing compile errors due to non-existent symbols.
> > > In those cases, use the definitions of those symbols from
> > > include/asm-generic/io-64-nonatomic-hi-lo.h
> > > 
> > > Signed-off-by: Jon Mason <jon.mason@intel.com>
> > > ---
> > >  drivers/ntb/ntb_hw.c |    1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > > index 4c71b17..0b46fef 100644
> > > --- a/drivers/ntb/ntb_hw.c
> > > +++ b/drivers/ntb/ntb_hw.c
> > > @@ -45,6 +45,7 @@
> > >   * Contact Information:
> > >   * Jon Mason <jon.mason@intel.com>
> > >   */
> > > +#include <asm-generic/io-64-nonatomic-hi-lo.h>
> > 
> > Really?  This seems really odd.  Usually we just don't build the code
> > for any platform that doesn't have readq/writeq.  Otherwise, shouldn't
> > those arches just include this file themselves to keep everything
> > working?  Individual drivers shouldn't have to do this.
> 
> There is no readq/writeq for 32bit x86.  I was using this header file
> to get around that.  I freely admit that I have done 0 testing on
> 32bit, but it should work.  If you prefer I modify the Kconfig to only
> enable this for x86_64 until such time as I test on x86_32, I will
> happily submit that patch.

No, other drivers on 32bit x86 also use readq, and do not include this
asm header file.  So there is a proper way to do this, just dig through
the header file include mess to determine the proper one to include.
You should never have to include asm-generic file directly, that's the
issue here.

thanks,

greg k-h

Re: [PATCH 01/21] NTB: correct missing readq/writeq errors

[Ben Hutchings]

On Mon, 2013-01-21 at 10:23 -0800, Greg KH wrote:
> On Mon, Jan 21, 2013 at 10:38:18AM -0700, Jon Mason wrote:
> > On Sun, Jan 20, 2013 at 03:40:05PM -0800, Greg KH wrote:
> > > On Sat, Jan 19, 2013 at 02:02:15AM -0700, Jon Mason wrote:
> > > > Atomic readq and writeq do not exist by default on some 32bit
> > > > architectures, thus causing compile errors due to non-existent symbols.
> > > > In those cases, use the definitions of those symbols from
> > > > include/asm-generic/io-64-nonatomic-hi-lo.h
> > > > 
> > > > Signed-off-by: Jon Mason <jon.mason@intel.com>
> > > > ---
> > > >  drivers/ntb/ntb_hw.c |    1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > > > index 4c71b17..0b46fef 100644
> > > > --- a/drivers/ntb/ntb_hw.c
> > > > +++ b/drivers/ntb/ntb_hw.c
> > > > @@ -45,6 +45,7 @@
> > > >   * Contact Information:
> > > >   * Jon Mason <jon.mason@intel.com>
> > > >   */
> > > > +#include <asm-generic/io-64-nonatomic-hi-lo.h>
> > > 
> > > Really?  This seems really odd.  Usually we just don't build the code
> > > for any platform that doesn't have readq/writeq.  Otherwise, shouldn't
> > > those arches just include this file themselves to keep everything
> > > working?  Individual drivers shouldn't have to do this.
> > 
> > There is no readq/writeq for 32bit x86.  I was using this header file
> > to get around that.  I freely admit that I have done 0 testing on
> > 32bit, but it should work.  If you prefer I modify the Kconfig to only
> > enable this for x86_64 until such time as I test on x86_32, I will
> > happily submit that patch.
> 
> No, other drivers on 32bit x86 also use readq,

I don't think so, because it's not defined for CONFIG_X86_32.

> and do not include this
> asm header file.  So there is a proper way to do this, just dig through
> the header file include mess to determine the proper one to include.
> You should never have to include asm-generic file directly, that's the
> issue here.

Different devices can have different requirements for the order of
access to 32-bit halves of a 64-bit register.  Only the driver knows
which is right.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH 01/21] NTB: correct missing readq/writeq errors

[Greg KH]

On Mon, Jan 21, 2013 at 08:34:53PM +0000, Ben Hutchings wrote:
> On Mon, 2013-01-21 at 10:23 -0800, Greg KH wrote:
> > On Mon, Jan 21, 2013 at 10:38:18AM -0700, Jon Mason wrote:
> > > On Sun, Jan 20, 2013 at 03:40:05PM -0800, Greg KH wrote:
> > > > On Sat, Jan 19, 2013 at 02:02:15AM -0700, Jon Mason wrote:
> > > > > Atomic readq and writeq do not exist by default on some 32bit
> > > > > architectures, thus causing compile errors due to non-existent symbols.
> > > > > In those cases, use the definitions of those symbols from
> > > > > include/asm-generic/io-64-nonatomic-hi-lo.h
> > > > > 
> > > > > Signed-off-by: Jon Mason <jon.mason@intel.com>
> > > > > ---
> > > > >  drivers/ntb/ntb_hw.c |    1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > > > > index 4c71b17..0b46fef 100644
> > > > > --- a/drivers/ntb/ntb_hw.c
> > > > > +++ b/drivers/ntb/ntb_hw.c
> > > > > @@ -45,6 +45,7 @@
> > > > >   * Contact Information:
> > > > >   * Jon Mason <jon.mason@intel.com>
> > > > >   */
> > > > > +#include <asm-generic/io-64-nonatomic-hi-lo.h>
> > > > 
> > > > Really?  This seems really odd.  Usually we just don't build the code
> > > > for any platform that doesn't have readq/writeq.  Otherwise, shouldn't
> > > > those arches just include this file themselves to keep everything
> > > > working?  Individual drivers shouldn't have to do this.
> > > 
> > > There is no readq/writeq for 32bit x86.  I was using this header file
> > > to get around that.  I freely admit that I have done 0 testing on
> > > 32bit, but it should work.  If you prefer I modify the Kconfig to only
> > > enable this for x86_64 until such time as I test on x86_32, I will
> > > happily submit that patch.
> > 
> > No, other drivers on 32bit x86 also use readq,
> 
> I don't think so, because it's not defined for CONFIG_X86_32.

Ah, you are right, I was confused by drivers/char/hpet.c, which used
readq, but only on 64bit processors.

Jon, as you say you never tested this on 32bit processors, I'd recommend
just making the Kconfig file enforce that and only build on x86-64 for
now.

thanks,

greg k-h

[PATCH 02/21] NTB: Handle ntb client device probes without present hardware

[Jon Mason]

Attempts to probe client ntb drivers without ntb hardware present will
result in null pointer dereference due to the lack of the ntb bus device
being registers.  Check to see if this is the case, and fail all calls
by the clients registering their drivers.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 250190f..1d17857 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -288,6 +288,9 @@ int ntb_register_client_dev(char *device_name)
 	struct ntb_transport *nt;
 	int rc;
 
+	if (list_empty(&ntb_transport_list))
+		return -ENODEV;
+
 	list_for_each_entry(nt, &ntb_transport_list, entry) {
 		struct device *dev;
 
@@ -336,6 +339,9 @@ int ntb_register_client(struct ntb_client *drv)
 {
 	drv->driver.bus = &ntb_bus_type;
 
+	if (list_empty(&ntb_transport_list))
+		return -ENODEV;
+
 	return driver_register(&drv->driver);
 }
 EXPORT_SYMBOL_GPL(ntb_register_client);
-- 
1.7.9.5

[PATCH 03/21] NTB: correct memory barrier

[Jon Mason]

mmiowb is not sufficient to flush the data and is causing data
corruption.  Change to wmb and the data corruption is no more.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 1d17857..e9666bd 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -1009,7 +1009,7 @@ static void ntb_tx_copy_task(struct ntb_transport_qp *qp,
 	hdr->ver = qp->tx_pkts;
 
 	/* Ensure that the data is fully copied out before setting the flag */
-	mmiowb();
+	wmb();
 	hdr->flags = entry->flags | DESC_DONE_FLAG;
 
 	ntb_ring_sdb(qp->ndev, qp->qp_num);
-- 
1.7.9.5

[PATCH 04/21] NTB: separate transmit and receive windows

[Jon Mason]

Since it is possible for the memory windows on the two NTB connected
systems to be different sizes, the divergent sizes must be accounted for
in the segmentation of the MW's on each side.  Create separate size
variables and initialization as necessary.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |   79 ++++++++++++++++++++++++-------------------
 1 file changed, 44 insertions(+), 35 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index e9666bd..2823087 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -60,7 +60,7 @@
 
 #define NTB_TRANSPORT_VERSION	1
 
-static int transport_mtu = 0x401E;
+static unsigned int transport_mtu = 0x401E;
 module_param(transport_mtu, uint, 0644);
 MODULE_PARM_DESC(transport_mtu, "Maximum size of NTB transport packets");
 
@@ -94,6 +94,7 @@ struct ntb_transport_qp {
 	void *tx_mw_begin;
 	void *tx_mw_end;
 	void *tx_offset;
+	unsigned int tx_max_frame;
 
 	void (*rx_handler) (struct ntb_transport_qp *qp, void *qp_data,
 			    void *data, int len);
@@ -105,6 +106,7 @@ struct ntb_transport_qp {
 	void *rx_buff_begin;
 	void *rx_buff_end;
 	void *rx_offset;
+	unsigned int rx_max_frame;
 
 	void (*event_handler) (void *data, int status);
 	struct delayed_work link_work;
@@ -458,28 +460,29 @@ static void ntb_transport_setup_qp_mw(struct ntb_transport *nt,
 				      unsigned int qp_num)
 {
 	struct ntb_transport_qp *qp = &nt->qps[qp_num];
-	unsigned int size, num_qps_mw;
+	unsigned int rx_size, num_qps_mw;
 	u8 mw_num = QP_TO_MW(qp_num);
+	void *offset;
 
 	WARN_ON(nt->mw[mw_num].virt_addr == 0);
 
-	if (nt->max_qps % NTB_NUM_MW && !mw_num)
-		num_qps_mw = nt->max_qps / NTB_NUM_MW +
-			     (nt->max_qps % NTB_NUM_MW - mw_num);
+	if (nt->max_qps % NTB_NUM_MW && mw_num < nt->max_qps % NTB_NUM_MW)
+		num_qps_mw = nt->max_qps / NTB_NUM_MW + 1;
 	else
 		num_qps_mw = nt->max_qps / NTB_NUM_MW;
 
-	size = nt->mw[mw_num].size / num_qps_mw;
-
+	rx_size = nt->mw[mw_num].size / num_qps_mw;
 	qp->rx_buff_begin = nt->mw[mw_num].virt_addr +
-			    (qp_num / NTB_NUM_MW * size);
-	qp->rx_buff_end = qp->rx_buff_begin + size;
+			    (qp_num / NTB_NUM_MW * rx_size);
+	qp->rx_buff_end = qp->rx_buff_begin + rx_size;
 	qp->rx_offset = qp->rx_buff_begin;
+	qp->rx_max_frame = min(transport_mtu, rx_size);
 
-	qp->tx_mw_begin = ntb_get_mw_vbase(nt->ndev, mw_num) +
-			  (qp_num / NTB_NUM_MW * size);
-	qp->tx_mw_end = qp->tx_mw_begin + size;
-	qp->tx_offset = qp->tx_mw_begin;
+	/* setup the hdr offsets with 0's */
+	for (offset = qp->rx_buff_begin + qp->rx_max_frame -
+		      sizeof(struct ntb_payload_header);
+	     offset < qp->rx_buff_end; offset += qp->rx_max_frame)
+		memset(offset, 0, sizeof(struct ntb_payload_header));
 
 	qp->rx_pkts = 0;
 	qp->tx_pkts = 0;
@@ -489,7 +492,6 @@ static int ntb_set_mw(struct ntb_transport *nt, int num_mw, unsigned int size)
 {
 	struct ntb_transport_mw *mw = &nt->mw[num_mw];
 	struct pci_dev *pdev = ntb_query_pdev(nt->ndev);
-	void *offset;
 
 	/* Alloc memory for receiving data.  Must be 4k aligned */
 	mw->size = ALIGN(size, 4096);
@@ -502,12 +504,6 @@ static int ntb_set_mw(struct ntb_transport *nt, int num_mw, unsigned int size)
 		return -ENOMEM;
 	}
 
-	/* setup the hdr offsets with 0's */
-	for (offset = mw->virt_addr + transport_mtu -
-		      sizeof(struct ntb_payload_header);
-	     offset < mw->virt_addr + size; offset += transport_mtu)
-		memset(offset, 0, sizeof(struct ntb_payload_header));
-
 	/* Notify HW the memory location of the receive buffer */
 	ntb_set_mw_addr(nt->ndev, num_mw, mw->dma_addr);
 
@@ -737,6 +733,8 @@ static void ntb_transport_init_queue(struct ntb_transport *nt,
 				     unsigned int qp_num)
 {
 	struct ntb_transport_qp *qp;
+	unsigned int num_qps_mw, tx_size;
+	u8 mw_num = QP_TO_MW(qp_num);
 
 	qp = &nt->qps[qp_num];
 	qp->qp_num = qp_num;
@@ -746,6 +744,18 @@ static void ntb_transport_init_queue(struct ntb_transport *nt,
 	qp->client_ready = NTB_LINK_DOWN;
 	qp->event_handler = NULL;
 
+	if (nt->max_qps % NTB_NUM_MW && mw_num < nt->max_qps % NTB_NUM_MW)
+		num_qps_mw = nt->max_qps / NTB_NUM_MW + 1;
+	else
+		num_qps_mw = nt->max_qps / NTB_NUM_MW;
+
+	tx_size = ntb_get_mw_size(qp->ndev, mw_num) / num_qps_mw;
+	qp->tx_mw_begin = ntb_get_mw_vbase(nt->ndev, mw_num) +
+			  (qp_num / NTB_NUM_MW * tx_size);
+	qp->tx_mw_end = qp->tx_mw_begin + tx_size;
+	qp->tx_offset = qp->tx_mw_begin;
+	qp->tx_max_frame = min(transport_mtu, tx_size);
+
 	if (nt->debugfs_dir) {
 		char debugfs_name[4];
 
@@ -873,9 +883,9 @@ static void ntb_rx_copy_task(struct ntb_transport_qp *qp,
 	struct ntb_payload_header *hdr;
 
 	BUG_ON(offset < qp->rx_buff_begin ||
-	       offset + transport_mtu >= qp->rx_buff_end);
+	       offset + qp->rx_max_frame >= qp->rx_buff_end);
 
-	hdr = offset + transport_mtu - sizeof(struct ntb_payload_header);
+	hdr = offset + qp->rx_max_frame - sizeof(struct ntb_payload_header);
 	entry->len = hdr->len;
 
 	memcpy(entry->buf, offset, entry->len);
@@ -898,7 +908,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 
 	entry = ntb_list_rm(&qp->ntb_rx_pend_q_lock, &qp->rx_pend_q);
 	if (!entry) {
-		hdr = offset + transport_mtu -
+		hdr = offset + qp->rx_max_frame -
 		      sizeof(struct ntb_payload_header);
 		dev_dbg(&ntb_query_pdev(qp->ndev)->dev,
 			"no buffer - HDR ver %llu, len %d, flags %x\n",
@@ -908,7 +918,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 	}
 
 	offset = qp->rx_offset;
-	hdr = offset + transport_mtu - sizeof(struct ntb_payload_header);
+	hdr = offset + qp->rx_max_frame - sizeof(struct ntb_payload_header);
 
 	if (!(hdr->flags & DESC_DONE_FLAG)) {
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
@@ -966,8 +976,8 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 	qp->rx_pkts++;
 
 out:
-	qp->rx_offset += transport_mtu;
-	if (qp->rx_offset + transport_mtu >= qp->rx_buff_end)
+	qp->rx_offset += qp->rx_max_frame;
+	if (qp->rx_offset + qp->rx_max_frame >= qp->rx_buff_end)
 		qp->rx_offset = qp->rx_buff_begin;
 
 	return 0;
@@ -1000,11 +1010,11 @@ static void ntb_tx_copy_task(struct ntb_transport_qp *qp,
 	struct ntb_payload_header *hdr;
 
 	BUG_ON(offset < qp->tx_mw_begin ||
-	       offset + transport_mtu >= qp->tx_mw_end);
+	       offset + qp->tx_max_frame >= qp->tx_mw_end);
 
 	memcpy_toio(offset, entry->buf, entry->len);
 
-	hdr = offset + transport_mtu - sizeof(struct ntb_payload_header);
+	hdr = offset + qp->tx_max_frame - sizeof(struct ntb_payload_header);
 	hdr->len = entry->len;
 	hdr->ver = qp->tx_pkts;
 
@@ -1036,7 +1046,7 @@ static int ntb_process_tx(struct ntb_transport_qp *qp,
 	void *offset;
 
 	offset = qp->tx_offset;
-	hdr = offset + transport_mtu - sizeof(struct ntb_payload_header);
+	hdr = offset + qp->tx_max_frame - sizeof(struct ntb_payload_header);
 
 	dev_dbg(&ntb_query_pdev(qp->ndev)->dev, "%lld - offset %p, tx %p, entry len %d flags %x buff %p\n",
 		 qp->tx_pkts, offset, qp->tx_offset, entry->len, entry->flags,
@@ -1046,7 +1056,7 @@ static int ntb_process_tx(struct ntb_transport_qp *qp,
 		return -EAGAIN;
 	}
 
-	if (entry->len > transport_mtu - sizeof(struct ntb_payload_header)) {
+	if (entry->len > qp->tx_max_frame - sizeof(struct ntb_payload_header)) {
 		if (qp->tx_handler)
 			qp->tx_handler(qp->cb_data, qp, NULL, -EIO);
 
@@ -1057,8 +1067,8 @@ static int ntb_process_tx(struct ntb_transport_qp *qp,
 
 	ntb_tx_copy_task(qp, entry, offset);
 
-	qp->tx_offset += transport_mtu;
-	if (qp->tx_offset + transport_mtu >= qp->tx_mw_end)
+	qp->tx_offset += qp->tx_max_frame;
+	if (qp->tx_offset + qp->tx_max_frame >= qp->tx_mw_end)
 		qp->tx_offset = qp->tx_mw_begin;
 
 	qp->tx_pkts++;
@@ -1425,9 +1435,8 @@ EXPORT_SYMBOL_GPL(ntb_transport_qp_num);
  *
  * RETURNS: the max payload size of a qp
  */
-unsigned int
-ntb_transport_max_size(__attribute__((unused)) struct ntb_transport_qp *qp)
+unsigned int ntb_transport_max_size(struct ntb_transport_qp *qp)
 {
-	return transport_mtu - sizeof(struct ntb_payload_header);
+	return qp->tx_max_frame - sizeof(struct ntb_payload_header);
 }
 EXPORT_SYMBOL_GPL(ntb_transport_max_size);
-- 
1.7.9.5

[PATCH 05/21] NTB: No sleeping in interrupt context

[Jon Mason]

Move all cancel_delayed_work_sync to a work thread to prevent sleeping
in interrupt context (when the NTB link goes down).  Caught via
'Sleep inside atomic section checking'

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 2823087..bf7ade1 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -110,6 +110,7 @@ struct ntb_transport_qp {
 
 	void (*event_handler) (void *data, int status);
 	struct delayed_work link_work;
+	struct work_struct link_cleanup;
 
 	struct dentry *debugfs_dir;
 	struct dentry *debugfs_stats;
@@ -148,6 +149,7 @@ struct ntb_transport {
 	unsigned long qp_bitmap;
 	bool transport_link;
 	struct delayed_work link_work;
+	struct work_struct link_cleanup;
 	struct dentry *debugfs_dir;
 };
 
@@ -510,8 +512,11 @@ static int ntb_set_mw(struct ntb_transport *nt, int num_mw, unsigned int size)
 	return 0;
 }
 
-static void ntb_qp_link_down(struct ntb_transport_qp *qp)
+static void ntb_qp_link_cleanup(struct work_struct *work)
 {
+	struct ntb_transport_qp *qp = container_of(work,
+						   struct ntb_transport_qp,
+						   link_cleanup);
 	struct ntb_transport *nt = qp->transport;
 	struct pci_dev *pdev = ntb_query_pdev(nt->ndev);
 
@@ -531,8 +536,15 @@ static void ntb_qp_link_down(struct ntb_transport_qp *qp)
 				      msecs_to_jiffies(NTB_LINK_DOWN_TIMEOUT));
 }
 
-static void ntb_transport_conn_down(struct ntb_transport *nt)
+static void ntb_qp_link_down(struct ntb_transport_qp *qp)
+{
+	schedule_work(&qp->link_cleanup);
+}
+
+static void ntb_transport_link_cleanup(struct work_struct *work)
 {
+	struct ntb_transport *nt = container_of(work, struct ntb_transport,
+						link_cleanup);
 	int i;
 
 	if (nt->transport_link == NTB_LINK_DOWN)
@@ -562,7 +574,7 @@ static void ntb_transport_event_callback(void *data, enum ntb_hw_event event)
 		schedule_delayed_work(&nt->link_work, 0);
 		break;
 	case NTB_EVENT_HW_LINK_DOWN:
-		ntb_transport_conn_down(nt);
+		schedule_work(&nt->link_cleanup);
 		break;
 	default:
 		BUG();
@@ -769,6 +781,7 @@ static void ntb_transport_init_queue(struct ntb_transport *nt,
 	}
 
 	INIT_DELAYED_WORK(&qp->link_work, ntb_qp_link_work);
+	INIT_WORK(&qp->link_cleanup, ntb_qp_link_cleanup);
 
 	spin_lock_init(&qp->ntb_rx_pend_q_lock);
 	spin_lock_init(&qp->ntb_rx_free_q_lock);
@@ -814,6 +827,7 @@ int ntb_transport_init(struct pci_dev *pdev)
 		ntb_transport_init_queue(nt, i);
 
 	INIT_DELAYED_WORK(&nt->link_work, ntb_transport_link_work);
+	INIT_WORK(&nt->link_cleanup, ntb_transport_link_cleanup);
 
 	rc = ntb_register_event_callback(nt->ndev,
 					 ntb_transport_event_callback);
-- 
1.7.9.5

[PATCH 06/21] NTB: use simple_open for debugfs

[Jon Mason]

Use simple_open for debugfs instead of recreating it in the NTB driver.
Caught by coccicheck.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |    8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index bf7ade1..c0eca02 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -364,12 +364,6 @@ void ntb_unregister_client(struct ntb_client *drv)
 }
 EXPORT_SYMBOL_GPL(ntb_unregister_client);
 
-static int debugfs_open(struct inode *inode, struct file *filp)
-{
-	filp->private_data = inode->i_private;
-	return 0;
-}
-
 static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,
 			    loff_t *offp)
 {
@@ -425,7 +419,7 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,
 
 static const struct file_operations ntb_qp_debugfs_stats = {
 	.owner = THIS_MODULE,
-	.open = debugfs_open,
+	.open = simple_open,
 	.read = debugfs_read,
 };
 
-- 
1.7.9.5

[PATCH 07/21] NTB: zero PCI driver data

[Jon Mason]

Zero pci_device_id driver_data variable.  Unused, but 'EXTRA_CFLAGS=-W'
complained of uninitialized variables.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_hw.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 0b46fef..867ccaa 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -83,14 +83,14 @@ enum {
 #define MW_TO_BAR(mw)	(mw * 2 + 2)
 
 static DEFINE_PCI_DEVICE_TABLE(ntb_pci_tbl) = {
-	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_BWD)},
-	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_JSF)},
-	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_CLASSIC_JSF)},
-	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_RP_JSF)},
-	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_RP_SNB)},
-	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_SNB)},
-	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_CLASSIC_SNB)},
-	{0}
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_BWD),		0},
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_JSF),		0},
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_CLASSIC_JSF),	0},
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_RP_JSF),		0},
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_RP_SNB),		0},
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_SNB),		0},
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_CLASSIC_SNB),	0},
+	{0, 0, 0, 0, 0, 0, 0}
 };
 MODULE_DEVICE_TABLE(pci, ntb_pci_tbl);
 
-- 
1.7.9.5

Re: [PATCH 07/21] NTB: zero PCI driver data

[Greg KH]

On Sat, Jan 19, 2013 at 02:02:21AM -0700, Jon Mason wrote:
> Zero pci_device_id driver_data variable.  Unused, but 'EXTRA_CFLAGS=-W'
> complained of uninitialized variables.
> 
> Signed-off-by: Jon Mason <jon.mason@intel.com>
> ---
>  drivers/ntb/ntb_hw.c |   16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index 0b46fef..867ccaa 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -83,14 +83,14 @@ enum {
>  #define MW_TO_BAR(mw)	(mw * 2 + 2)
>  
>  static DEFINE_PCI_DEVICE_TABLE(ntb_pci_tbl) = {
> -	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_BWD)},
> -	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_JSF)},
> -	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_CLASSIC_JSF)},
> -	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_RP_JSF)},
> -	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_RP_SNB)},
> -	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_SNB)},
> -	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_CLASSIC_SNB)},
> -	{0}
> +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_BWD),		0},
> +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_JSF),		0},
> +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_CLASSIC_JSF),	0},
> +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_RP_JSF),		0},
> +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_RP_SNB),		0},
> +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_SNB),		0},
> +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_CLASSIC_SNB),	0},
> +	{0, 0, 0, 0, 0, 0, 0}

Ick, why not just use PCI_DEVICE() instead?  Is it that hard to type out
the INTEL define?  That would, overall, look nicer than the 0, 0, 0...
line at the end, which is horrible (and not needed, { } is the same
thing.)

thanks,

greg k-h

[PATCH 08/21] NTB: declare unused variables

[Jon Mason]

Tag pci_device_id in ntb_pci_probe as unused function parameters.  This
corrects issues found by 'EXTRA_CFLAGS=-W'.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_hw.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 867ccaa..2357c2d 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -1007,7 +1007,8 @@ static void ntb_free_callbacks(struct ntb_device *ndev)
 	kfree(ndev->db_cb);
 }
 
-static int ntb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+static int ntb_pci_probe(struct pci_dev *pdev,
+			 __attribute__((unused)) const struct pci_device_id *id)
 {
 	struct ntb_device *ndev;
 	int rc, i;
-- 
1.7.9.5

Re: [PATCH 08/21] NTB: declare unused variables

[Greg KH]

On Sat, Jan 19, 2013 at 02:02:22AM -0700, Jon Mason wrote:
> Tag pci_device_id in ntb_pci_probe as unused function parameters.  This
> corrects issues found by 'EXTRA_CFLAGS=-W'.

Why would you build the kernel with that option?  This isn't needed, so
I'm not going to apply it.

thanks,

greg k-h

Re: [PATCH 08/21] NTB: declare unused variables

[Jon Mason]

On Sun, Jan 20, 2013 at 03:42:28PM -0800, Greg KH wrote:
> On Sat, Jan 19, 2013 at 02:02:22AM -0700, Jon Mason wrote:
> > Tag pci_device_id in ntb_pci_probe as unused function parameters.  This
> > corrects issues found by 'EXTRA_CFLAGS=-W'.
> 
> Why would you build the kernel with that option?  This isn't needed, so

Per Documentation/SubmitChecklist:

22: Newly-added code has been compiled with `gcc -W' (use "make
    EXTRA_CFLAGS=-W").  This will generate lots of noise, but is good for
    finding bugs like "warning: comparison between signed and unsigned".


I was attempting to comply with that suggestion.  If this is
unnecessary, then I can toss these, but I don't really see them
hurting anything.

Same goes for the other 2 patches found via 'EXTRA_CFLAGS=-W'.

Thanks,
Jon

> I'm not going to apply it.
> 
> thanks,
> 
> greg k-h

Re: [PATCH 08/21] NTB: declare unused variables

[Greg KH]

On Mon, Jan 21, 2013 at 10:50:55AM -0700, Jon Mason wrote:
> On Sun, Jan 20, 2013 at 03:42:28PM -0800, Greg KH wrote:
> > On Sat, Jan 19, 2013 at 02:02:22AM -0700, Jon Mason wrote:
> > > Tag pci_device_id in ntb_pci_probe as unused function parameters.  This
> > > corrects issues found by 'EXTRA_CFLAGS=-W'.
> > 
> > Why would you build the kernel with that option?  This isn't needed, so
> 
> Per Documentation/SubmitChecklist:
> 
> 22: Newly-added code has been compiled with `gcc -W' (use "make
>     EXTRA_CFLAGS=-W").  This will generate lots of noise, but is good for
>     finding bugs like "warning: comparison between signed and unsigned".
> 
> 
> I was attempting to comply with that suggestion.  If this is
> unnecessary, then I can toss these, but I don't really see them
> hurting anything.

It's messy and unneeded, so I'd not recommend using it.  As the
checklist says, it generates a lot of noise, and is good for finding
real bugs, this isn't a "real" bug at all.

> Same goes for the other 2 patches found via 'EXTRA_CFLAGS=-W'.

Understood, I've not applied them either :)

thanks,

greg k-h

[PATCH 09/21] NTB: namespacecheck cleanups

[Jon Mason]

Declare ntb_bus_type static to remove it from name space, and remove
unused ntb_get_max_spads function.  Found via `make namespacecheck`.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_hw.c        |   14 --------------
 drivers/ntb/ntb_transport.c |    2 +-
 2 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 2357c2d..18cb5dc 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -238,20 +238,6 @@ void ntb_unregister_transport(struct ntb_device *ndev)
 }
 
 /**
- * ntb_get_max_spads() - get the total scratch regs usable
- * @ndev: pointer to ntb_device instance
- *
- * This function returns the max 32bit scratchpad registers usable by the
- * upper layer.
- *
- * RETURNS: total number of scratch pad registers available
- */
-int ntb_get_max_spads(struct ntb_device *ndev)
-{
-	return ndev->limits.max_spads;
-}
-
-/**
  * ntb_write_local_spad() - write to the secondary scratchpad register
  * @ndev: pointer to ntb_device instance
  * @idx: index to the scratchpad register, 0 based
diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index c0eca02..903a72e 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -212,7 +212,7 @@ static int ntb_client_remove(struct device *dev)
 	return 0;
 }
 
-struct bus_type ntb_bus_type = {
+static struct bus_type ntb_bus_type = {
 	.name = "ntb_bus",
 	.match = ntb_match_bus,
 	.probe = ntb_client_probe,
-- 
1.7.9.5

[PATCH 10/21] NTB: whitespace cleanups

[Jon Mason]

Whitespace cleanups found via `indent`

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |   40 ++++++++++++++++------------------------
 1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 903a72e..e11b57e 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -930,7 +930,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 
 	if (!(hdr->flags & DESC_DONE_FLAG)) {
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
-				  &qp->rx_pend_q);
+			     &qp->rx_pend_q);
 		qp->rx_ring_empty++;
 		return -EAGAIN;
 	}
@@ -940,7 +940,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 			"qp %d: version mismatch, expected %llu - got %llu\n",
 			qp->qp_num, qp->rx_pkts, hdr->ver);
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
-				  &qp->rx_pend_q);
+			     &qp->rx_pend_q);
 		qp->rx_err_ver++;
 		return -EIO;
 	}
@@ -949,7 +949,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 		ntb_qp_link_down(qp);
 
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
-				  &qp->rx_pend_q);
+			     &qp->rx_pend_q);
 
 		/* Ensure that the data is fully copied out before clearing the
 		 * done flag
@@ -967,7 +967,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 		ntb_rx_copy_task(qp, entry, offset);
 	else {
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
-				  &qp->rx_pend_q);
+			     &qp->rx_pend_q);
 
 		/* Ensure that the data is fully copied out before clearing the
 		 * done flag
@@ -1057,8 +1057,8 @@ static int ntb_process_tx(struct ntb_transport_qp *qp,
 	hdr = offset + qp->tx_max_frame - sizeof(struct ntb_payload_header);
 
 	dev_dbg(&ntb_query_pdev(qp->ndev)->dev, "%lld - offset %p, tx %p, entry len %d flags %x buff %p\n",
-		 qp->tx_pkts, offset, qp->tx_offset, entry->len, entry->flags,
-		 entry->buf);
+		qp->tx_pkts, offset, qp->tx_offset, entry->len, entry->flags,
+		entry->buf);
 	if (hdr->flags) {
 		qp->tx_ring_full++;
 		return -EAGAIN;
@@ -1097,8 +1097,7 @@ static void ntb_send_link_down(struct ntb_transport_qp *qp)
 	dev_info(&pdev->dev, "qp %d: Link Down\n", qp->qp_num);
 
 	for (i = 0; i < NTB_LINK_DOWN_TIMEOUT; i++) {
-		entry = ntb_list_rm(&qp->ntb_tx_free_q_lock,
-					 &qp->tx_free_q);
+		entry = ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q);
 		if (entry)
 			break;
 		msleep(100);
@@ -1167,7 +1166,7 @@ ntb_transport_create_queue(void *data, struct pci_dev *pdev,
 			goto err1;
 
 		ntb_list_add(&qp->ntb_rx_free_q_lock, &entry->entry,
-				  &qp->rx_free_q);
+			     &qp->rx_free_q);
 	}
 
 	for (i = 0; i < NTB_QP_DEF_NUM_ENTRIES; i++) {
@@ -1176,7 +1175,7 @@ ntb_transport_create_queue(void *data, struct pci_dev *pdev,
 			goto err2;
 
 		ntb_list_add(&qp->ntb_tx_free_q_lock, &entry->entry,
-				  &qp->tx_free_q);
+			     &qp->tx_free_q);
 	}
 
 	tasklet_init(&qp->rx_work, ntb_transport_rx, (unsigned long) qp);
@@ -1193,12 +1192,10 @@ ntb_transport_create_queue(void *data, struct pci_dev *pdev,
 err3:
 	tasklet_disable(&qp->rx_work);
 err2:
-	while ((entry =
-		ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q)))
+	while ((entry = ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q)))
 		kfree(entry);
 err1:
-	while ((entry =
-		ntb_list_rm(&qp->ntb_rx_free_q_lock, &qp->rx_free_q)))
+	while ((entry = ntb_list_rm(&qp->ntb_rx_free_q_lock, &qp->rx_free_q)))
 		kfree(entry);
 	set_bit(free_queue, &nt->qp_bitmap);
 err:
@@ -1225,18 +1222,15 @@ void ntb_transport_free_queue(struct ntb_transport_qp *qp)
 	ntb_unregister_db_callback(qp->ndev, qp->qp_num);
 	tasklet_disable(&qp->rx_work);
 
-	while ((entry =
-		ntb_list_rm(&qp->ntb_rx_free_q_lock, &qp->rx_free_q)))
+	while ((entry = ntb_list_rm(&qp->ntb_rx_free_q_lock, &qp->rx_free_q)))
 		kfree(entry);
 
-	while ((entry =
-		ntb_list_rm(&qp->ntb_rx_pend_q_lock, &qp->rx_pend_q))) {
+	while ((entry = ntb_list_rm(&qp->ntb_rx_pend_q_lock, &qp->rx_pend_q))) {
 		dev_warn(&pdev->dev, "Freeing item from a non-empty queue\n");
 		kfree(entry);
 	}
 
-	while ((entry =
-		ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q)))
+	while ((entry = ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q)))
 		kfree(entry);
 
 	set_bit(qp->qp_num, &qp->transport->qp_bitmap);
@@ -1270,8 +1264,7 @@ void *ntb_transport_rx_remove(struct ntb_transport_qp *qp, unsigned int *len)
 	buf = entry->cb_data;
 	*len = entry->len;
 
-	ntb_list_add(&qp->ntb_rx_free_q_lock, &entry->entry,
-			  &qp->rx_free_q);
+	ntb_list_add(&qp->ntb_rx_free_q_lock, &entry->entry, &qp->rx_free_q);
 
 	return buf;
 }
@@ -1305,8 +1298,7 @@ int ntb_transport_rx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
 	entry->buf = data;
 	entry->len = len;
 
-	ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
-			  &qp->rx_pend_q);
+	ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry, &qp->rx_pend_q);
 
 	return 0;
 }
-- 
1.7.9.5

[PATCH 11/21] NTB: correct stack usage warning in debugfs_read

[Jon Mason]

Correct gcc warning of using too much stack debugfs_read.  This is done
by kmallocing the buffer instead of using the char array on stack.
Also, shrinking the buffer to something closer to what is currently
being used.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index e11b57e..1bed1ba 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -368,10 +368,14 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,
 			    loff_t *offp)
 {
 	struct ntb_transport_qp *qp;
-	char buf[1024];
+	char *buf;
 	ssize_t ret, out_offset, out_count;
 
-	out_count = 1024;
+	out_count = 600;
+
+	buf = kmalloc(out_count, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
 
 	qp = filp->private_data;
 	out_offset = 0;
@@ -410,10 +414,13 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,
 			       "tx_mw_end - \t%p\n", qp->tx_mw_end);
 
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "QP Link %s\n", (qp->qp_link == NTB_LINK_UP) ?
+			       "\nQP Link %s\n", (qp->qp_link == NTB_LINK_UP) ?
 			       "Up" : "Down");
+	if (out_offset > out_count)
+		out_offset = out_count;
 
 	ret = simple_read_from_buffer(ubuf, count, offp, buf, out_offset);
+	kfree(buf);
 	return ret;
 }
 
-- 
1.7.9.5

[PATCH 12/21] NTB: Remove reads across NTB

[Jon Mason]

CPU reads across NTB are slow(er) and can hang the local system if an
ECC error is encountered on the remote.  To work around the need for a
read, have the remote side write its current position in the rx buffer
to the local system's buffer and use that to see if there is room when
transmitting.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |  137 ++++++++++++++++++++-----------------------
 1 file changed, 63 insertions(+), 74 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 1bed1ba..69c58da 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -78,6 +78,10 @@ struct ntb_queue_entry {
 	unsigned int flags;
 };
 
+struct ntb_rx_info {
+	unsigned int entry;
+};
+
 struct ntb_transport_qp {
 	struct ntb_transport *transport;
 	struct ntb_device *ndev;
@@ -87,13 +91,16 @@ struct ntb_transport_qp {
 	bool qp_link;
 	u8 qp_num;	/* Only 64 QP's are allowed.  0-63 */
 
+	struct ntb_rx_info *rx_info;
+	struct ntb_rx_info *remote_rx_info;
+
 	void (*tx_handler) (struct ntb_transport_qp *qp, void *qp_data,
 			    void *data, int len);
 	struct list_head tx_free_q;
 	spinlock_t ntb_tx_free_q_lock;
-	void *tx_mw_begin;
-	void *tx_mw_end;
-	void *tx_offset;
+	void *tx_mw;
+	unsigned int tx_index;
+	unsigned int tx_max_entry;
 	unsigned int tx_max_frame;
 
 	void (*rx_handler) (struct ntb_transport_qp *qp, void *qp_data,
@@ -103,9 +110,9 @@ struct ntb_transport_qp {
 	struct list_head rx_free_q;
 	spinlock_t ntb_rx_pend_q_lock;
 	spinlock_t ntb_rx_free_q_lock;
-	void *rx_buff_begin;
-	void *rx_buff_end;
-	void *rx_offset;
+	void *rx_buff;
+	unsigned int rx_index;
+	unsigned int rx_max_entry;
 	unsigned int rx_max_frame;
 
 	void (*event_handler) (void *data, int status);
@@ -394,11 +401,11 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
 			       "rx_err_ver - \t%llu\n", qp->rx_err_ver);
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "rx_buff_begin - %p\n", qp->rx_buff_begin);
+			       "rx_buff - \t%p\n", qp->rx_buff);
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "rx_offset - \t%p\n", qp->rx_offset);
+			       "rx_index - \t%u\n", qp->rx_index);
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "rx_buff_end - \t%p\n", qp->rx_buff_end);
+			       "rx_max_entry - \t%u\n", qp->rx_max_entry);
 
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
 			       "tx_bytes - \t%llu\n", qp->tx_bytes);
@@ -407,11 +414,11 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
 			       "tx_ring_full - \t%llu\n", qp->tx_ring_full);
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "tx_mw_begin - \t%p\n", qp->tx_mw_begin);
+			       "tx_mw - \t%p\n", qp->tx_mw);
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "tx_offset - \t%p\n", qp->tx_offset);
+			       "tx_index - \t%u\n", qp->tx_index);
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "tx_mw_end - \t%p\n", qp->tx_mw_end);
+			       "tx_max_entry - \t%u\n", qp->tx_max_entry);
 
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
 			       "\nQP Link %s\n", (qp->qp_link == NTB_LINK_UP) ?
@@ -465,7 +472,7 @@ static void ntb_transport_setup_qp_mw(struct ntb_transport *nt,
 	struct ntb_transport_qp *qp = &nt->qps[qp_num];
 	unsigned int rx_size, num_qps_mw;
 	u8 mw_num = QP_TO_MW(qp_num);
-	void *offset;
+	unsigned int i;
 
 	WARN_ON(nt->mw[mw_num].virt_addr == 0);
 
@@ -474,18 +481,24 @@ static void ntb_transport_setup_qp_mw(struct ntb_transport *nt,
 	else
 		num_qps_mw = nt->max_qps / NTB_NUM_MW;
 
-	rx_size = nt->mw[mw_num].size / num_qps_mw;
-	qp->rx_buff_begin = nt->mw[mw_num].virt_addr +
-			    (qp_num / NTB_NUM_MW * rx_size);
-	qp->rx_buff_end = qp->rx_buff_begin + rx_size;
-	qp->rx_offset = qp->rx_buff_begin;
+	rx_size = (unsigned int) nt->mw[mw_num].size / num_qps_mw;
+	qp->remote_rx_info = nt->mw[mw_num].virt_addr +
+			     (qp_num / NTB_NUM_MW * rx_size);
+	rx_size -= sizeof(struct ntb_rx_info);
+
+	qp->rx_buff = qp->remote_rx_info + sizeof(struct ntb_rx_info);
 	qp->rx_max_frame = min(transport_mtu, rx_size);
+	qp->rx_max_entry = rx_size / qp->rx_max_frame;
+	qp->rx_index = 0;
+
+	qp->remote_rx_info->entry = qp->rx_max_entry;
 
 	/* setup the hdr offsets with 0's */
-	for (offset = qp->rx_buff_begin + qp->rx_max_frame -
-		      sizeof(struct ntb_payload_header);
-	     offset < qp->rx_buff_end; offset += qp->rx_max_frame)
+	for (i = 0; i < qp->rx_max_entry; i++) {
+		void *offset = qp->rx_buff + qp->rx_max_frame * (i + 1) -
+			       sizeof(struct ntb_payload_header);
 		memset(offset, 0, sizeof(struct ntb_payload_header));
+	}
 
 	qp->rx_pkts = 0;
 	qp->tx_pkts = 0;
@@ -762,12 +775,15 @@ static void ntb_transport_init_queue(struct ntb_transport *nt,
 	else
 		num_qps_mw = nt->max_qps / NTB_NUM_MW;
 
-	tx_size = ntb_get_mw_size(qp->ndev, mw_num) / num_qps_mw;
-	qp->tx_mw_begin = ntb_get_mw_vbase(nt->ndev, mw_num) +
-			  (qp_num / NTB_NUM_MW * tx_size);
-	qp->tx_mw_end = qp->tx_mw_begin + tx_size;
-	qp->tx_offset = qp->tx_mw_begin;
+	tx_size = (unsigned int) ntb_get_mw_size(qp->ndev, mw_num) / num_qps_mw;
+	qp->rx_info = ntb_get_mw_vbase(nt->ndev, mw_num) +
+		      (qp_num / NTB_NUM_MW * tx_size);
+	tx_size -= sizeof(struct ntb_rx_info);
+
+	qp->tx_mw = qp->rx_info + sizeof(struct ntb_rx_info);
 	qp->tx_max_frame = min(transport_mtu, tx_size);
+	qp->tx_max_entry = tx_size / qp->tx_max_frame;
+	qp->tx_index = 0;
 
 	if (nt->debugfs_dir) {
 		char debugfs_name[4];
@@ -894,21 +910,8 @@ void ntb_transport_free(void *transport)
 static void ntb_rx_copy_task(struct ntb_transport_qp *qp,
 			     struct ntb_queue_entry *entry, void *offset)
 {
-
-	struct ntb_payload_header *hdr;
-
-	BUG_ON(offset < qp->rx_buff_begin ||
-	       offset + qp->rx_max_frame >= qp->rx_buff_end);
-
-	hdr = offset + qp->rx_max_frame - sizeof(struct ntb_payload_header);
-	entry->len = hdr->len;
-
 	memcpy(entry->buf, offset, entry->len);
 
-	/* Ensure that the data is fully copied out before clearing the flag */
-	wmb();
-	hdr->flags = 0;
-
 	if (qp->rx_handler && qp->client_ready == NTB_LINK_UP)
 		qp->rx_handler(qp, qp->cb_data, entry->cb_data, entry->len);
 
@@ -921,10 +924,11 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 	struct ntb_queue_entry *entry;
 	void *offset;
 
+	offset = qp->rx_buff + qp->rx_max_frame * qp->rx_index;
+	hdr = offset + qp->rx_max_frame - sizeof(struct ntb_payload_header);
+
 	entry = ntb_list_rm(&qp->ntb_rx_pend_q_lock, &qp->rx_pend_q);
 	if (!entry) {
-		hdr = offset + qp->rx_max_frame -
-		      sizeof(struct ntb_payload_header);
 		dev_dbg(&ntb_query_pdev(qp->ndev)->dev,
 			"no buffer - HDR ver %llu, len %d, flags %x\n",
 			hdr->ver, hdr->len, hdr->flags);
@@ -932,9 +936,6 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 		return -ENOMEM;
 	}
 
-	offset = qp->rx_offset;
-	hdr = offset + qp->rx_max_frame - sizeof(struct ntb_payload_header);
-
 	if (!(hdr->flags & DESC_DONE_FLAG)) {
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
 			     &qp->rx_pend_q);
@@ -957,30 +958,20 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
 			     &qp->rx_pend_q);
-
-		/* Ensure that the data is fully copied out before clearing the
-		 * done flag
-		 */
-		wmb();
-		hdr->flags = 0;
 		goto out;
 	}
 
 	dev_dbg(&ntb_query_pdev(qp->ndev)->dev,
-		"rx offset %p, ver %llu - %d payload received, buf size %d\n",
-		qp->rx_offset, hdr->ver, hdr->len, entry->len);
+		"rx offset %u, ver %llu - %d payload received, buf size %d\n",
+		qp->rx_index, hdr->ver, hdr->len, entry->len);
 
-	if (hdr->len <= entry->len)
+	if (hdr->len <= entry->len) {
+		entry->len = hdr->len;
 		ntb_rx_copy_task(qp, entry, offset);
-	else {
+	} else {
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
 			     &qp->rx_pend_q);
 
-		/* Ensure that the data is fully copied out before clearing the
-		 * done flag
-		 */
-		wmb();
-		hdr->flags = 0;
 		qp->rx_err_oflow++;
 		dev_dbg(&ntb_query_pdev(qp->ndev)->dev,
 			"RX overflow! Wanted %d got %d\n",
@@ -991,9 +982,13 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 	qp->rx_pkts++;
 
 out:
-	qp->rx_offset += qp->rx_max_frame;
-	if (qp->rx_offset + qp->rx_max_frame >= qp->rx_buff_end)
-		qp->rx_offset = qp->rx_buff_begin;
+	/* Ensure that the data is fully copied out before clearing the flag */
+	wmb();
+	hdr->flags = 0;
+	qp->rx_info->entry = qp->rx_index;
+
+	qp->rx_index++;
+	qp->rx_index %= qp->rx_max_entry;
 
 	return 0;
 }
@@ -1024,9 +1019,6 @@ static void ntb_tx_copy_task(struct ntb_transport_qp *qp,
 {
 	struct ntb_payload_header *hdr;
 
-	BUG_ON(offset < qp->tx_mw_begin ||
-	       offset + qp->tx_max_frame >= qp->tx_mw_end);
-
 	memcpy_toio(offset, entry->buf, entry->len);
 
 	hdr = offset + qp->tx_max_frame - sizeof(struct ntb_payload_header);
@@ -1057,16 +1049,14 @@ static void ntb_tx_copy_task(struct ntb_transport_qp *qp,
 static int ntb_process_tx(struct ntb_transport_qp *qp,
 			  struct ntb_queue_entry *entry)
 {
-	struct ntb_payload_header *hdr;
 	void *offset;
 
-	offset = qp->tx_offset;
-	hdr = offset + qp->tx_max_frame - sizeof(struct ntb_payload_header);
+	offset = qp->tx_mw + qp->tx_max_frame * qp->tx_index;
 
-	dev_dbg(&ntb_query_pdev(qp->ndev)->dev, "%lld - offset %p, tx %p, entry len %d flags %x buff %p\n",
-		qp->tx_pkts, offset, qp->tx_offset, entry->len, entry->flags,
+	dev_dbg(&ntb_query_pdev(qp->ndev)->dev, "%lld - offset %p, tx %u, entry len %d flags %x buff %p\n",
+		qp->tx_pkts, offset, qp->tx_index, entry->len, entry->flags,
 		entry->buf);
-	if (hdr->flags) {
+	if (qp->tx_index == qp->remote_rx_info->entry) {
 		qp->tx_ring_full++;
 		return -EAGAIN;
 	}
@@ -1082,9 +1072,8 @@ static int ntb_process_tx(struct ntb_transport_qp *qp,
 
 	ntb_tx_copy_task(qp, entry, offset);
 
-	qp->tx_offset += qp->tx_max_frame;
-	if (qp->tx_offset + qp->tx_max_frame >= qp->tx_mw_end)
-		qp->tx_offset = qp->tx_mw_begin;
+	qp->tx_index++;
+	qp->tx_index %= qp->tx_max_entry;
 
 	qp->tx_pkts++;
 
-- 
1.7.9.5

[PATCH 13/21] NTB: Out of free receive entries issue

[Jon Mason]

If the NTB client driver enqueues the maximum number of rx buffers, it
will not be able to re-enqueue another in its callback handler due to a
lack of free entries.  This can be avoided by adding the current entry
to the free queue prior to calling the client callback handler.  With
this change, ntb_netdev will no longer encounter a rx error on its first
packet.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 69c58da..b3afb24 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -910,12 +910,15 @@ void ntb_transport_free(void *transport)
 static void ntb_rx_copy_task(struct ntb_transport_qp *qp,
 			     struct ntb_queue_entry *entry, void *offset)
 {
-	memcpy(entry->buf, offset, entry->len);
+	void *cb_data = entry->cb_data;
+	unsigned int len = entry->len;
 
-	if (qp->rx_handler && qp->client_ready == NTB_LINK_UP)
-		qp->rx_handler(qp, qp->cb_data, entry->cb_data, entry->len);
+	memcpy(entry->buf, offset, entry->len);
 
 	ntb_list_add(&qp->ntb_rx_free_q_lock, &entry->entry, &qp->rx_free_q);
+
+	if (qp->rx_handler && qp->client_ready == NTB_LINK_UP)
+		qp->rx_handler(qp, qp->cb_data, cb_data, len);
 }
 
 static int ntb_process_rxc(struct ntb_transport_qp *qp)
-- 
1.7.9.5

[PATCH 14/21] NTB: Fix Sparse Warnings

[Jon Mason]

Address the sparse warnings and resulting fallout

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_hw.c        |    7 ++++---
 drivers/ntb/ntb_hw.h        |    4 ++--
 drivers/ntb/ntb_transport.c |   32 ++++++++++++++++----------------
 3 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 18cb5dc..b792ccd 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -104,8 +104,9 @@ MODULE_DEVICE_TABLE(pci, ntb_pci_tbl);
  *
  * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
  */
-int ntb_register_event_callback(struct ntb_device *ndev,
-				void (*func)(void *handle, unsigned int event))
+int
+ntb_register_event_callback(struct ntb_device *ndev,
+			    void (*func)(void *handle, enum ntb_hw_event event))
 {
 	if (ndev->event_cb)
 		return -EINVAL;
@@ -344,7 +345,7 @@ int ntb_read_remote_spad(struct ntb_device *ndev, unsigned int idx, u32 *val)
  *
  * RETURNS: pointer to virtual address, or NULL on error.
  */
-void *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw)
+void __iomem *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw)
 {
 	if (mw > NTB_NUM_MW)
 		return NULL;
diff --git a/drivers/ntb/ntb_hw.h b/drivers/ntb/ntb_hw.h
index 5e00951..3a3038c 100644
--- a/drivers/ntb/ntb_hw.h
+++ b/drivers/ntb/ntb_hw.h
@@ -165,14 +165,14 @@ int ntb_register_db_callback(struct ntb_device *ndev, unsigned int idx,
 void ntb_unregister_db_callback(struct ntb_device *ndev, unsigned int idx);
 int ntb_register_event_callback(struct ntb_device *ndev,
 				void (*event_cb_func) (void *handle,
-						       unsigned int event));
+						      enum ntb_hw_event event));
 void ntb_unregister_event_callback(struct ntb_device *ndev);
 int ntb_get_max_spads(struct ntb_device *ndev);
 int ntb_write_local_spad(struct ntb_device *ndev, unsigned int idx, u32 val);
 int ntb_read_local_spad(struct ntb_device *ndev, unsigned int idx, u32 *val);
 int ntb_write_remote_spad(struct ntb_device *ndev, unsigned int idx, u32 val);
 int ntb_read_remote_spad(struct ntb_device *ndev, unsigned int idx, u32 *val);
-void *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw);
+void __iomem *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw);
 resource_size_t ntb_get_mw_size(struct ntb_device *ndev, unsigned int mw);
 void ntb_ring_sdb(struct ntb_device *ndev, unsigned int idx);
 void *ntb_find_transport(struct pci_dev *pdev);
diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index b3afb24..e0bdfd7 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -58,7 +58,7 @@
 #include <linux/ntb.h>
 #include "ntb_hw.h"
 
-#define NTB_TRANSPORT_VERSION	1
+#define NTB_TRANSPORT_VERSION	2
 
 static unsigned int transport_mtu = 0x401E;
 module_param(transport_mtu, uint, 0644);
@@ -91,14 +91,14 @@ struct ntb_transport_qp {
 	bool qp_link;
 	u8 qp_num;	/* Only 64 QP's are allowed.  0-63 */
 
-	struct ntb_rx_info *rx_info;
+	struct ntb_rx_info __iomem *rx_info;
 	struct ntb_rx_info *remote_rx_info;
 
 	void (*tx_handler) (struct ntb_transport_qp *qp, void *qp_data,
 			    void *data, int len);
 	struct list_head tx_free_q;
 	spinlock_t ntb_tx_free_q_lock;
-	void *tx_mw;
+	void __iomem *tx_mw;
 	unsigned int tx_index;
 	unsigned int tx_max_entry;
 	unsigned int tx_max_frame;
@@ -166,7 +166,7 @@ enum {
 };
 
 struct ntb_payload_header {
-	u64 ver;
+	unsigned int ver;
 	unsigned int len;
 	unsigned int flags;
 };
@@ -474,7 +474,7 @@ static void ntb_transport_setup_qp_mw(struct ntb_transport *nt,
 	u8 mw_num = QP_TO_MW(qp_num);
 	unsigned int i;
 
-	WARN_ON(nt->mw[mw_num].virt_addr == 0);
+	WARN_ON(nt->mw[mw_num].virt_addr == NULL);
 
 	if (nt->max_qps % NTB_NUM_MW && mw_num < nt->max_qps % NTB_NUM_MW)
 		num_qps_mw = nt->max_qps / NTB_NUM_MW + 1;
@@ -933,7 +933,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 	entry = ntb_list_rm(&qp->ntb_rx_pend_q_lock, &qp->rx_pend_q);
 	if (!entry) {
 		dev_dbg(&ntb_query_pdev(qp->ndev)->dev,
-			"no buffer - HDR ver %llu, len %d, flags %x\n",
+			"no buffer - HDR ver %u, len %d, flags %x\n",
 			hdr->ver, hdr->len, hdr->flags);
 		qp->rx_err_no_buf++;
 		return -ENOMEM;
@@ -946,9 +946,9 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 		return -EAGAIN;
 	}
 
-	if (hdr->ver != qp->rx_pkts) {
+	if (hdr->ver != (u32) qp->rx_pkts) {
 		dev_dbg(&ntb_query_pdev(qp->ndev)->dev,
-			"qp %d: version mismatch, expected %llu - got %llu\n",
+			"qp %d: version mismatch, expected %llu - got %u\n",
 			qp->qp_num, qp->rx_pkts, hdr->ver);
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
 			     &qp->rx_pend_q);
@@ -965,7 +965,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 	}
 
 	dev_dbg(&ntb_query_pdev(qp->ndev)->dev,
-		"rx offset %u, ver %llu - %d payload received, buf size %d\n",
+		"rx offset %u, ver %u - %d payload received, buf size %d\n",
 		qp->rx_index, hdr->ver, hdr->len, entry->len);
 
 	if (hdr->len <= entry->len) {
@@ -988,7 +988,7 @@ out:
 	/* Ensure that the data is fully copied out before clearing the flag */
 	wmb();
 	hdr->flags = 0;
-	qp->rx_info->entry = qp->rx_index;
+	iowrite32(qp->rx_index, &qp->rx_info->entry);
 
 	qp->rx_index++;
 	qp->rx_index %= qp->rx_max_entry;
@@ -1018,19 +1018,19 @@ static void ntb_transport_rxc_db(void *data, int db_num)
 
 static void ntb_tx_copy_task(struct ntb_transport_qp *qp,
 			     struct ntb_queue_entry *entry,
-			     void *offset)
+			     void __iomem *offset)
 {
-	struct ntb_payload_header *hdr;
+	struct ntb_payload_header __iomem *hdr;
 
 	memcpy_toio(offset, entry->buf, entry->len);
 
 	hdr = offset + qp->tx_max_frame - sizeof(struct ntb_payload_header);
-	hdr->len = entry->len;
-	hdr->ver = qp->tx_pkts;
+	iowrite32(entry->len, &hdr->len);
+	iowrite32((u32) qp->tx_pkts, &hdr->ver);
 
 	/* Ensure that the data is fully copied out before setting the flag */
 	wmb();
-	hdr->flags = entry->flags | DESC_DONE_FLAG;
+	iowrite32(entry->flags | DESC_DONE_FLAG, &hdr->flags);
 
 	ntb_ring_sdb(qp->ndev, qp->qp_num);
 
@@ -1052,7 +1052,7 @@ static void ntb_tx_copy_task(struct ntb_transport_qp *qp,
 static int ntb_process_tx(struct ntb_transport_qp *qp,
 			  struct ntb_queue_entry *entry)
 {
-	void *offset;
+	void __iomem *offset;
 
 	offset = qp->tx_mw + qp->tx_max_frame * qp->tx_index;
 
-- 
1.7.9.5

Re: [PATCH 14/21] NTB: Fix Sparse Warnings

[Greg KH]

On Sat, Jan 19, 2013 at 02:02:28AM -0700, Jon Mason wrote:
> Address the sparse warnings and resulting fallout
> 
> Signed-off-by: Jon Mason <jon.mason@intel.com>
> ---
>  drivers/ntb/ntb_hw.c        |    7 ++++---
>  drivers/ntb/ntb_hw.h        |    4 ++--
>  drivers/ntb/ntb_transport.c |   32 ++++++++++++++++----------------
>  3 files changed, 22 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index 18cb5dc..b792ccd 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -104,8 +104,9 @@ MODULE_DEVICE_TABLE(pci, ntb_pci_tbl);
>   *
>   * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
>   */
> -int ntb_register_event_callback(struct ntb_device *ndev,
> -				void (*func)(void *handle, unsigned int event))
> +int
> +ntb_register_event_callback(struct ntb_device *ndev,
> +			    void (*func)(void *handle, enum ntb_hw_event event))

What is sparse complaining about here that moving the function name to
the start of the line fixes?  That shouldn't be necessary at all.  Ah,
the enum?  If so, why change the first line at all?

>  {
>  	if (ndev->event_cb)
>  		return -EINVAL;
> @@ -344,7 +345,7 @@ int ntb_read_remote_spad(struct ntb_device *ndev, unsigned int idx, u32 *val)
>   *
>   * RETURNS: pointer to virtual address, or NULL on error.
>   */
> -void *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw)
> +void __iomem *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw)

That makes sense to fix.

>  {
>  	if (mw > NTB_NUM_MW)
>  		return NULL;
> diff --git a/drivers/ntb/ntb_hw.h b/drivers/ntb/ntb_hw.h
> index 5e00951..3a3038c 100644
> --- a/drivers/ntb/ntb_hw.h
> +++ b/drivers/ntb/ntb_hw.h
> @@ -165,14 +165,14 @@ int ntb_register_db_callback(struct ntb_device *ndev, unsigned int idx,
>  void ntb_unregister_db_callback(struct ntb_device *ndev, unsigned int idx);
>  int ntb_register_event_callback(struct ntb_device *ndev,
>  				void (*event_cb_func) (void *handle,
> -						       unsigned int event));
> +						      enum ntb_hw_event event));
>  void ntb_unregister_event_callback(struct ntb_device *ndev);
>  int ntb_get_max_spads(struct ntb_device *ndev);
>  int ntb_write_local_spad(struct ntb_device *ndev, unsigned int idx, u32 val);
>  int ntb_read_local_spad(struct ntb_device *ndev, unsigned int idx, u32 *val);
>  int ntb_write_remote_spad(struct ntb_device *ndev, unsigned int idx, u32 val);
>  int ntb_read_remote_spad(struct ntb_device *ndev, unsigned int idx, u32 *val);
> -void *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw);
> +void __iomem *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw);
>  resource_size_t ntb_get_mw_size(struct ntb_device *ndev, unsigned int mw);
>  void ntb_ring_sdb(struct ntb_device *ndev, unsigned int idx);
>  void *ntb_find_transport(struct pci_dev *pdev);
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index b3afb24..e0bdfd7 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -58,7 +58,7 @@
>  #include <linux/ntb.h>
>  #include "ntb_hw.h"
>  
> -#define NTB_TRANSPORT_VERSION	1
> +#define NTB_TRANSPORT_VERSION	2

How is this a sparse fix?

>  static unsigned int transport_mtu = 0x401E;
>  module_param(transport_mtu, uint, 0644);
> @@ -91,14 +91,14 @@ struct ntb_transport_qp {
>  	bool qp_link;
>  	u8 qp_num;	/* Only 64 QP's are allowed.  0-63 */
>  
> -	struct ntb_rx_info *rx_info;
> +	struct ntb_rx_info __iomem *rx_info;
>  	struct ntb_rx_info *remote_rx_info;
>  
>  	void (*tx_handler) (struct ntb_transport_qp *qp, void *qp_data,
>  			    void *data, int len);
>  	struct list_head tx_free_q;
>  	spinlock_t ntb_tx_free_q_lock;
> -	void *tx_mw;
> +	void __iomem *tx_mw;
>  	unsigned int tx_index;
>  	unsigned int tx_max_entry;
>  	unsigned int tx_max_frame;
> @@ -166,7 +166,7 @@ enum {
>  };
>  
>  struct ntb_payload_header {
> -	u64 ver;
> +	unsigned int ver;

why does sparse care about this?

thanks,

greg k-h

Re: [PATCH 14/21] NTB: Fix Sparse Warnings

[Jon Mason]

On Sun, Jan 20, 2013 at 03:45:34PM -0800, Greg KH wrote:
> On Sat, Jan 19, 2013 at 02:02:28AM -0700, Jon Mason wrote:
> > Address the sparse warnings and resulting fallout
> > 
> > Signed-off-by: Jon Mason <jon.mason@intel.com>
> > ---
> >  drivers/ntb/ntb_hw.c        |    7 ++++---
> >  drivers/ntb/ntb_hw.h        |    4 ++--
> >  drivers/ntb/ntb_transport.c |   32 ++++++++++++++++----------------
> >  3 files changed, 22 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > index 18cb5dc..b792ccd 100644
> > --- a/drivers/ntb/ntb_hw.c
> > +++ b/drivers/ntb/ntb_hw.c
> > @@ -104,8 +104,9 @@ MODULE_DEVICE_TABLE(pci, ntb_pci_tbl);
> >   *
> >   * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
> >   */
> > -int ntb_register_event_callback(struct ntb_device *ndev,
> > -				void (*func)(void *handle, unsigned int event))
> > +int
> > +ntb_register_event_callback(struct ntb_device *ndev,
> > +			    void (*func)(void *handle, enum ntb_hw_event event))
> 
> What is sparse complaining about here that moving the function name to
> the start of the line fixes?  That shouldn't be necessary at all.  Ah,
> the enum?  If so, why change the first line at all?

All an effort to get it to fit in under 80 chars.

> 
> >  {
> >  	if (ndev->event_cb)
> >  		return -EINVAL;
> > @@ -344,7 +345,7 @@ int ntb_read_remote_spad(struct ntb_device *ndev, unsigned int idx, u32 *val)
> >   *
> >   * RETURNS: pointer to virtual address, or NULL on error.
> >   */
> > -void *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw)
> > +void __iomem *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw)
> 
> That makes sense to fix.
> 
> >  {
> >  	if (mw > NTB_NUM_MW)
> >  		return NULL;
> > diff --git a/drivers/ntb/ntb_hw.h b/drivers/ntb/ntb_hw.h
> > index 5e00951..3a3038c 100644
> > --- a/drivers/ntb/ntb_hw.h
> > +++ b/drivers/ntb/ntb_hw.h
> > @@ -165,14 +165,14 @@ int ntb_register_db_callback(struct ntb_device *ndev, unsigned int idx,
> >  void ntb_unregister_db_callback(struct ntb_device *ndev, unsigned int idx);
> >  int ntb_register_event_callback(struct ntb_device *ndev,
> >  				void (*event_cb_func) (void *handle,
> > -						       unsigned int event));
> > +						      enum ntb_hw_event event));
> >  void ntb_unregister_event_callback(struct ntb_device *ndev);
> >  int ntb_get_max_spads(struct ntb_device *ndev);
> >  int ntb_write_local_spad(struct ntb_device *ndev, unsigned int idx, u32 val);
> >  int ntb_read_local_spad(struct ntb_device *ndev, unsigned int idx, u32 *val);
> >  int ntb_write_remote_spad(struct ntb_device *ndev, unsigned int idx, u32 val);
> >  int ntb_read_remote_spad(struct ntb_device *ndev, unsigned int idx, u32 *val);
> > -void *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw);
> > +void __iomem *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw);
> >  resource_size_t ntb_get_mw_size(struct ntb_device *ndev, unsigned int mw);
> >  void ntb_ring_sdb(struct ntb_device *ndev, unsigned int idx);
> >  void *ntb_find_transport(struct pci_dev *pdev);
> > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> > index b3afb24..e0bdfd7 100644
> > --- a/drivers/ntb/ntb_transport.c
> > +++ b/drivers/ntb/ntb_transport.c
> > @@ -58,7 +58,7 @@
> >  #include <linux/ntb.h>
> >  #include "ntb_hw.h"
> >  
> > -#define NTB_TRANSPORT_VERSION	1
> > +#define NTB_TRANSPORT_VERSION	2
> 
> How is this a sparse fix?

I changed the format of the header below.

> 
> >  static unsigned int transport_mtu = 0x401E;
> >  module_param(transport_mtu, uint, 0644);
> > @@ -91,14 +91,14 @@ struct ntb_transport_qp {
> >  	bool qp_link;
> >  	u8 qp_num;	/* Only 64 QP's are allowed.  0-63 */
> >  
> > -	struct ntb_rx_info *rx_info;
> > +	struct ntb_rx_info __iomem *rx_info;
> >  	struct ntb_rx_info *remote_rx_info;
> >  
> >  	void (*tx_handler) (struct ntb_transport_qp *qp, void *qp_data,
> >  			    void *data, int len);
> >  	struct list_head tx_free_q;
> >  	spinlock_t ntb_tx_free_q_lock;
> > -	void *tx_mw;
> > +	void __iomem *tx_mw;
> >  	unsigned int tx_index;
> >  	unsigned int tx_max_entry;
> >  	unsigned int tx_max_frame;
> > @@ -166,7 +166,7 @@ enum {
> >  };
> >  
> >  struct ntb_payload_header {
> > -	u64 ver;
> > +	unsigned int ver;
> 
> why does sparse care about this?

I am using the number of tx/rx packets transmitted as a sanity check
on the data.  This is a 64bit counter.  There is no iowrite64, so I
truncated this to 32bits.  I can rework this part of the patch to do 2
iowrite32's and keep the value as 64bit.

Thanks,
Jon

> 
> thanks,
> 
> greg k-h

Re: [PATCH 14/21] NTB: Fix Sparse Warnings

[Greg KH]

On Mon, Jan 21, 2013 at 02:13:50PM -0700, Jon Mason wrote:
> On Sun, Jan 20, 2013 at 03:45:34PM -0800, Greg KH wrote:
> > On Sat, Jan 19, 2013 at 02:02:28AM -0700, Jon Mason wrote:
> > > Address the sparse warnings and resulting fallout
> > > 
> > > Signed-off-by: Jon Mason <jon.mason@intel.com>
> > > ---
> > >  drivers/ntb/ntb_hw.c        |    7 ++++---
> > >  drivers/ntb/ntb_hw.h        |    4 ++--
> > >  drivers/ntb/ntb_transport.c |   32 ++++++++++++++++----------------
> > >  3 files changed, 22 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > > index 18cb5dc..b792ccd 100644
> > > --- a/drivers/ntb/ntb_hw.c
> > > +++ b/drivers/ntb/ntb_hw.c
> > > @@ -104,8 +104,9 @@ MODULE_DEVICE_TABLE(pci, ntb_pci_tbl);
> > >   *
> > >   * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
> > >   */
> > > -int ntb_register_event_callback(struct ntb_device *ndev,
> > > -				void (*func)(void *handle, unsigned int event))
> > > +int
> > > +ntb_register_event_callback(struct ntb_device *ndev,
> > > +			    void (*func)(void *handle, enum ntb_hw_event event))
> > 
> > What is sparse complaining about here that moving the function name to
> > the start of the line fixes?  That shouldn't be necessary at all.  Ah,
> > the enum?  If so, why change the first line at all?
> 
> All an effort to get it to fit in under 80 chars.

The original lines were less than 80 characters :)

> > > --- a/drivers/ntb/ntb_transport.c
> > > +++ b/drivers/ntb/ntb_transport.c
> > > @@ -58,7 +58,7 @@
> > >  #include <linux/ntb.h>
> > >  #include "ntb_hw.h"
> > >  
> > > -#define NTB_TRANSPORT_VERSION	1
> > > +#define NTB_TRANSPORT_VERSION	2
> > 
> > How is this a sparse fix?
> 
> I changed the format of the header below.

Ah, ok.

Care to resend this without the first chunk?

thanks,

greg k-h

Re: [PATCH 14/21] NTB: Fix Sparse Warnings

[Jon Mason]

On Mon, Jan 21, 2013 at 01:37:19PM -0800, Greg KH wrote:
> On Mon, Jan 21, 2013 at 02:13:50PM -0700, Jon Mason wrote:
> > On Sun, Jan 20, 2013 at 03:45:34PM -0800, Greg KH wrote:
> > > On Sat, Jan 19, 2013 at 02:02:28AM -0700, Jon Mason wrote:
> > > > Address the sparse warnings and resulting fallout
> > > > 
> > > > Signed-off-by: Jon Mason <jon.mason@intel.com>
> > > > ---
> > > >  drivers/ntb/ntb_hw.c        |    7 ++++---
> > > >  drivers/ntb/ntb_hw.h        |    4 ++--
> > > >  drivers/ntb/ntb_transport.c |   32 ++++++++++++++++----------------
> > > >  3 files changed, 22 insertions(+), 21 deletions(-)
> > > > 
> > > > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > > > index 18cb5dc..b792ccd 100644
> > > > --- a/drivers/ntb/ntb_hw.c
> > > > +++ b/drivers/ntb/ntb_hw.c
> > > > @@ -104,8 +104,9 @@ MODULE_DEVICE_TABLE(pci, ntb_pci_tbl);
> > > >   *
> > > >   * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
> > > >   */
> > > > -int ntb_register_event_callback(struct ntb_device *ndev,
> > > > -				void (*func)(void *handle, unsigned int event))
> > > > +int
> > > > +ntb_register_event_callback(struct ntb_device *ndev,
> > > > +			    void (*func)(void *handle, enum ntb_hw_event event))
> > > 
> > > What is sparse complaining about here that moving the function name to
> > > the start of the line fixes?  That shouldn't be necessary at all.  Ah,
> > > the enum?  If so, why change the first line at all?
> > 
> > All an effort to get it to fit in under 80 chars.
> 
> The original lines were less than 80 characters :)
> 
> > > > --- a/drivers/ntb/ntb_transport.c
> > > > +++ b/drivers/ntb/ntb_transport.c
> > > > @@ -58,7 +58,7 @@
> > > >  #include <linux/ntb.h>
> > > >  #include "ntb_hw.h"
> > > >  
> > > > -#define NTB_TRANSPORT_VERSION	1
> > > > +#define NTB_TRANSPORT_VERSION	2
> > > 
> > > How is this a sparse fix?
> > 
> > I changed the format of the header below.
> 
> Ah, ok.
> 
> Care to resend this without the first chunk?

Works for me :)

Resending with the minor edit now.

Thanks,
Jon

> 
> thanks,
> 
> greg k-h

[PATCH 15/21] NTB: Update Version

[Jon Mason]

Update NTB version to 0.25

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_hw.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index b792ccd..df86882 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -56,7 +56,7 @@
 #include "ntb_regs.h"
 
 #define NTB_NAME	"Intel(R) PCI-E Non-Transparent Bridge Driver"
-#define NTB_VER		"0.24"
+#define NTB_VER		"0.25"
 
 MODULE_DESCRIPTION(NTB_NAME);
 MODULE_VERSION(NTB_VER);
-- 
1.7.9.5

Re: [PATCH 15/21] NTB: Update Version

[Greg KH]

On Sat, Jan 19, 2013 at 02:02:29AM -0700, Jon Mason wrote:
> Update NTB version to 0.25
> 
> Signed-off-by: Jon Mason <jon.mason@intel.com>
> ---
>  drivers/ntb/ntb_hw.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index b792ccd..df86882 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -56,7 +56,7 @@
>  #include "ntb_regs.h"
>  
>  #define NTB_NAME	"Intel(R) PCI-E Non-Transparent Bridge Driver"
> -#define NTB_VER		"0.24"
> +#define NTB_VER		"0.25"

I'm not objecting to this, but in the end, does it really matter?  Why
not just stick with whatever kernel version you are using?  Keeping this
up to date is going to be hard over the long haul, right?

thanks,

greg k-h

Re: [PATCH 15/21] NTB: Update Version

[Jon Mason]

On Sun, Jan 20, 2013 at 03:47:14PM -0800, Greg KH wrote:
> On Sat, Jan 19, 2013 at 02:02:29AM -0700, Jon Mason wrote:
> > Update NTB version to 0.25
> > 
> > Signed-off-by: Jon Mason <jon.mason@intel.com>
> > ---
> >  drivers/ntb/ntb_hw.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > index b792ccd..df86882 100644
> > --- a/drivers/ntb/ntb_hw.c
> > +++ b/drivers/ntb/ntb_hw.c
> > @@ -56,7 +56,7 @@
> >  #include "ntb_regs.h"
> >  
> >  #define NTB_NAME	"Intel(R) PCI-E Non-Transparent Bridge Driver"
> > -#define NTB_VER		"0.24"
> > +#define NTB_VER		"0.25"
> 
> I'm not objecting to this, but in the end, does it really matter?  Why
> not just stick with whatever kernel version you are using?  Keeping this
> up to date is going to be hard over the long haul, right?

Currently, it is a convenient way to verify the people testing the
code are running the latest version, since they are usually running
their tests on a stable kernel.  As the code becomes more stable, I
can see this becoming unnecessary and I will remove it then.

Thanks,
Jon

> 
> thanks,
> 
> greg k-h

Re: [PATCH 15/21] NTB: Update Version

[Greg KH]

On Mon, Jan 21, 2013 at 10:57:41AM -0700, Jon Mason wrote:
> On Sun, Jan 20, 2013 at 03:47:14PM -0800, Greg KH wrote:
> > On Sat, Jan 19, 2013 at 02:02:29AM -0700, Jon Mason wrote:
> > > Update NTB version to 0.25
> > > 
> > > Signed-off-by: Jon Mason <jon.mason@intel.com>
> > > ---
> > >  drivers/ntb/ntb_hw.c |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > > index b792ccd..df86882 100644
> > > --- a/drivers/ntb/ntb_hw.c
> > > +++ b/drivers/ntb/ntb_hw.c
> > > @@ -56,7 +56,7 @@
> > >  #include "ntb_regs.h"
> > >  
> > >  #define NTB_NAME	"Intel(R) PCI-E Non-Transparent Bridge Driver"
> > > -#define NTB_VER		"0.24"
> > > +#define NTB_VER		"0.25"
> > 
> > I'm not objecting to this, but in the end, does it really matter?  Why
> > not just stick with whatever kernel version you are using?  Keeping this
> > up to date is going to be hard over the long haul, right?
> 
> Currently, it is a convenient way to verify the people testing the
> code are running the latest version, since they are usually running
> their tests on a stable kernel.  As the code becomes more stable, I
> can see this becoming unnecessary and I will remove it then.

Ok, that sounds good, thanks.

greg k-h

[PATCH 16/21] ntb_netdev: remove init/exit from probe/remove

[Jon Mason]

Remove init/exit from probe/remove routines to correct warnings of
"Section mismatch".

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/net/ntb_netdev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
index 4e52fd2..0d4a6ee 100644
--- a/drivers/net/ntb_netdev.c
+++ b/drivers/net/ntb_netdev.c
@@ -373,7 +373,7 @@ err:
 	return rc;
 }
 
-static void __exit ntb_netdev_remove(struct pci_dev *pdev)
+static void ntb_netdev_remove(struct pci_dev *pdev)
 {
 	struct net_device *ndev;
 	struct ntb_netdev *dev;
-- 
1.7.9.5

[PATCH 17/21] ntb_netdev: correct skb leak

[Jon Mason]

If ntb_netdev is unable to pass a new skb to the ntb transport for
future rx packets, it should free the newly alloc'ed skb in the error
case.  Found by Kernel memory leak detector.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/net/ntb_netdev.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
index 0d4a6ee..28d6fea 100644
--- a/drivers/net/ntb_netdev.c
+++ b/drivers/net/ntb_netdev.c
@@ -119,6 +119,7 @@ static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,
 
 	rc = ntb_transport_rx_enqueue(qp, skb, skb->data, ndev->mtu + ETH_HLEN);
 	if (rc) {
+		dev_kfree_skb(skb);
 		ndev->stats.rx_errors++;
 		ndev->stats.rx_fifo_errors++;
 	}
-- 
1.7.9.5

[PATCH 18/21] ntb_netdev: remove tx timeout

[Jon Mason]

There is a race between disabling and enabling the tx queue, resulting
in tx timeouts.  Since all the tx timeout does is re-enable the tx
queue, simple remove the start/stop of the queue and the tx timeout
routine.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/net/ntb_netdev.c |   11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
index 28d6fea..07c06cd 100644
--- a/drivers/net/ntb_netdev.c
+++ b/drivers/net/ntb_netdev.c
@@ -144,9 +144,6 @@ static void ntb_netdev_tx_handler(struct ntb_transport_qp *qp, void *qp_data,
 	}
 
 	dev_kfree_skb(skb);
-
-	if (netif_queue_stopped(ndev))
-		netif_wake_queue(ndev);
 }
 
 static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
@@ -166,7 +163,6 @@ static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
 err:
 	ndev->stats.tx_dropped++;
 	ndev->stats.tx_errors++;
-	netif_stop_queue(ndev);
 	return NETDEV_TX_BUSY;
 }
 
@@ -270,18 +266,11 @@ err:
 	return rc;
 }
 
-static void ntb_netdev_tx_timeout(struct net_device *ndev)
-{
-	if (netif_running(ndev))
-		netif_wake_queue(ndev);
-}
-
 static const struct net_device_ops ntb_netdev_ops = {
 	.ndo_open = ntb_netdev_open,
 	.ndo_stop = ntb_netdev_close,
 	.ndo_start_xmit = ntb_netdev_start_xmit,
 	.ndo_change_mtu = ntb_netdev_change_mtu,
-	.ndo_tx_timeout = ntb_netdev_tx_timeout,
 	.ndo_set_mac_address = eth_mac_addr,
 };
 
-- 
1.7.9.5

[PATCH 19/21] ntb_netdev: declare unused variables and fix missing initializer

[Jon Mason]

Tag qp in ntb_netdev_tx_handler and net_device in ntb_get_settings as
unused function parameters.  Also, correct missing initializer errors
for 'struct device_driver' in ntb_netdev_client.  This corrects issues
found by 'EXTRA_CFLAGS=-W'.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/net/ntb_netdev.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
index 07c06cd..37173ce 100644
--- a/drivers/net/ntb_netdev.c
+++ b/drivers/net/ntb_netdev.c
@@ -125,8 +125,9 @@ static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,
 	}
 }
 
-static void ntb_netdev_tx_handler(struct ntb_transport_qp *qp, void *qp_data,
-				  void *data, int len)
+static void
+ntb_netdev_tx_handler(__attribute__((unused)) struct ntb_transport_qp *qp,
+		      void *qp_data, void *data, int len)
 {
 	struct net_device *ndev = qp_data;
 	struct sk_buff *skb;
@@ -284,7 +285,8 @@ static void ntb_get_drvinfo(struct net_device *ndev,
 	strlcpy(info->bus_info, pci_name(dev->pdev), sizeof(info->bus_info));
 }
 
-static int ntb_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+static int ntb_get_settings(__attribute__((unused)) struct net_device *dev,
+			    struct ethtool_cmd *cmd)
 {
 	cmd->supported = SUPPORTED_Backplane;
 	cmd->advertising = ADVERTISED_Backplane;
@@ -383,8 +385,10 @@ static void ntb_netdev_remove(struct pci_dev *pdev)
 }
 
 static struct ntb_client ntb_netdev_client = {
-	.driver.name = KBUILD_MODNAME,
-	.driver.owner = THIS_MODULE,
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.owner = THIS_MODULE,
+	},
 	.probe = ntb_netdev_probe,
 	.remove = ntb_netdev_remove,
 };
-- 
1.7.9.5

Re: [PATCH 19/21] ntb_netdev: declare unused variables and fix missing initializer

[Greg KH]

On Sat, Jan 19, 2013 at 02:02:33AM -0700, Jon Mason wrote:
> Tag qp in ntb_netdev_tx_handler and net_device in ntb_get_settings as
> unused function parameters.  Also, correct missing initializer errors
> for 'struct device_driver' in ntb_netdev_client.  This corrects issues
> found by 'EXTRA_CFLAGS=-W'.

Again, why would you want to ever build the kernel with that option?
This shouldn't be needed.

greg k-h

[PATCH 20/21] ntb_netdev: improve logging

[Jon Mason]

Improve driver logging to be more helpful

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/net/ntb_netdev.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
index 37173ce..02213a8 100644
--- a/drivers/net/ntb_netdev.c
+++ b/drivers/net/ntb_netdev.c
@@ -153,7 +153,7 @@ static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
 	struct ntb_netdev *dev = netdev_priv(ndev);
 	int rc;
 
-	netdev_dbg(ndev, "ntb_transport_tx_enqueue\n");
+	netdev_dbg(ndev, "%s: skb len %d\n", __func__, skb->len);
 
 	rc = ntb_transport_tx_enqueue(dev->qp, skb, skb->data, skb->len);
 	if (rc)
@@ -355,7 +355,7 @@ static int ntb_netdev_probe(struct pci_dev *pdev)
 		goto err1;
 
 	list_add(&dev->list, &dev_list);
-	pr_info("%s: %s created\n", KBUILD_MODNAME, ndev->name);
+	dev_info(&pdev->dev, "%s created\n", ndev->name);
 	return 0;
 
 err1:
@@ -408,6 +408,5 @@ static void __exit ntb_netdev_exit_module(void)
 {
 	ntb_unregister_client(&ntb_netdev_client);
 	ntb_unregister_client_dev(KBUILD_MODNAME);
-	pr_info("%s: Driver removed\n", KBUILD_MODNAME);
 }
 module_exit(ntb_netdev_exit_module);
-- 
1.7.9.5

Re: [PATCH 20/21] ntb_netdev: improve logging

[Joe Perches]

On Sat, 2013-01-19 at 02:02 -0700, Jon Mason wrote:
> Improve driver logging to be more helpful
[]
> diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
[]
> @@ -408,6 +408,5 @@ static void __exit ntb_netdev_exit_module(void)
>  {
>  	ntb_unregister_client(&ntb_netdev_client);
>  	ntb_unregister_client_dev(KBUILD_MODNAME);
> -	pr_info("%s: Driver removed\n", KBUILD_MODNAME);

It's almost always true that using
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
before any #include

and changing all pr_<level>( uses from

	pr_<level>("s: etc...", KBUiLD_MODNAME, ...)
to
	pr_<level>("etc...", ...)

produces overall smaller object code.
That's even more likely for 64 bit compiled code.

[PATCH 21/21] ntb_netdev: Update Version

[Jon Mason]

Update NTB netdev version to 0.7

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/net/ntb_netdev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
index 02213a8..6ea1a7b 100644
--- a/drivers/net/ntb_netdev.c
+++ b/drivers/net/ntb_netdev.c
@@ -51,7 +51,7 @@
 #include <linux/pci.h>
 #include <linux/ntb.h>
 
-#define NTB_NETDEV_VER	"0.6"
+#define NTB_NETDEV_VER	"0.7"
 
 MODULE_DESCRIPTION(KBUILD_MODNAME);
 MODULE_VERSION(NTB_NETDEV_VER);
-- 
1.7.9.5

Re: [PATCH 0/21] NTB and ntb_netdev patches

[Greg KH]

On Sat, Jan 19, 2013 at 02:02:14AM -0700, Jon Mason wrote:
> Please apply these patches to Greg KH's char-misc-next tree.

I've applied most of them, other than the ones I've commented on.

thanks,

greg k-h