NTB: Bug Fixes
Subsystems (3)
v2 cover NTB: Bug Fixes
I split the v1 series into two series. These patches are bug fixes.
These are in github.com/allenbh/linux.git branch ntb/master.
Allen Hubbe (4):
NTB: Fix ntb_transport out-of-order RX update
NTB: Schedule to receive on QP link up
NTB: Fix zero size or integer overflow in ntb_set_mw
NTB: Fix dereference before check
Dave Jiang (3):
NTB: Fix transport stats for multiple devices
NTB: ntb_netdev not covering all receive errors
NTB: Fix oops in debugfs when transport is half-up
drivers/net/ntb_netdev.c | 9 +- drivers/ntb/ntb_transport.c | 201 +++++++++++++++++++++++++++----------------- 2 files changed, 132 insertions(+), 78 deletions(-)
Signature · 1 line
2.5.0.rc1
PATCH v2 1/7 NTB: Fix ntb_transport out-of-order RX update
It was possible for a synchronous update of the RX index in the error case to get ahead of the asynchronous RX index update in the normal case. Change the RX processing to preserve an RX completion order.
There were two error cases. First, if a buffer is not present to receive data, there would be no queue entry to preserve the RX completion order. Instead of dropping the RX frame, leave the RX frame in the ring. Schedule RX processing when RX entries are enqueued, in case there are RX frames waiting in the ring to be received.
Second, if a buffer is too small to receive data, drop the frame in the ring, mark the RX entry as done, and indicate the error in the RX entry length. Check for a negative length in the receive callback in ntb_netdev, and count occurrences as rx_length_errors.
--- drivers/net/ntb_netdev.c | 7 ++ drivers/ntb/ntb_transport.c | 169 ++++++++++++++++++++++++++------------------ 2 files changed, 107 insertions(+), 69 deletions(-)
drivers/net/ntb_netdev.c +7 -0
@@ -102,6 +102,12 @@ static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,netdev_dbg (ndev,"%s: %d byte payload received\n" , __func__, len); +if (len <0 ) { + ndev->stats.rx_errors++; + ndev->stats.rx_length_errors++; + goto enqueue_again; + } +skb_put (skb, len); skb->protocol =eth_type_trans (skb, ndev); skb->ip_summed =CHECKSUM_NONE ;
@@ -121,6 +127,7 @@ static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,return ; } +enqueue_again: rc =ntb_transport_rx_enqueue (qp, skb, skb->data, ndev->mtu +ETH_HLEN );if (rc) {dev_kfree_skb (skb);
drivers/ntb/ntb_transport.c +100 -69
300 lines not on this page · raw message
Signature · 1 line
2.5.0.rc1
PATCH v2 2/7 NTB: Fix transport stats for multiple devices
In reply to Allen HubbeFrom: Dave Jiang <[email protected]>
Currently the debugfs does not have files for all NTB transport queue pairs. When there are multiple NTBs present in a system, the QP names of the last transport clobber the names of previously added transport QPs. Only the last added QPs can be observed via debugfs.
Create a directory per NTB transport to associate the QPs with that transport. Name the directory the same as the PCI device.
--- drivers/ntb/ntb_transport.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
drivers/ntb/ntb_transport.c +10 -2
@@ -212,6 +212,8 @@ struct ntb_transport_ctx {bool link_is_up;struct delayed_work link_work;struct work_struct link_cleanup; + +struct dentry *debugfs_node_dir; };enum {
@@ -945,12 +947,12 @@ static int ntb_transport_init_queue(struct ntb_transport_ctx *nt, qp->tx_max_frame =min (transport_mtu, tx_size /2 ); qp->tx_max_entry = tx_size / qp->tx_max_frame; -if (nt_debugfs_dir) { +if (nt->debugfs_node_dir) {char debugfs_name[4 ];snprintf (debugfs_name,4 ,"qp%d" , qp_num); qp->debugfs_dir =debugfs_create_dir (debugfs_name, - nt_debugfs_dir); + nt->debugfs_node_dir); qp->debugfs_stats = debugfs_create_file("stats", S_IRUSR, qp->debugfs_dir, qp,
@@ -1053,6 +1055,12 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev) goto err2; } +if (nt_debugfs_dir) { + nt->debugfs_node_dir = +debugfs_create_dir (pci_name (ndev->pdev), + nt_debugfs_dir); + } +for (i =0 ; i < qp_count; i++) { rc =ntb_transport_init_queue (nt, i);if (rc)
Signature · 1 line
2.5.0.rc1
PATCH v2 3/7 NTB: ntb_netdev not covering all receive errors
In reply to Allen HubbeFrom: Dave Jiang <[email protected]>
ntb_netdev is allowing the link to come up even when -ENOMEM is returned from ntb_transport_rx_enqueue. Fix to cover all possible errors.
--- drivers/net/ntb_netdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ntb_netdev.c +1 -1
@@ -191,7 +191,7 @@ static int ntb_netdev_open(struct net_device *ndev) rc =ntb_transport_rx_enqueue (dev->qp, skb, skb->data, ndev->mtu +ETH_HLEN ); -if (rc == -EINVAL ) { +if (rc) {dev_kfree_skb (skb); goto err; }
Signature · 1 line
2.5.0.rc1
PATCH v2 8/14 NTB: Add list to MAINTAINERS
In reply to Allen HubbeFrom: Jon Mason <[email protected]>
Add the new NTB mailing list to MAINTAINERS
--- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+)
MAINTAINERS +2 -0
@@ -7223,6 +7223,7 @@ NTB DRIVER CORE M: Jon Mason <[email protected]> M: Dave Jiang <[email protected]> M: Allen Hubbe <[email protected]> +L: [email protected] S: Supported W: https://github.com/jonmason/ntb/wiki T: git git://github.com/jonmason/ntb.git
@@ -7234,6 +7235,7 @@ F: include/linux/ntb_transport.h NTB INTEL DRIVER M: Jon Mason <[email protected]> M: Dave Jiang <[email protected]> +L: [email protected] S: Supported W: https://github.com/jonmason/ntb/wiki T: git git://github.com/jonmason/ntb.git
Signature · 1 line
2.5.0.rc1
PATCH v2 9/14 NTB: Add flow control to the ntb_netdev
In reply to Allen HubbeFrom: Dave Jiang <[email protected]>
Right now if we push the NTB really hard, we start dropping packets due to not able to process the packets fast enough. We need to st:qop the upper layer from flooding us when that happens.
A timer is necessary in order to restart the queue once the resource has been processed on the receive side. Due to the way NTB is setup, the resources on the tx side are tied to the processing of the rx side and there's no async way to know when the rx side has released those resources.
--- drivers/net/ntb_netdev.c | 77 +++++++++++++++++++++++++++++++++++++++++++ drivers/ntb/ntb_transport.c | 18 +++++++++- include/linux/ntb_transport.h | 1 + 3 files changed, 95 insertions(+), 1 deletion(-)
drivers/net/ntb_netdev.c +77 -0
@@ -61,11 +61,21 @@ MODULE_VERSION(NTB_NETDEV_VER);MODULE_LICENSE ("Dual BSD/GPL" );MODULE_AUTHOR ("Intel Corporation" ); +/* Time in usecs for tx resource reaper */ +static unsigned int tx_time =1 ; + +/* Number of descriptors to free before resuming tx */ +static unsigned int tx_start =10 ; + +/* Number of descriptors still available before stop upper layer tx */ +static unsigned int tx_stop =5 ; +struct ntb_netdev {struct list_head list;struct pci_dev *pdev;struct net_device *ndev;struct ntb_transport_qp *qp; +struct timer_list tx_timer; };#define NTB_TX_TIMEOUT_MS 1000
@@ -136,11 +146,42 @@ enqueue_again: } } +static int __ntb_netdev_maybe_stop_tx (struct net_device *netdev, +struct ntb_transport_qp *qp,int size) +{ +struct ntb_netdev *dev =netdev_priv (netdev); + +netif_stop_queue (netdev); +/* Make sure to see the latest value of ntb_transport_tx_free_entry() +* since the queue was last started. +*/ +smp_mb (); + +if (likely (ntb_transport_tx_free_entry (qp) < size)) { +mod_timer (&dev->tx_timer, jiffies +usecs_to_jiffies (tx_time)); +return -EBUSY ; + } + +netif_start_queue (netdev); +return 0 ; +} + +static int ntb_netdev_maybe_stop_tx (struct net_device *ndev, +struct ntb_transport_qp *qp,int size) +{ +if (netif_queue_stopped (ndev) || + (ntb_transport_tx_free_entry (qp) >= size)) +return 0 ; + +return __ntb_netdev_maybe_stop_tx (ndev, qp, size); +} +static void ntb_netdev_tx_handler (struct ntb_transport_qp *qp,void *qp_data,void *data,int len) {struct net_device *ndev = qp_data;struct sk_buff *skb; +struct ntb_netdev *dev =netdev_priv (ndev); skb = data;if (!skb || !ndev)
@@ -155,6 +196,15 @@ static void ntb_netdev_tx_handler(struct ntb_transport_qp *qp, void *qp_data, }dev_kfree_skb (skb); + +if (ntb_transport_tx_free_entry (dev->qp) >= tx_start) { +/* Make sure anybody stopping the queue after this sees the new +* value of ntb_transport_tx_free_entry() +*/ +smp_mb (); +if (netif_queue_stopped (ndev)) +netif_wake_queue (ndev); + } } static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
@@ -163,10 +213,15 @@ static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,struct ntb_netdev *dev =netdev_priv (ndev);int rc; +ntb_netdev_maybe_stop_tx (ndev, dev->qp, tx_stop); + rc =ntb_transport_tx_enqueue (dev->qp, skb, skb->data, skb->len);if (rc) goto err; +/* check for next submit */ +ntb_netdev_maybe_stop_tx (ndev, dev->qp, tx_stop); +return NETDEV_TX_OK ; err:
@@ -175,6 +230,23 @@ err:return NETDEV_TX_BUSY ; } +static void ntb_netdev_tx_timer (unsigned long data) +{ +struct net_device *ndev = (struct net_device *)data; +struct ntb_netdev *dev =netdev_priv (ndev); + +if (ntb_transport_tx_free_entry (dev->qp) < tx_stop) { +mod_timer (&dev->tx_timer, jiffies +msecs_to_jiffies (tx_time)); + }else { +/* Make sure anybody stopping the queue after this sees the new +* value of ntb_transport_tx_free_entry() +*/ +smp_mb (); +if (netif_queue_stopped (ndev)) +netif_wake_queue (ndev); + } +} +static int ntb_netdev_open (struct net_device *ndev) {struct ntb_netdev *dev =netdev_priv (ndev);
@@ -197,8 +269,11 @@ static int ntb_netdev_open(struct net_device *ndev) } } +setup_timer (&dev->tx_timer, ntb_netdev_tx_timer, (unsigned long )ndev); +netif_carrier_off (ndev);ntb_transport_link_up (dev->qp); +netif_start_queue (ndev);return 0 ;
@@ -219,6 +294,8 @@ static int ntb_netdev_close(struct net_device *ndev)while ((skb =ntb_transport_rx_remove (dev->qp, &len)))dev_kfree_skb (skb); +del_timer_sync (&dev->tx_timer); +return 0 ; }
drivers/ntb/ntb_transport.c +17 -1
@@ -494,6 +494,12 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count, "tx_index - \t%u\n", qp->tx_index); out_offset +=snprintf (buf + out_offset, out_count - out_offset,"tx_max_entry - \t%u\n" , qp->tx_max_entry); + out_offset +=snprintf (buf + out_offset, out_count - out_offset, +"qp->remote_rx_info->entry - \t%u\n" , + qp->remote_rx_info->entry); + out_offset +=snprintf (buf + out_offset, out_count - out_offset, +"free tx - \t%u\n" , +ntb_transport_tx_free_entry (qp)); out_offset += snprintf(buf + out_offset, out_count - out_offset, "\nQP Link %s\n",
@@ -535,6 +541,7 @@ static struct ntb_queue_entry *ntb_list_rm(spinlock_t *lock, } entry = list_first_entry(list, struct ntb_queue_entry, entry);list_del (&entry->entry); + out:spin_unlock_irqrestore (lock, flags);
@@ -1843,7 +1850,7 @@ int ntb_transport_tx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data, entry =ntb_list_rm (&qp->ntb_tx_free_q_lock, &qp->tx_free_q);if (!entry) { qp->tx_err_no_buf++; -return -ENOMEM ; +return -EBUSY ; } entry->cb_data = cb;
@@ -1969,6 +1976,15 @@ unsigned int ntb_transport_max_size(struct ntb_transport_qp *qp) }EXPORT_SYMBOL_GPL (ntb_transport_max_size); +unsigned int ntb_transport_tx_free_entry (struct ntb_transport_qp *qp) +{ +unsigned int head = qp->tx_index; +unsigned int tail = qp->remote_rx_info->entry; + +return tail > head ? tail - head : qp->tx_max_entry + tail - head; +} +EXPORT_SYMBOL_GPL (ntb_transport_tx_free_entry); +static void ntb_transport_doorbell_callback (void *data,int vector) {struct ntb_transport_ctx *nt = data;
include/linux/ntb_transport.h +1 -0
@@ -83,3 +83,4 @@ void *ntb_transport_rx_remove(struct ntb_transport_qp *qp, unsigned int *len);void ntb_transport_link_up (struct ntb_transport_qp *qp);void ntb_transport_link_down (struct ntb_transport_qp *qp);bool ntb_transport_link_query (struct ntb_transport_qp *qp); +unsigned int ntb_transport_tx_free_entry (struct ntb_transport_qp *qp);
Signature · 1 line
2.5.0.rc1
PATCH v2 10/14 NTB: Add PCI Device IDs for Broadwell Xeon
In reply to Allen HubbeFrom: Dave Jiang <[email protected]>
Adding PCI Device IDs for B2B (back to back), RP (root port, primary), and TB (transparent bridge, secondary) devices.
--- drivers/ntb/hw/intel/ntb_hw_intel.c | 15 +++++++++++++++ drivers/ntb/hw/intel/ntb_hw_intel.h | 3 +++ 2 files changed, 18 insertions(+)
drivers/ntb/hw/intel/ntb_hw_intel.c +15 -0
@@ -190,14 +190,17 @@ static inline int pdev_is_xeon(struct pci_dev *pdev)case PCI_DEVICE_ID_INTEL_NTB_SS_SNB :case PCI_DEVICE_ID_INTEL_NTB_SS_IVT :case PCI_DEVICE_ID_INTEL_NTB_SS_HSX : +case PCI_DEVICE_ID_INTEL_NTB_SS_BDX :case PCI_DEVICE_ID_INTEL_NTB_PS_JSF :case PCI_DEVICE_ID_INTEL_NTB_PS_SNB :case PCI_DEVICE_ID_INTEL_NTB_PS_IVT :case PCI_DEVICE_ID_INTEL_NTB_PS_HSX : +case PCI_DEVICE_ID_INTEL_NTB_PS_BDX :case PCI_DEVICE_ID_INTEL_NTB_B2B_JSF :case PCI_DEVICE_ID_INTEL_NTB_B2B_SNB :case PCI_DEVICE_ID_INTEL_NTB_B2B_IVT :case PCI_DEVICE_ID_INTEL_NTB_B2B_HSX : +case PCI_DEVICE_ID_INTEL_NTB_B2B_BDX :return 1 ; }return 0 ;
@@ -1843,6 +1846,9 @@ static int xeon_init_dev(struct intel_ntb_dev *ndev)case PCI_DEVICE_ID_INTEL_NTB_SS_HSX :case PCI_DEVICE_ID_INTEL_NTB_PS_HSX :case PCI_DEVICE_ID_INTEL_NTB_B2B_HSX : +case PCI_DEVICE_ID_INTEL_NTB_SS_BDX : +case PCI_DEVICE_ID_INTEL_NTB_PS_BDX : +case PCI_DEVICE_ID_INTEL_NTB_B2B_BDX : ndev->hwerr_flags |=NTB_HWERR_SDOORBELL_LOCKUP ;break ; }
@@ -1857,6 +1863,9 @@ static int xeon_init_dev(struct intel_ntb_dev *ndev)case PCI_DEVICE_ID_INTEL_NTB_SS_HSX :case PCI_DEVICE_ID_INTEL_NTB_PS_HSX :case PCI_DEVICE_ID_INTEL_NTB_B2B_HSX : +case PCI_DEVICE_ID_INTEL_NTB_SS_BDX : +case PCI_DEVICE_ID_INTEL_NTB_PS_BDX : +case PCI_DEVICE_ID_INTEL_NTB_B2B_BDX : ndev->hwerr_flags |=NTB_HWERR_SB01BASE_LOCKUP ;break ; }
@@ -1878,6 +1887,9 @@ static int xeon_init_dev(struct intel_ntb_dev *ndev)case PCI_DEVICE_ID_INTEL_NTB_SS_HSX :case PCI_DEVICE_ID_INTEL_NTB_PS_HSX :case PCI_DEVICE_ID_INTEL_NTB_B2B_HSX : +case PCI_DEVICE_ID_INTEL_NTB_SS_BDX : +case PCI_DEVICE_ID_INTEL_NTB_PS_BDX : +case PCI_DEVICE_ID_INTEL_NTB_B2B_BDX : ndev->hwerr_flags |=NTB_HWERR_B2BDOORBELL_BIT14 ;break ; }
@@ -2234,14 +2246,17 @@ static const struct pci_device_id intel_ntb_pci_tbl[] = { {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_SNB)}, {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_IVT)}, {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_HSX)}, + {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_BDX)}, {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_PS_JSF)}, {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_PS_SNB)}, {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_PS_IVT)}, {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_PS_HSX)}, + {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_PS_BDX)}, {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_SS_JSF)}, {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_SS_SNB)}, {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_SS_IVT)}, {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_SS_HSX)}, + {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_SS_BDX)}, {0} };MODULE_DEVICE_TABLE (pci, intel_ntb_pci_tbl);
drivers/ntb/hw/intel/ntb_hw_intel.h +3 -0
@@ -67,6 +67,9 @@#define PCI_DEVICE_ID_INTEL_NTB_PS_HSX 0x2F0E#define PCI_DEVICE_ID_INTEL_NTB_SS_HSX 0x2F0F#define PCI_DEVICE_ID_INTEL_NTB_B2B_BWD 0x0C4E +#define PCI_DEVICE_ID_INTEL_NTB_B2B_BDX 0x6F0D +#define PCI_DEVICE_ID_INTEL_NTB_PS_BDX 0x6F0E +#define PCI_DEVICE_ID_INTEL_NTB_SS_BDX 0x6F0F/* Intel Xeon hardware */
Signature · 1 line
2.5.0.rc1
PATCH v2 11/14 NTB: Make the transport list in order of discovery
In reply to Allen HubbeFrom: Dave Jiang <[email protected]>
The list should be added from the bottom and not the top in order to ensure the transport is provided in the same order to clients as ntb devices are discovered.
--- drivers/ntb/ntb_transport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/ntb/ntb_transport.c +1 -1
@@ -297,7 +297,7 @@ static LIST_HEAD(ntb_transport_list);static int ntb_bus_init (struct ntb_transport_ctx *nt) { -list_add (&nt->entry, &ntb_transport_list); +list_add_tail (&nt->entry, &ntb_transport_list);return 0 ; }
Signature · 1 line
2.5.0.rc1
PATCH v2 4/7 NTB: Fix oops in debugfs when transport is half-up
In reply to Allen HubbeFrom: Dave Jiang <[email protected]>
When the remote side is not up, we do not have all the context for the transport, and that causes NULL ptr access. Have the debugfs reads check to see if transport is up before we make access.
--- drivers/ntb/ntb_transport.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/ntb/ntb_transport.c +5 -1
@@ -439,13 +439,17 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,char *buf;ssize_t ret, out_offset, out_count; + qp = filp->private_data; + +if (!qp || !qp->link_is_up) +return 0 ; + out_count =1000 ; buf =kmalloc (out_count,GFP_KERNEL );if (!buf)return -ENOMEM ; - qp = filp->private_data; out_offset =0 ; out_offset +=snprintf (buf + out_offset, out_count - out_offset,"NTB QP stats\n" );
Signature · 1 line
2.5.0.rc1
PATCH v2 5/7 NTB: Schedule to receive on QP link up
In reply to Allen HubbeSchedule to receive on QP link up, to make sure that the doorbell is properly cleared for interrupts.
--- drivers/ntb/ntb_transport.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/ntb/ntb_transport.c +2 -0
@@ -895,6 +895,8 @@ static void ntb_qp_link_work(struct work_struct *work)if (qp->event_handler) qp->event_handler (qp->cb_data, qp->link_is_up); + +tasklet_schedule (&qp->rxc_db_work); }else if (nt->link_is_up)schedule_delayed_work (&qp->link_work,msecs_to_jiffies (NTB_LINK_DOWN_TIMEOUT ));
Signature · 1 line
2.5.0.rc1
PATCH v2 12/14 NTB: Clean up QP stats info
In reply to Allen HubbeFrom: Dave Jiang <[email protected]>
Make QP stats info more readable for debugging purposes. Also add an entry to indicate whether DMA is being used.
--- drivers/ntb/ntb_transport.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)
drivers/ntb/ntb_transport.c +16 -9
@@ -452,7 +452,7 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count, out_offset =0 ; out_offset +=snprintf (buf + out_offset, out_count - out_offset, -"NTB QP stats\n" ); +"\nNTB QP stats:\n\n" ); out_offset +=snprintf (buf + out_offset, out_count - out_offset,"rx_bytes - \t%llu\n" , qp->rx_bytes); out_offset += snprintf(buf + out_offset, out_count - out_offset,
@@ -470,11 +470,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 - \t%p\n" , qp->rx_buff); +"rx_buff - \t0x%p\n" , qp->rx_buff); out_offset +=snprintf (buf + out_offset, out_count - out_offset,"rx_index - \t%u\n" , qp->rx_index); out_offset +=snprintf (buf + out_offset, out_count - out_offset, -"rx_max_entry - \t%u\n" , qp->rx_max_entry); +"rx_max_entry - \t%u\n\n" , qp->rx_max_entry); out_offset +=snprintf (buf + out_offset, out_count - out_offset,"tx_bytes - \t%llu\n" , qp->tx_bytes);
@@ -489,21 +489,28 @@ 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_err_no_buf - %llu\n" , qp->tx_err_no_buf); out_offset +=snprintf (buf + out_offset, out_count - out_offset, -"tx_mw - \t%p\n" , qp->tx_mw); +"tx_mw - \t0x%p\n" , qp->tx_mw); out_offset +=snprintf (buf + out_offset, out_count - out_offset, -"tx_index - \t%u\n" , qp->tx_index); +"tx_index (H) - \t%u\n" , qp->tx_index); out_offset +=snprintf (buf + out_offset, out_count - out_offset, -"tx_max_entry - \t%u\n" , qp->tx_max_entry); - out_offset +=snprintf (buf + out_offset, out_count - out_offset, -"qp->remote_rx_info->entry - \t%u\n" , +"RRI (T) - \t%u\n" , qp->remote_rx_info->entry); out_offset +=snprintf (buf + out_offset, out_count - out_offset, +"tx_max_entry - \t%u\n" , qp->tx_max_entry); + out_offset +=snprintf (buf + out_offset, out_count - out_offset,"free tx - \t%u\n" ,ntb_transport_tx_free_entry (qp)); out_offset +=snprintf (buf + out_offset, out_count - out_offset, -"\nQP Link %s\n" , +"\n" ); + out_offset +=snprintf (buf + out_offset, out_count - out_offset, +"Using DMA - \t%s\n" , use_dma ?"Yes" :"No" ); + out_offset +=snprintf (buf + out_offset, out_count - out_offset, +"QP Link - \t%s\n" , qp->link_is_up ?"Up" :"Down" ); + out_offset +=snprintf (buf + out_offset, out_count - out_offset, +"\n" ); +if (out_offset > out_count) out_offset = out_count;
Signature · 1 line
2.5.0.rc1
PATCH v2 6/7 NTB: Fix zero size or integer overflow in ntb_set_mw
In reply to Allen HubbeA plain 32 bit integer will overflow for values over 4GiB.
Change the plain integer size to the appropriate size type in ntb_set_mw. Change the type of the size parameter and two local variables used for size.
Even if there is no overflow, a size of zero is invalid here.
--- drivers/ntb/ntb_transport.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/ntb/ntb_transport.c +6 -3
@@ -629,13 +629,16 @@ static void ntb_free_mw(struct ntb_transport_ctx *nt, int num_mw) }static int ntb_set_mw (struct ntb_transport_ctx *nt,int num_mw, -unsigned int size) +resource_size_t size) {struct ntb_transport_mw *mw = &nt->mw_vec[num_mw];struct pci_dev *pdev = nt->ndev->pdev; -unsigned int xlat_size, buff_size; +size_t xlat_size, buff_size;int rc; +if (!size) +return -EINVAL ; + xlat_size =round_up (size, mw->xlat_align_size); buff_size =round_up (size, mw->xlat_align);
@@ -655,7 +658,7 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw,if (!mw->virt_addr) { mw->xlat_size =0 ; mw->buff_size =0 ; -dev_err (&pdev->dev,"Unable to alloc MW buff of size %d\n" , +dev_err (&pdev->dev,"Unable to alloc MW buff of size %lu\n" , buff_size);return -ENOMEM ; }
Signature · 1 line
2.5.0.rc1
PATCH v2 13/14 NTB: Remove dma_sync_wait from ntb_async_rx
In reply to Allen HubbeThe dma_sync_wait can hurt the performance of workloads mixed with both large and small frames. Large frames will be copied using the dma engine. Small frames will be copied by the cpu. The dma_sync_wait prevents the cpu and dma engine copying in parallel.
In the period where the cpu is copying, the dma engine is stopped. The dma engine is not doing any useful work to copy large frames during that time, and the additional time to restart the dma engine for the next large frame. This will decrease the throughput for the portion of a workload with large frames.
In the period where the dma engine is copying, the cpu is held up waiting for dma to complete. The small frames processing will be delayed until the dma is complete. The RX frames are completed in-order, and the processing of small frames takes very little time, so dma_sync_wait may have an insignificant impact on the respose time of frames. The more significant impact is to the system, because the delay in dma_sync_wait is implemented as busy non-blocking wait. This can prevent the delayed core from doing any useful work, even if it could be processing work for other drivers, unrelated to transport RX processing.
After applying the earlier patch to fix out-of-order RX acknoledgement, the dma_sync_wait is no longer necessary. Remove it, so that cpu memcpy will proceed immediately for small frames, in parallel with ongoing dma for large frames. Do not hold up the cpu from doing work while dma is in progress. The prior fix will continue to ensure in-order completion of the RX frames to the upper layer, and in-order delivery of the RX acknoledgement.
--- drivers/ntb/ntb_transport.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
drivers/ntb/ntb_transport.c +3 -9
@@ -1233,18 +1233,18 @@ static void ntb_async_rx(struct ntb_queue_entry *entry, void *offset) goto err;if (len < copy_bytes) - goto err_wait; + goto err; device = chan->device; pay_off = (size_t )offset & ~PAGE_MASK ; buff_off = (size_t )buf & ~PAGE_MASK ;if (!is_dma_copy_aligned (device, pay_off, buff_off, len)) - goto err_wait; + goto err; unmap =dmaengine_get_unmap_data (device->dev,2 ,GFP_NOWAIT );if (!unmap) - goto err_wait; + goto err; unmap->len = len; unmap->addr[0] = dma_map_page(device->dev, virt_to_page(offset),
@@ -1287,12 +1287,6 @@ err_set_unmap:dmaengine_unmap_put (unmap); err_get_unmap:dmaengine_unmap_put (unmap); -err_wait: -/* If the callbacks come out of order, the writing of the index to the -* last completed will be out of order. This may result in the -* receive stalling forever. -*/ -dma_sync_wait (chan, qp->last_cookie); err:ntb_memcpy_rx (entry, offset); qp->rx_memcpy++;
Signature · 1 line
2.5.0.rc1
PATCH v2 7/7 NTB: Fix dereference before check
In reply to Allen HubbeRemove early dereference of a pointer that is checked later in the code.
--- drivers/ntb/ntb_transport.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/ntb/ntb_transport.c +1 -2
@@ -1692,7 +1692,6 @@ EXPORT_SYMBOL_GPL(ntb_transport_create_queue);*/ void ntb_transport_free_queue (struct ntb_transport_qp *qp) { -struct ntb_transport_ctx *nt = qp->transport;struct pci_dev *pdev;struct ntb_queue_entry *entry;u64 qp_bit;
@@ -1745,7 +1744,7 @@ void ntb_transport_free_queue(struct ntb_transport_qp *qp)while ((entry =ntb_list_rm (&qp->ntb_tx_free_q_lock, &qp->tx_free_q)))kfree (entry); - nt->qp_bitmap_free |= qp_bit; + qp->transport->qp_bitmap_free |= qp_bit;dev_info (&pdev->dev,"NTB Transport QP %d freed\n" , qp->qp_num); }
Signature · 1 line
2.5.0.rc1
v2 cover NTB: Performance and Usability Features
In reply to Allen HubbeI split the v1 series into two series. These patches are _not_ bug fixes.
Numbering in this v2 starts at 8. This is the second half of the patches.
Module parameters are removed from "Add flow control".
These are in github.com/allenbh/linux.git branch ntb/next.
Allen Hubbe (1):
NTB: Remove dma_sync_wait from ntb_async_rx
Dave Jiang (5):
NTB: Add flow control to the ntb_netdev
NTB: Add PCI Device IDs for Broadwell Xeon
NTB: Make the transport list in order of discovery
NTB: Clean up QP stats info
NTB: Use unique DMA channels for TX and RX
Jon Mason (1):
NTB: Add list to MAINTAINERS
MAINTAINERS | 2 + drivers/net/ntb_netdev.c | 77 ++++++++++++++++++++++ drivers/ntb/hw/intel/ntb_hw_intel.c | 15 +++++ drivers/ntb/hw/intel/ntb_hw_intel.h | 3 + drivers/ntb/ntb_transport.c | 126 ++++++++++++++++++++++++++---------- include/linux/ntb_transport.h | 1 + 6 files changed, 189 insertions(+), 35 deletions(-)
Signature · 1 line
2.5.0.rc1
PATCH v2 14/14 NTB: Use unique DMA channels for TX and RX
In reply to Allen HubbeFrom: Dave Jiang <[email protected]>
Allocate two DMA channels, one for TX operation and one for RX operation, instead of having one DMA channel for everything. This provides slightly better performance, and also will make error handling cleaner later on.
--- drivers/ntb/ntb_transport.c | 77 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 58 insertions(+), 19 deletions(-)
drivers/ntb/ntb_transport.c +58 -19
@@ -119,7 +119,8 @@ struct ntb_transport_qp {struct ntb_transport_ctx *transport;struct ntb_dev *ndev;void *cb_data; -struct dma_chan *dma_chan; +struct dma_chan *tx_dma_chan; +struct dma_chan *rx_dma_chan;bool client_ready;bool link_is_up;
@@ -504,7 +505,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,"\n" ); out_offset +=snprintf (buf + out_offset, out_count - out_offset, -"Using DMA - \t%s\n" , use_dma ?"Yes" :"No" ); +"Using TX DMA - \t%s\n" , + qp->tx_dma_chan ?"Yes" :"No" ); + out_offset +=snprintf (buf + out_offset, out_count - out_offset, +"Using RX DMA - \t%s\n" , + qp->rx_dma_chan ?"Yes" :"No" ); out_offset +=snprintf (buf + out_offset, out_count - out_offset,"QP Link - \t%s\n" , qp->link_is_up ?"Up" :"Down" );
@@ -1220,7 +1225,7 @@ static void ntb_async_rx(struct ntb_queue_entry *entry, void *offset) {struct dma_async_tx_descriptor *txd;struct ntb_transport_qp *qp = entry->qp; -struct dma_chan *chan = qp->dma_chan; +struct dma_chan *chan = qp->rx_dma_chan;struct dma_device *device;size_t pay_off, buff_off, len;struct dmaengine_unmap_data *unmap;
@@ -1381,8 +1386,8 @@ static void ntb_transport_rxc_db(unsigned long data)break ; } -if (i && qp->dma_chan) -dma_async_issue_pending (qp->dma_chan); +if (i && qp->rx_dma_chan) +dma_async_issue_pending (qp->rx_dma_chan);if (i == qp->rx_max_entry) {/* there is more work to do */
@@ -1449,7 +1454,7 @@ static void ntb_async_tx(struct ntb_transport_qp *qp, {struct ntb_payload_header __iomem *hdr;struct dma_async_tx_descriptor *txd; -struct dma_chan *chan = qp->dma_chan; +struct dma_chan *chan = qp->tx_dma_chan;struct dma_device *device;size_t dest_off, buff_off;struct dmaengine_unmap_data *unmap;
@@ -1642,14 +1647,27 @@ ntb_transport_create_queue(void *data, struct device *client_dev,dma_cap_set (DMA_MEMCPY , dma_mask);if (use_dma) { - qp->dma_chan =dma_request_channel (dma_mask, ntb_dma_filter_fn, - (void *)(unsigned long )node); -if (!qp->dma_chan) -dev_info (&pdev->dev,"Unable to allocate DMA channel\n" ); + qp->tx_dma_chan = +dma_request_channel (dma_mask, ntb_dma_filter_fn, + (void *)(unsigned long )node); +if (!qp->tx_dma_chan) +dev_info (&pdev->dev,"Unable to allocate TX DMA channel\n" ); + + qp->rx_dma_chan = +dma_request_channel (dma_mask, ntb_dma_filter_fn, + (void *)(unsigned long )node); +if (!qp->rx_dma_chan) +dev_info (&pdev->dev,"Unable to allocate RX DMA channel\n" ); }else { - qp->dma_chan =NULL ; + qp->tx_dma_chan =NULL ; + qp->rx_dma_chan =NULL ; } -dev_dbg (&pdev->dev,"Using %s memcpy\n" , qp->dma_chan ?"DMA" :"CPU" ); + +dev_dbg (&pdev->dev,"Using %s memcpy for TX\n" , + qp->tx_dma_chan ?"DMA" :"CPU" ); + +dev_dbg (&pdev->dev,"Using %s memcpy for RX\n" , + qp->rx_dma_chan ?"DMA" :"CPU" );for (i =0 ; i <NTB_QP_DEF_NUM_ENTRIES ; i++) { entry =kzalloc_node (sizeof (*entry),GFP_ATOMIC , node);
@@ -1684,8 +1702,10 @@ err2: err1:while ((entry =ntb_list_rm (&qp->ntb_rx_q_lock, &qp->rx_free_q)))kfree (entry); -if (qp->dma_chan) -dma_release_channel (qp->dma_chan); +if (qp->tx_dma_chan) +dma_release_channel (qp->tx_dma_chan); +if (qp->rx_dma_chan) +dma_release_channel (qp->rx_dma_chan); nt->qp_bitmap_free |= qp_bit; err:return NULL ;
@@ -1709,12 +1729,27 @@ void ntb_transport_free_queue(struct ntb_transport_qp *qp) pdev = qp->ndev->pdev; -if (qp->dma_chan) { -struct dma_chan *chan = qp->dma_chan; +if (qp->tx_dma_chan) { +struct dma_chan *chan = qp->tx_dma_chan; +/* Putting the dma_chan to NULL will force any new traffic to be +* processed by the CPU instead of the DAM engine +*/ + qp->tx_dma_chan =NULL ; + +/* Try to be nice and wait for any queued DMA engine +* transactions to process before smashing it with a rock +*/ +dma_sync_wait (chan, qp->last_cookie); +dmaengine_terminate_all (chan); +dma_release_channel (chan); + } + +if (qp->rx_dma_chan) { +struct dma_chan *chan = qp->rx_dma_chan;/* Putting the dma_chan to NULL will force any new traffic to be * processed by the CPU instead of the DAM engine */ - qp->dma_chan =NULL ; + qp->rx_dma_chan =NULL ;/* Try to be nice and wait for any queued DMA engine * transactions to process before smashing it with a rock
@@ -1962,16 +1997,20 @@ EXPORT_SYMBOL_GPL(ntb_transport_qp_num);unsigned int ntb_transport_max_size (struct ntb_transport_qp *qp) {unsigned int max; +unsigned int copy_align;if (!qp)return 0 ; -if (!qp->dma_chan) +if (!qp->tx_dma_chan && !qp->rx_dma_chan)return qp->tx_max_frame -sizeof (struct ntb_payload_header ); + copy_align =max (qp->tx_dma_chan->device->copy_align, + qp->rx_dma_chan->device->copy_align); +/* If DMA engine usage is possible, try to find the max size for that */ max = qp->tx_max_frame -sizeof (struct ntb_payload_header ); - max -= max % (1 << qp->dma_chan->device->copy_align); + max -= max % (1 << copy_align);return max; }
Signature · 1 line
2.5.0.rc1