NTB and ntb_netdev patches

PATCH In review Jon Mason 4 participants 41 messages Jan 19, 2013 – Jan 21, 2013

Subsystems (3)

Networking Network drivers NTB driver core

lore

Share & tools

lore

Jon Mason

author

2013-01-19 09:02 UTC #0 lore raw

cover NTB and ntb_netdev patches

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

Thanks,
Jon

Jon Mason

author

2013-01-19 09:02 UTC #1 lore raw

PATCH 1/21 NTB: correct missing readq/writeq errors

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 <[email protected]>

---
 drivers/ntb/ntb_hw.c |    1 +
 1 file changed, 1 insertion(+)

drivers/ntb/ntb_hw.c +1 -0

index 4c71b17..0b46fef 100644
@@ -45,6 +45,7 @@
  * Contact Information:
  * Jon Mason <[email protected]>
  */
+#include <asm-generic/io-64-nonatomic-hi-lo.h>
 #include <linux/debugfs.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>

Signature · 1 line

1.7.9.5

Greg KH

2013-01-20 23:40 UTC #2 lore raw

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

Quoted 15 lines · 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 <[email protected]>
---
 drivers/ntb/ntb_hw.c |    1 +
 1 file changed, 1 insertion(+)
drivers/ntb/ntb_hw.c +0 -0
index 4c71b17..0b46fef 100644

drivers/ntb/ntb_hw.c line 48

Quoted 4 lines

drivers/ntb/ntb_hw.c +1 -0

@@ -45,6 +45,7 @@
  * Contact Information:
  * Jon Mason <[email protected]>
  */
+#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

Jon Mason

author

2013-01-21 17:38 UTC #3 lore raw

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

Quoted 11 lines · Greg KH

Quoted 10 lines · 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 <[email protected]>
---
 drivers/ntb/ntb_hw.c |    1 +
 1 file changed, 1 insertion(+)

Quoted 14 lines

drivers/ntb/ntb_hw.c line 48
Quoted 9 lines
drivers/ntb/ntb_hw.c +1 -0
index 4c71b17..0b46fef 100644
@@ -45,6 +45,7 @@
  * Contact Information:
  * Jon Mason <[email protected]>
  */
+#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

Quoted 4 lines

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

Greg KH

2013-01-21 18:22 UTC #4 lore raw

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

Quoted 27 lines · Jon Mason

Quoted 11 lines · Greg KH
Quoted 10 lines · 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 <[email protected]>
---
 drivers/ntb/ntb_hw.c |    1 +
 1 file changed, 1 insertion(+)
Quoted 14 lines
drivers/ntb/ntb_hw.c line 48
Quoted 9 lines
drivers/ntb/ntb_hw.c +1 -0
index 4c71b17..0b46fef 100644
@@ -45,6 +45,7 @@
  * Contact Information:
  * Jon Mason <[email protected]>
  */
+#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.

Quoted 5 lines

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

Ben Hutchings

2013-01-21 20:34 UTC #5 lore raw

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

Quoted 28 lines · Greg KH

Quoted 27 lines · Jon Mason
Quoted 25 lines · Greg KH
Quoted 15 lines · 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 <[email protected]>
---
 drivers/ntb/ntb_hw.c |    1 +
 1 file changed, 1 insertion(+)
drivers/ntb/ntb_hw.c +0 -0
index 4c71b17..0b46fef 100644
drivers/ntb/ntb_hw.c line 48
Quoted 4 lines
drivers/ntb/ntb_hw.c +1 -0
@@ -45,6 +45,7 @@
  * Contact Information:
  * Jon Mason <[email protected]>
  */
+#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.

Quoted 7 lines

Quoted 5 lines
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.

Quoted 5 lines

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.

Signature · 3 lines

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.

Greg KH

2013-01-21 20:47 UTC #6 lore raw

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

Quoted 35 lines · Ben Hutchings

Quoted 34 lines · Greg KH
Quoted 32 lines · Jon Mason
Quoted 11 lines · Greg KH
Quoted 10 lines · 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 <[email protected]>
---
 drivers/ntb/ntb_hw.c |    1 +
 1 file changed, 1 insertion(+)
Quoted 14 lines
drivers/ntb/ntb_hw.c line 48
Quoted 9 lines
drivers/ntb/ntb_hw.c +1 -0
index 4c71b17..0b46fef 100644
@@ -45,6 +45,7 @@
  * Contact Information:
  * Jon Mason <[email protected]>
  */
+#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.

Quoted 3 lines

Quoted 1 line
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

Jon Mason

author

2013-01-19 09:02 UTC #7 lore raw

PATCH 2/21 NTB: Handle ntb client device probes without present hardware

In reply to 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 <[email protected]>

---
 drivers/ntb/ntb_transport.c |    6 ++++++
 1 file changed, 6 insertions(+)

drivers/ntb/ntb_transport.c +6 -0

index 250190f..1d17857 100644
@@ -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);

Signature · 1 line

1.7.9.5

Jon Mason

author

2013-01-19 09:02 UTC #8 lore raw

PATCH 3/21 NTB: correct memory barrier

In reply to 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 <[email protected]>

---
 drivers/ntb/ntb_transport.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/ntb/ntb_transport.c +1 -1

index 1d17857..e9666bd 100644
@@ -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);

Signature · 1 line

1.7.9.5

Jon Mason

author

2013-01-19 09:03 UTC #9 lore raw

PATCH 4/21 NTB: separate transmit and receive windows

In reply to 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 <[email protected]>

---
 drivers/ntb/ntb_transport.c |   79 ++++++++++++++++++++++++-------------------
 1 file changed, 44 insertions(+), 35 deletions(-)

drivers/ntb/ntb_transport.c +44 ~~-35~~

index e9666bd..2823087 100644
@@ -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);

Signature · 1 line

1.7.9.5

Jon Mason

author

2013-01-19 09:02 UTC #10 lore raw

PATCH 5/21 NTB: No sleeping in interrupt context

In reply to 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 <[email protected]>

---
 drivers/ntb/ntb_transport.c |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

drivers/ntb/ntb_transport.c +17 -3

index 2823087..bf7ade1 100644
@@ -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);

Signature · 1 line

1.7.9.5

Jon Mason

author

2013-01-19 09:03 UTC #11 lore raw

PATCH 6/21 NTB: use simple_open for debugfs

In reply to Jon Mason

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

Signed-off-by Jon Mason <[email protected]>

---
 drivers/ntb/ntb_transport.c |    8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

drivers/ntb/ntb_transport.c +1 -7

index bf7ade1..c0eca02 100644
@@ -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,
 };

Signature · 1 line

1.7.9.5

Jon Mason

author

2013-01-19 09:02 UTC #12 lore raw

PATCH 7/21 NTB: zero PCI driver data

In reply to Jon Mason

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

Signed-off-by Jon Mason <[email protected]>

---
 drivers/ntb/ntb_hw.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

drivers/ntb/ntb_hw.c +8 -8

index 0b46fef..867ccaa 100644
@@ -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);

Signature · 1 line

1.7.9.5

Greg KH

2013-01-20 23:41 UTC #13 lore raw

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

Quoted 22 lines · Jon Mason

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

Signed-off-by Jon Mason <[email protected]>

---
 drivers/ntb/ntb_hw.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

drivers/ntb/ntb_hw.c +0 -6

index 0b46fef..867ccaa 100644
@@ -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)},

drivers/ntb/ntb_hw.c enum { · line 93

Quoted 10 lines

drivers/ntb/ntb_hw.c +8 -2

-	{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

Jon Mason

author

2013-01-19 09:02 UTC #14 lore raw

PATCH 8/21 NTB: declare unused variables

In reply to 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 <[email protected]>

---
 drivers/ntb/ntb_hw.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/ntb/ntb_hw.c +2 -1

index 867ccaa..2357c2d 100644
@@ -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;

Signature · 1 line

1.7.9.5

Greg KH

2013-01-20 23:42 UTC #15 lore raw

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

Quoted 2 lines · Jon Mason

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

Jon Mason

author

2013-01-21 17:50 UTC #16 lore raw

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

Quoted 5 lines · Greg KH

Quoted 2 lines · Jon Mason
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

Quoted 5 lines

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

Greg KH

2013-01-21 18:24 UTC #17 lore raw

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

Quoted 9 lines · Jon Mason

Quoted 5 lines · Greg KH
Quoted 2 lines · Jon Mason
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:

Quoted 8 lines

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.

Quoted 1 line

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

Understood, I've not applied them either :)

thanks,

greg k-h

Jon Mason

author

2013-01-19 09:02 UTC #18 lore raw

PATCH 9/21 NTB: namespacecheck cleanups

In reply to 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 <[email protected]>

---
 drivers/ntb/ntb_hw.c        |   14 --------------
 drivers/ntb/ntb_transport.c |    2 +-
 2 files changed, 1 insertion(+), 15 deletions(-)

drivers/ntb/ntb_hw.c +0 ~~-14~~

index 2357c2d..18cb5dc 100644
@@ -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

drivers/ntb/ntb_transport.c +1 -1

index c0eca02..903a72e 100644
@@ -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,

Signature · 1 line

1.7.9.5

Jon Mason

author

2013-01-19 09:02 UTC #19 lore raw

PATCH 10/21 NTB: whitespace cleanups

In reply to Jon Mason

Whitespace cleanups found via indent

Signed-off-by Jon Mason <[email protected]>

---
 drivers/ntb/ntb_transport.c |   40 ++++++++++++++++------------------------
 1 file changed, 16 insertions(+), 24 deletions(-)

drivers/ntb/ntb_transport.c +16 ~~-24~~

index 903a72e..e11b57e 100644
@@ -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;
 }

Signature · 1 line

1.7.9.5

Jon Mason

author

2013-01-19 09:02 UTC #20 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/ntb/ntb_transport.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

drivers/ntb/ntb_transport.c +10 -3

index e11b57e..1bed1ba 100644
@@ -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;
 }

Signature · 1 line

1.7.9.5

Jon Mason

author

2013-01-19 09:02 UTC #21 lore raw

PATCH 12/21 NTB: Remove reads across NTB

In reply to 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 <[email protected]>

---
 drivers/ntb/ntb_transport.c |  137 ++++++++++++++++++++-----------------------
 1 file changed, 63 insertions(+), 74 deletions(-)

drivers/ntb/ntb_transport.c +63 ~~-74~~

index 1bed1ba..69c58da 100644
@@ -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++;

Signature · 1 line

1.7.9.5

Jon Mason

author

2013-01-19 09:02 UTC #22 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/ntb/ntb_transport.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

drivers/ntb/ntb_transport.c +6 -3

index 69c58da..b3afb24 100644
@@ -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)

Signature · 1 line

1.7.9.5

Jon Mason

author

2013-01-19 09:02 UTC #23 lore raw

PATCH 14/21 NTB: Fix Sparse Warnings

In reply to Jon Mason

Address the sparse warnings and resulting fallout

Signed-off-by Jon Mason <[email protected]>

---
 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(-)

drivers/ntb/ntb_hw.c +4 -3

index 18cb5dc..b792ccd 100644
@@ -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;

drivers/ntb/ntb_hw.h +2 -2

index 5e00951..3a3038c 100644
@@ -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);

drivers/ntb/ntb_transport.c +16 ~~-16~~

index b3afb24..e0bdfd7 100644
@@ -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;

Signature · 1 line

1.7.9.5

Greg KH

2013-01-20 23:45 UTC #24 lore raw

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

Quoted 14 lines · Jon Mason

Address the sparse warnings and resulting fallout
Signed-off-by Jon Mason <[email protected]>
---
 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(-)
drivers/ntb/ntb_hw.c +0 -0
index 18cb5dc..b792ccd 100644

drivers/ntb/ntb_hw.c MODULE_DEVICE_TABLE(pci, ntb_pci_tbl); · line 109

Quoted 8 lines

drivers/ntb/ntb_hw.c +3 -2

@@ -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?

drivers/ntb/ntb_hw.c int ntb_read_remote_spad(struct ntb_device *ndev, unsigned int idx, u32 *val) · line 348

Quoted 9 lines

drivers/ntb/ntb_hw.c +1 -1

 {
 	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.

Quoted 29 lines

drivers/ntb/ntb_hw.c +0 -0

 {
 	if (mw > NTB_NUM_MW)
 		return NULL;

drivers/ntb/ntb_hw.h +2 -2

index 5e00951..3a3038c 100644
@@ -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);

drivers/ntb/ntb_transport.c +0 -0

index b3afb24..e0bdfd7 100644

drivers/ntb/ntb_transport.c line 61

Quoted 5 lines

drivers/ntb/ntb_transport.c +1 -1

@@ -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?

Quoted 20 lines

drivers/ntb/ntb_transport.c +2 -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;

drivers/ntb/ntb_transport.c enum { · line 169

Quoted 5 lines

drivers/ntb/ntb_transport.c +1 -1

@@ -166,7 +166,7 @@ enum {
 };
 
 struct ntb_payload_header {
-	u64 ver;
+	unsigned int ver;

why does sparse care about this?

thanks,

greg k-h

Jon Mason

author

2013-01-21 21:13 UTC #25 lore raw

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

Quoted 19 lines · Greg KH

Quoted 18 lines · Jon Mason

Address the sparse warnings and resulting fallout

Signed-off-by Jon Mason <[email protected]>

---
 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(-)

drivers/ntb/ntb_hw.c +0 -1

index 18cb5dc..b792ccd 100644
@@ -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,

Quoted 8 lines

drivers/ntb/ntb_hw.c MODULE_DEVICE_TABLE(pci, ntb_pci_tbl); · line 109
Quoted 4 lines
drivers/ntb/ntb_hw.c +3 -1
-				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.

Quoted 45 lines

drivers/ntb/ntb_hw.c int ntb_read_remote_spad(struct ntb_device *ndev, unsigned int idx, u32 *val) · line 348

Quoted 10 lines

drivers/ntb/ntb_hw.c +1 -1

 {
 	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.

Quoted 32 lines

drivers/ntb/ntb_hw.c +0 -0

 {
 	if (mw > NTB_NUM_MW)
 		return NULL;

drivers/ntb/ntb_hw.h +2 -2

index 5e00951..3a3038c 100644
@@ -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);

drivers/ntb/ntb_transport.c +0 -0

index b3afb24..e0bdfd7 100644
@@ -58,7 +58,7 @@
 #include <linux/ntb.h>
 #include "ntb_hw.h"

Quoted 4 lines

drivers/ntb/ntb_transport.c line 61
Quoted 2 lines
drivers/ntb/ntb_transport.c +1 -1
-#define NTB_TRANSPORT_VERSION	1
+#define NTB_TRANSPORT_VERSION	2
How is this a sparse fix?

I changed the format of the header below.

Quoted 23 lines

drivers/ntb/ntb_transport.c +2 -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 {
 };

Quoted 5 lines

drivers/ntb/ntb_transport.c enum { · line 169
Quoted 3 lines
drivers/ntb/ntb_transport.c +1 -1
 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

Quoted 4 lines

thanks,
greg k-h

Greg KH

2013-01-21 21:37 UTC #26 lore raw

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

Quoted 29 lines · Jon Mason

Quoted 19 lines · Greg KH
Quoted 18 lines · Jon Mason
Address the sparse warnings and resulting fallout
Signed-off-by Jon Mason <[email protected]>
---
 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(-)
drivers/ntb/ntb_hw.c +0 -1
index 18cb5dc..b792ccd 100644
@@ -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,
Quoted 8 lines
drivers/ntb/ntb_hw.c MODULE_DEVICE_TABLE(pci, ntb_pci_tbl); · line 109
Quoted 4 lines
drivers/ntb/ntb_hw.c +3 -1
-				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?

Quoted 1 line

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

The original lines were less than 80 characters :)

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
drivers/ntb/ntb_transport.c +0 -0
@@ -58,7 +58,7 @@
 #include <linux/ntb.h>
 #include "ntb_hw.h"
 

Quoted 6 lines

Quoted 4 lines
drivers/ntb/ntb_transport.c line 61
Quoted 2 lines
drivers/ntb/ntb_transport.c +1 -1
-#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

Jon Mason

author

2013-01-21 22:28 UTC #27 lore raw

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

Quoted 45 lines · Greg KH

Quoted 29 lines · Jon Mason
Quoted 19 lines · Greg KH
Quoted 18 lines · Jon Mason
Address the sparse warnings and resulting fallout
Signed-off-by Jon Mason <[email protected]>
---
 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(-)
drivers/ntb/ntb_hw.c +0 -1
index 18cb5dc..b792ccd 100644
@@ -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,
Quoted 8 lines
drivers/ntb/ntb_hw.c MODULE_DEVICE_TABLE(pci, ntb_pci_tbl); · line 109
Quoted 4 lines
drivers/ntb/ntb_hw.c +3 -1
-				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?
Quoted 1 line
All an effort to get it to fit in under 80 chars.
The original lines were less than 80 characters :)
Quoted 11 lines
Quoted 10 lines
drivers/ntb/ntb_transport.c line 61
Quoted 8 lines
drivers/ntb/ntb_transport.c +1 -1
@@ -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?

Quoted 5 lines

Quoted 1 line
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

Quoted 4 lines

thanks,
greg k-h

Jon Mason

author

2013-01-19 09:02 UTC #28 lore raw

PATCH 15/21 NTB: Update Version

In reply to Jon Mason

Update NTB version to 0.25

Signed-off-by Jon Mason <[email protected]>

---
 drivers/ntb/ntb_hw.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/ntb/ntb_hw.c +1 -1

index b792ccd..df86882 100644
@@ -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);

Signature · 1 line

1.7.9.5

Greg KH

2013-01-20 23:47 UTC #29 lore raw

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

Quoted 12 lines · Jon Mason

Update NTB version to 0.25
Signed-off-by Jon Mason <[email protected]>
---
 drivers/ntb/ntb_hw.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/ntb/ntb_hw.c +0 -0
index b792ccd..df86882 100644

drivers/ntb/ntb_hw.c line 59

Quoted 5 lines

drivers/ntb/ntb_hw.c +1 -1

@@ -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

Jon Mason

author

2013-01-21 17:57 UTC #30 lore raw

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

Quoted 15 lines · Greg KH

Quoted 14 lines · Jon Mason
Update NTB version to 0.25
Signed-off-by Jon Mason <[email protected]>
---
 drivers/ntb/ntb_hw.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/ntb/ntb_hw.c +0 -0
index b792ccd..df86882 100644
@@ -56,7 +56,7 @@
 #include "ntb_regs.h"
 

Quoted 7 lines

drivers/ntb/ntb_hw.c line 59
Quoted 3 lines
drivers/ntb/ntb_hw.c +1 -1
 #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

Quoted 4 lines

thanks,
greg k-h

Greg KH

2013-01-21 18:24 UTC #31 lore raw

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

Quoted 24 lines · Jon Mason

Quoted 15 lines · Greg KH
Quoted 14 lines · Jon Mason
Update NTB version to 0.25
Signed-off-by Jon Mason <[email protected]>
---
 drivers/ntb/ntb_hw.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/ntb/ntb_hw.c +0 -0
index b792ccd..df86882 100644
@@ -56,7 +56,7 @@
 #include "ntb_regs.h"
 
Quoted 7 lines
drivers/ntb/ntb_hw.c line 59
Quoted 3 lines
drivers/ntb/ntb_hw.c +1 -1
 #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?

Quoted 4 lines

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

Jon Mason

author

2013-01-19 09:02 UTC #32 lore raw

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

In reply to Jon Mason

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

Signed-off-by Jon Mason <[email protected]>

---
 drivers/net/ntb_netdev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ntb_netdev.c +1 -1

index 4e52fd2..0d4a6ee 100644
@@ -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;

Signature · 1 line

1.7.9.5

Jon Mason

author

2013-01-19 09:02 UTC #33 lore raw

PATCH 17/21 ntb_netdev: correct skb leak

In reply to 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 <[email protected]>

---
 drivers/net/ntb_netdev.c |    1 +
 1 file changed, 1 insertion(+)

drivers/net/ntb_netdev.c +1 -0

index 0d4a6ee..28d6fea 100644
@@ -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++;
 	}

Signature · 1 line

1.7.9.5

Jon Mason

author

2013-01-19 09:02 UTC #34 lore raw

PATCH 18/21 ntb_netdev: remove tx timeout

In reply to 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 <[email protected]>

---
 drivers/net/ntb_netdev.c |   11 -----------
 1 file changed, 11 deletions(-)

drivers/net/ntb_netdev.c +0 ~~-11~~

index 28d6fea..07c06cd 100644
@@ -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,
 };

Signature · 1 line

1.7.9.5

Jon Mason

author

2013-01-19 09:02 UTC #35 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/net/ntb_netdev.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

drivers/net/ntb_netdev.c +9 -5

index 07c06cd..37173ce 100644
@@ -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,
 };

Signature · 1 line

1.7.9.5

Greg KH

2013-01-20 23:47 UTC #36 lore raw

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

Quoted 4 lines · 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'.

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

greg k-h

Jon Mason

author

2013-01-19 09:02 UTC #37 lore raw

PATCH 20/21 ntb_netdev: improve logging

In reply to Jon Mason

Improve driver logging to be more helpful

Signed-off-by Jon Mason <[email protected]>

---
 drivers/net/ntb_netdev.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

drivers/net/ntb_netdev.c +2 -3

index 37173ce..02213a8 100644
@@ -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);

Signature · 1 line

1.7.9.5

Joe Perches

2013-01-19 18:51 UTC #38 lore raw

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

Quoted 1 line · Jon Mason

Improve driver logging to be more helpful

[]

drivers/net/ntb_netdev.c

Quoted 1 line

drivers/net/ntb_netdev.c +0 -0

[]

drivers/net/ntb_netdev.c static void __exit ntb_netdev_exit_module(void) · line 411

Quoted 5 lines

drivers/net/ntb_netdev.c +0 -1

@@ -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.

Jon Mason

author

2013-01-19 09:02 UTC #39 lore raw

PATCH 21/21 ntb_netdev: Update Version

In reply to Jon Mason

Update NTB netdev version to 0.7

Signed-off-by Jon Mason <[email protected]>

---
 drivers/net/ntb_netdev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ntb_netdev.c +1 -1

index 02213a8..6ea1a7b 100644
@@ -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);

Signature · 1 line

1.7.9.5

Greg KH

2013-01-20 23:48 UTC #40 lore raw

In reply to Jon Mason

Quoted 1 line · Jon Mason

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