Netdev List
 help / color / mirror / Atom feed
* [PATCH v13 15/16] An example how to modifiy NIC driver to use napi_gro_frags() interface
From: xiaohui.xin @ 2010-10-15  9:12 UTC (permalink / raw)
  To: netdev, kvm, linux-kernel, mst, mingo, davem, herbert, jdike; +Cc: Xin Xiaohui
In-Reply-To: <fc6e95d63a2c62aaf77f8ded22fc43ccefcdbbff.1287132437.git.xiaohui.xin@intel.com>

From: Xin Xiaohui <xiaohui.xin@intel.com>

This example is made on ixgbe driver.
It provides API is_rx_buffer_mapped_as_page() to indicate
if the driver use napi_gro_frags() interface or not.
The example allocates 2 pages for DMA for one ring descriptor
using netdev_alloc_page(). When packets is coming, using
napi_gro_frags() to allocate skb and to receive the packets.

Signed-off-by: Xin Xiaohui <xiaohui.xin@intel.com>
---
 drivers/net/ixgbe/ixgbe.h      |    3 +
 drivers/net/ixgbe/ixgbe_main.c |  163 +++++++++++++++++++++++++++++++---------
 2 files changed, 131 insertions(+), 35 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe.h b/drivers/net/ixgbe/ixgbe.h
index 9e15eb9..89367ca 100644
--- a/drivers/net/ixgbe/ixgbe.h
+++ b/drivers/net/ixgbe/ixgbe.h
@@ -131,6 +131,9 @@ struct ixgbe_rx_buffer {
 	struct page *page;
 	dma_addr_t page_dma;
 	unsigned int page_offset;
+	u16 mapped_as_page;
+	struct page *page_skb;
+	unsigned int page_skb_offset;
 };
 
 struct ixgbe_queue_stats {
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index e32af43..a4a5263 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -1029,6 +1029,12 @@ static inline void ixgbe_release_rx_desc(struct ixgbe_hw *hw,
 	IXGBE_WRITE_REG(hw, IXGBE_RDT(rx_ring->reg_idx), val);
 }
 
+static bool is_rx_buffer_mapped_as_page(struct ixgbe_rx_buffer *bi,
+					struct net_device *dev)
+{
+	return true;
+}
+
 /**
  * ixgbe_alloc_rx_buffers - Replace used receive buffers; packet split
  * @adapter: address of board private structure
@@ -1045,13 +1051,17 @@ static void ixgbe_alloc_rx_buffers(struct ixgbe_adapter *adapter,
 	i = rx_ring->next_to_use;
 	bi = &rx_ring->rx_buffer_info[i];
 
+
 	while (cleaned_count--) {
 		rx_desc = IXGBE_RX_DESC_ADV(*rx_ring, i);
 
+		bi->mapped_as_page =
+			is_rx_buffer_mapped_as_page(bi, adapter->netdev);
+
 		if (!bi->page_dma &&
 		    (rx_ring->flags & IXGBE_RING_RX_PS_ENABLED)) {
 			if (!bi->page) {
-				bi->page = alloc_page(GFP_ATOMIC);
+				bi->page = netdev_alloc_page(adapter->netdev);
 				if (!bi->page) {
 					adapter->alloc_rx_page_failed++;
 					goto no_buffers;
@@ -1068,7 +1078,7 @@ static void ixgbe_alloc_rx_buffers(struct ixgbe_adapter *adapter,
 						    DMA_FROM_DEVICE);
 		}
 
-		if (!bi->skb) {
+		if (!bi->mapped_as_page && !bi->skb) {
 			struct sk_buff *skb;
 			/* netdev_alloc_skb reserves 32 bytes up front!! */
 			uint bufsz = rx_ring->rx_buf_len + SMP_CACHE_BYTES;
@@ -1088,6 +1098,19 @@ static void ixgbe_alloc_rx_buffers(struct ixgbe_adapter *adapter,
 			                         rx_ring->rx_buf_len,
 						 DMA_FROM_DEVICE);
 		}
+
+		if (bi->mapped_as_page && !bi->page_skb) {
+			bi->page_skb = netdev_alloc_page(adapter->netdev);
+			if (!bi->page_skb) {
+				adapter->alloc_rx_page_failed++;
+				goto no_buffers;
+			}
+			bi->page_skb_offset = 0;
+			bi->dma = dma_map_page(&pdev->dev, bi->page_skb,
+					bi->page_skb_offset,
+					(PAGE_SIZE / 2),
+					PCI_DMA_FROMDEVICE);
+		}
 		/* Refresh the desc even if buffer_addrs didn't change because
 		 * each write-back erases this info. */
 		if (rx_ring->flags & IXGBE_RING_RX_PS_ENABLED) {
@@ -1165,6 +1188,13 @@ struct ixgbe_rsc_cb {
 	bool delay_unmap;
 };
 
+static bool is_no_buffer(struct ixgbe_rx_buffer *rx_buffer_info)
+{
+	return (!rx_buffer_info->skb ||
+		!rx_buffer_info->page_skb) &&
+		!rx_buffer_info->page;
+}
+
 #define IXGBE_RSC_CB(skb) ((struct ixgbe_rsc_cb *)(skb)->cb)
 
 static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
@@ -1174,6 +1204,7 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 	struct ixgbe_adapter *adapter = q_vector->adapter;
 	struct net_device *netdev = adapter->netdev;
 	struct pci_dev *pdev = adapter->pdev;
+	struct napi_struct *napi = &q_vector->napi;
 	union ixgbe_adv_rx_desc *rx_desc, *next_rxd;
 	struct ixgbe_rx_buffer *rx_buffer_info, *next_buffer;
 	struct sk_buff *skb;
@@ -1211,32 +1242,68 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 			len = le16_to_cpu(rx_desc->wb.upper.length);
 		}
 
+		if (is_no_buffer(rx_buffer_info))
+			break;
 		cleaned = true;
-		skb = rx_buffer_info->skb;
-		prefetch(skb->data);
-		rx_buffer_info->skb = NULL;
 
-		if (rx_buffer_info->dma) {
-			if ((adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) &&
-			    (!(staterr & IXGBE_RXD_STAT_EOP)) &&
-				 (!(skb->prev))) {
-				/*
-				 * When HWRSC is enabled, delay unmapping
-				 * of the first packet. It carries the
-				 * header information, HW may still
-				 * access the header after the writeback.
-				 * Only unmap it when EOP is reached
-				 */
-				IXGBE_RSC_CB(skb)->delay_unmap = true;
-				IXGBE_RSC_CB(skb)->dma = rx_buffer_info->dma;
-			} else {
-				dma_unmap_single(&pdev->dev,
-				                 rx_buffer_info->dma,
-				                 rx_ring->rx_buf_len,
-				                 DMA_FROM_DEVICE);
+		if (!rx_buffer_info->mapped_as_page) {
+			skb = rx_buffer_info->skb;
+			prefetch(skb->data);
+			rx_buffer_info->skb = NULL;
+
+			if (rx_buffer_info->dma) {
+				if ((adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) &&
+						(!(staterr & IXGBE_RXD_STAT_EOP)) &&
+						(!(skb->prev))) {
+					/*
+					 * When HWRSC is enabled, delay unmapping
+					 * of the first packet. It carries the
+					 * header information, HW may still
+					 * access the header after the writeback.
+					 * Only unmap it when EOP is reached
+					 */
+					IXGBE_RSC_CB(skb)->delay_unmap = true;
+					IXGBE_RSC_CB(skb)->dma = rx_buffer_info->dma;
+				} else
+					dma_unmap_single(&pdev->dev,
+							rx_buffer_info->dma,
+							rx_ring->rx_buf_len,
+							DMA_FROM_DEVICE);
+				rx_buffer_info->dma = 0;
+				skb_put(skb, len);
+			}
+		} else {
+			skb = napi_get_frags(napi);
+			prefetch(rx_buffer_info->page_skb_offset);
+			rx_buffer_info->skb = NULL;
+			if (rx_buffer_info->dma) {
+				if ((adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) &&
+						(!(staterr & IXGBE_RXD_STAT_EOP)) &&
+						(!(skb->prev))) {
+					/*
+					 * When HWRSC is enabled, delay unmapping
+					 * of the first packet. It carries the
+					 * header information, HW may still
+					 * access the header after the writeback.
+					 * Only unmap it when EOP is reached
+					 */
+					IXGBE_RSC_CB(skb)->delay_unmap = true;
+					IXGBE_RSC_CB(skb)->dma = rx_buffer_info->dma;
+				} else
+					dma_unmap_page(&pdev->dev, rx_buffer_info->dma,
+							PAGE_SIZE / 2,
+							PCI_DMA_FROMDEVICE);
+				rx_buffer_info->dma = 0;
+				skb_fill_page_desc(skb,
+						skb_shinfo(skb)->nr_frags,
+						rx_buffer_info->page_skb,
+						rx_buffer_info->page_skb_offset,
+						len);
+				rx_buffer_info->page_skb = NULL;
+				skb->len += len;
+				skb->data_len += len;
+				skb->truesize += len;
 			}
-			rx_buffer_info->dma = 0;
-			skb_put(skb, len);
 		}
 
 		if (upper_len) {
@@ -1283,10 +1350,16 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 				skb = ixgbe_transform_rsc_queue(skb, &(rx_ring->rsc_count));
 			if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) {
 				if (IXGBE_RSC_CB(skb)->delay_unmap) {
-					dma_unmap_single(&pdev->dev,
-							 IXGBE_RSC_CB(skb)->dma,
-					                 rx_ring->rx_buf_len,
-							 DMA_FROM_DEVICE);
+					if (!rx_buffer_info->mapped_as_page)
+						dma_unmap_single(&pdev->dev,
+								IXGBE_RSC_CB(skb)->dma,
+								rx_ring->rx_buf_len,
+								DMA_FROM_DEVICE);
+					else
+						dma_unmap_page(&pdev->dev,
+								IXGBE_RSC_CB(skb)->dma,
+								PAGE_SIZE / 2,
+								DMA_FROM_DEVICE);
 					IXGBE_RSC_CB(skb)->dma = 0;
 					IXGBE_RSC_CB(skb)->delay_unmap = false;
 				}
@@ -1304,6 +1377,11 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 				rx_buffer_info->dma = next_buffer->dma;
 				next_buffer->skb = skb;
 				next_buffer->dma = 0;
+				if (rx_buffer_info->mapped_as_page) {
+					rx_buffer_info->page_skb =
+							next_buffer->page_skb;
+					next_buffer->page_skb = NULL;
+				}
 			} else {
 				skb->next = next_buffer->skb;
 				skb->next->prev = skb;
@@ -1323,7 +1401,8 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 		total_rx_bytes += skb->len;
 		total_rx_packets++;
 
-		skb->protocol = eth_type_trans(skb, adapter->netdev);
+		if (!rx_buffer_info->mapped_as_page)
+			skb->protocol = eth_type_trans(skb, adapter->netdev);
 #ifdef IXGBE_FCOE
 		/* if ddp, not passing to ULD unless for FCP_RSP or error */
 		if (adapter->flags & IXGBE_FLAG_FCOE_ENABLED) {
@@ -1332,7 +1411,14 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 				goto next_desc;
 		}
 #endif /* IXGBE_FCOE */
-		ixgbe_receive_skb(q_vector, skb, staterr, rx_ring, rx_desc);
+
+		if (!rx_buffer_info->mapped_as_page)
+			ixgbe_receive_skb(q_vector, skb, staterr,
+						rx_ring, rx_desc);
+		else {
+			skb_record_rx_queue(skb, rx_ring->queue_index);
+			napi_gro_frags(napi);
+		}
 
 next_desc:
 		rx_desc->wb.upper.status_error = 0;
@@ -3622,9 +3708,16 @@ static void ixgbe_clean_rx_ring(struct ixgbe_adapter *adapter,
 
 		rx_buffer_info = &rx_ring->rx_buffer_info[i];
 		if (rx_buffer_info->dma) {
-			dma_unmap_single(&pdev->dev, rx_buffer_info->dma,
-			                 rx_ring->rx_buf_len,
-					 DMA_FROM_DEVICE);
+			if (!rx_buffer_info->mapped_as_page)
+				dma_unmap_single(&pdev->dev, rx_buffer_info->dma,
+						rx_ring->rx_buf_len,
+						PCI_DMA_FROMDEVICE);
+			else {
+				dma_unmap_page(&pdev->dev, rx_buffer_info->dma,
+						PAGE_SIZE / 2,
+						PCI_DMA_FROMDEVICE);
+				rx_buffer_info->page_skb = NULL;
+			}
 			rx_buffer_info->dma = 0;
 		}
 		if (rx_buffer_info->skb) {
@@ -3651,7 +3744,7 @@ static void ixgbe_clean_rx_ring(struct ixgbe_adapter *adapter,
 				       PAGE_SIZE / 2, DMA_FROM_DEVICE);
 			rx_buffer_info->page_dma = 0;
 		}
-		put_page(rx_buffer_info->page);
+		netdev_free_page(adapter->netdev, rx_buffer_info->page);
 		rx_buffer_info->page = NULL;
 		rx_buffer_info->page_offset = 0;
 	}
-- 
1.7.3

^ permalink raw reply related

* [PATCH v13 16/16] An example how to alloc user buffer based on napi_gro_frags() interface.
From: xiaohui.xin @ 2010-10-15  9:12 UTC (permalink / raw)
  To: netdev, kvm, linux-kernel, mst, mingo, davem, herbert, jdike; +Cc: Xin Xiaohui
In-Reply-To: <fc6e95d63a2c62aaf77f8ded22fc43ccefcdbbff.1287132437.git.xiaohui.xin@intel.com>

From: Xin Xiaohui <xiaohui.xin@intel.com>

This example is made on ixgbe driver which using napi_gro_frags().
It can get buffers from guest side directly using netdev_alloc_page()
and release guest buffers using netdev_free_page().

Signed-off-by: Xin Xiaohui <xiaohui.xin@intel.com>
---
 drivers/net/ixgbe/ixgbe_main.c |   24 ++++++++++++++++++++----
 1 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index a4a5263..47663ac 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -1032,7 +1032,14 @@ static inline void ixgbe_release_rx_desc(struct ixgbe_hw *hw,
 static bool is_rx_buffer_mapped_as_page(struct ixgbe_rx_buffer *bi,
 					struct net_device *dev)
 {
-	return true;
+	return dev_is_mpassthru(dev);
+}
+
+static u32 get_page_skb_offset(struct net_device *dev)
+{
+	if (!dev_is_mpassthru(dev))
+		return 0;
+	return dev->mp_port->vnet_hlen;
 }
 
 /**
@@ -1105,7 +1112,8 @@ static void ixgbe_alloc_rx_buffers(struct ixgbe_adapter *adapter,
 				adapter->alloc_rx_page_failed++;
 				goto no_buffers;
 			}
-			bi->page_skb_offset = 0;
+			bi->page_skb_offset =
+				get_page_skb_offset(adapter->netdev);
 			bi->dma = dma_map_page(&pdev->dev, bi->page_skb,
 					bi->page_skb_offset,
 					(PAGE_SIZE / 2),
@@ -1242,8 +1250,10 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 			len = le16_to_cpu(rx_desc->wb.upper.length);
 		}
 
-		if (is_no_buffer(rx_buffer_info))
+		if (is_no_buffer(rx_buffer_info)) {
+			printk("no buffers\n");
 			break;
+		}
 		cleaned = true;
 
 		if (!rx_buffer_info->mapped_as_page) {
@@ -1299,6 +1309,11 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 						rx_buffer_info->page_skb,
 						rx_buffer_info->page_skb_offset,
 						len);
+				if (dev_is_mpassthru(netdev) &&
+						netdev->mp_port->hash)
+					skb_shinfo(skb)->destructor_arg =
+						netdev->mp_port->hash(netdev,
+						rx_buffer_info->page_skb);
 				rx_buffer_info->page_skb = NULL;
 				skb->len += len;
 				skb->data_len += len;
@@ -1316,7 +1331,8 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 			                   upper_len);
 
 			if ((rx_ring->rx_buf_len > (PAGE_SIZE / 2)) ||
-			    (page_count(rx_buffer_info->page) != 1))
+			    (page_count(rx_buffer_info->page) != 1) ||
+				dev_is_mpassthru(netdev))
 				rx_buffer_info->page = NULL;
 			else
 				get_page(rx_buffer_info->page);
-- 
1.7.3

^ permalink raw reply related

* [PATCH 08/22] rds: stop including asm-generic/bitops/le.h
From: Akinobu Mita @ 2010-10-15  9:46 UTC (permalink / raw)
  To: linux-kernel, linux-arch, Arnd Bergmann, Christoph Hellwig,
	Andrew Morton
  Cc: Akinobu Mita, Andy Grover, rds-devel, David S. Miller, netdev
In-Reply-To: <1287135981-17604-1-git-send-email-akinobu.mita@gmail.com>

No need to include asm-generic/bitops/le.h as all architectures
provide little endian bit operations now.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Andy Grover <andy.grover@oracle.com>
Cc: rds-devel@oss.oracle.com
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
---
 net/rds/cong.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/net/rds/cong.c b/net/rds/cong.c
index c6784d5..15a65f0 100644
--- a/net/rds/cong.c
+++ b/net/rds/cong.c
@@ -34,8 +34,6 @@
 #include <linux/types.h>
 #include <linux/rbtree.h>
 
-#include <asm-generic/bitops/le.h>
-
 #include "rds.h"
 
 /*
-- 
1.7.1.231.gd0b16

^ permalink raw reply related

* [PATCH] connector: remove lazy workqueue creation
From: Tejun Heo @ 2010-10-15  9:55 UTC (permalink / raw)
  To: Evgeniy Polyakov, netdev@vger.kernel.org, Frederic Weisbecker,
	David S. Miller

Commit 1a5645bc (connector: create connector workqueue only while
needed once) implements lazy workqueue creation for connector
workqueue.  With cmwq now in place, lazy workqueue creation doesn't
make much sense while adding a lot of complexity.  Remove it and
allocate an ordered workqueue during initialization.

This also removes a call to flush_scheduled_work() which is deprecated
and scheduled to be removed.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
---
 drivers/connector/cn_queue.c  |   75 ++++--------------------------------------
 drivers/connector/connector.c |    9 ++---
 include/linux/connector.h     |    8 ----
 3 files changed, 12 insertions(+), 80 deletions(-)

Index: work/drivers/connector/cn_queue.c
===================================================================
--- work.orig/drivers/connector/cn_queue.c
+++ work/drivers/connector/cn_queue.c
@@ -31,48 +31,6 @@
 #include <linux/connector.h>
 #include <linux/delay.h>

-
-/*
- * This job is sent to the kevent workqueue.
- * While no event is once sent to any callback, the connector workqueue
- * is not created to avoid a useless waiting kernel task.
- * Once the first event is received, we create this dedicated workqueue which
- * is necessary because the flow of data can be high and we don't want
- * to encumber keventd with that.
- */
-static void cn_queue_create(struct work_struct *work)
-{
-	struct cn_queue_dev *dev;
-
-	dev = container_of(work, struct cn_queue_dev, wq_creation);
-
-	dev->cn_queue = create_singlethread_workqueue(dev->name);
-	/* If we fail, we will use keventd for all following connector jobs */
-	WARN_ON(!dev->cn_queue);
-}
-
-/*
- * Queue a data sent to a callback.
- * If the connector workqueue is already created, we queue the job on it.
- * Otherwise, we queue the job to kevent and queue the connector workqueue
- * creation too.
- */
-int queue_cn_work(struct cn_callback_entry *cbq, struct work_struct *work)
-{
-	struct cn_queue_dev *pdev = cbq->pdev;
-
-	if (likely(pdev->cn_queue))
-		return queue_work(pdev->cn_queue, work);
-
-	/* Don't create the connector workqueue twice */
-	if (atomic_inc_return(&pdev->wq_requested) == 1)
-		schedule_work(&pdev->wq_creation);
-	else
-		atomic_dec(&pdev->wq_requested);
-
-	return schedule_work(work);
-}
-
 void cn_queue_wrapper(struct work_struct *work)
 {
 	struct cn_callback_entry *cbq =
@@ -111,11 +69,7 @@ cn_queue_alloc_callback_entry(char *name

 static void cn_queue_free_callback(struct cn_callback_entry *cbq)
 {
-	/* The first jobs have been sent to kevent, flush them too */
-	flush_scheduled_work();
-	if (cbq->pdev->cn_queue)
-		flush_workqueue(cbq->pdev->cn_queue);
-
+	flush_workqueue(cbq->pdev->cn_queue);
 	kfree(cbq);
 }

@@ -193,11 +147,14 @@ struct cn_queue_dev *cn_queue_alloc_dev(
 	atomic_set(&dev->refcnt, 0);
 	INIT_LIST_HEAD(&dev->queue_list);
 	spin_lock_init(&dev->queue_lock);
-	init_waitqueue_head(&dev->wq_created);

 	dev->nls = nls;

-	INIT_WORK(&dev->wq_creation, cn_queue_create);
+	dev->cn_queue = alloc_ordered_workqueue(dev->name, 0);
+	if (!dev->cn_queue) {
+		kfree(dev);
+		return NULL;
+	}

 	return dev;
 }
@@ -205,25 +162,9 @@ struct cn_queue_dev *cn_queue_alloc_dev(
 void cn_queue_free_dev(struct cn_queue_dev *dev)
 {
 	struct cn_callback_entry *cbq, *n;
-	long timeout;
-	DEFINE_WAIT(wait);

-	/* Flush the first pending jobs queued on kevent */
-	flush_scheduled_work();
-
-	/* If the connector workqueue creation is still pending, wait for it */
-	prepare_to_wait(&dev->wq_created, &wait, TASK_UNINTERRUPTIBLE);
-	if (atomic_read(&dev->wq_requested) && !dev->cn_queue) {
-		timeout = schedule_timeout(HZ * 2);
-		if (!timeout && !dev->cn_queue)
-			WARN_ON(1);
-	}
-	finish_wait(&dev->wq_created, &wait);
-
-	if (dev->cn_queue) {
-		flush_workqueue(dev->cn_queue);
-		destroy_workqueue(dev->cn_queue);
-	}
+	flush_workqueue(dev->cn_queue);
+	destroy_workqueue(dev->cn_queue);

 	spin_lock_bh(&dev->queue_lock);
 	list_for_each_entry_safe(cbq, n, &dev->queue_list, callback_entry)
Index: work/drivers/connector/connector.c
===================================================================
--- work.orig/drivers/connector/connector.c
+++ work/drivers/connector/connector.c
@@ -133,7 +133,8 @@ static int cn_call_callback(struct sk_bu
 					__cbq->data.skb == NULL)) {
 				__cbq->data.skb = skb;

-				if (queue_cn_work(__cbq, &__cbq->work))
+				if (queue_work(dev->cbdev->cn_queue,
+					       &__cbq->work))
 					err = 0;
 				else
 					err = -EINVAL;
@@ -148,13 +149,11 @@ static int cn_call_callback(struct sk_bu
 					d->callback = __cbq->data.callback;
 					d->free = __new_cbq;

-					__new_cbq->pdev = __cbq->pdev;
-
 					INIT_WORK(&__new_cbq->work,
 							&cn_queue_wrapper);

-					if (queue_cn_work(__new_cbq,
-						    &__new_cbq->work))
+					if (queue_work(dev->cbdev->cn_queue,
+						       &__new_cbq->work))
 						err = 0;
 					else {
 						kfree(__new_cbq);
Index: work/include/linux/connector.h
===================================================================
--- work.orig/include/linux/connector.h
+++ work/include/linux/connector.h
@@ -88,12 +88,6 @@ struct cn_queue_dev {
 	unsigned char name[CN_CBQ_NAMELEN];

 	struct workqueue_struct *cn_queue;
-	/* Sent to kevent to create cn_queue only when needed */
-	struct work_struct wq_creation;
-	/* Tell if the wq_creation job is pending/completed */
-	atomic_t wq_requested;
-	/* Wait for cn_queue to be created */
-	wait_queue_head_t wq_created;

 	struct list_head queue_list;
 	spinlock_t queue_lock;
@@ -141,8 +135,6 @@ int cn_netlink_send(struct cn_msg *, u32
 int cn_queue_add_callback(struct cn_queue_dev *dev, char *name, struct cb_id *id, void (*callback)(struct cn_msg *, struct netlink_skb_parms *));
 void cn_queue_del_callback(struct cn_queue_dev *dev, struct cb_id *id);

-int queue_cn_work(struct cn_callback_entry *cbq, struct work_struct *work);
-
 struct cn_queue_dev *cn_queue_alloc_dev(char *name, struct sock *);
 void cn_queue_free_dev(struct cn_queue_dev *dev);


^ permalink raw reply

* (unknown), 
From: Marc Kleine-Budde @ 2010-10-15 10:33 UTC (permalink / raw)
  To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA

Moin,

this series of patches improves the mcp251x driver. It first fixes the
local_softirq_pending problem. Then the amount of SPI transfers is reduced
in order to optimise the driver.

This series has been tested with a mcp2515 on i.MX35.

Please review and test,
cheers, Marc


The following changes since commit cd2638a86c7b90e77ce623c09de2a26177f2a5c1:
  Carolyn Wyborny (1):
        igb: add check for fiber/serdes devices to igb_set_spd_dplx;

are available in the git repository at:

  git://git.pengutronix.de/git/mkl/linux-2.6.git can/mcp251x-for-net-next

Marc Kleine-Budde (4):
      can: mcp251x: fix NOHZ local_softirq_pending 08 warning
      can: mcp251x: write intf only when needed
      can: mcp251x: define helper functions mcp251x_is_2510, mcp251x_is_2515
      can: mcp251x: optimize 2515, rx int gets cleared automatically

Sascha Hauer (3):
      can: mcp251x: increase rx_errors on overflow, not only rx_over_errors
      can: mcp251x: allow to read two registers in one spi transfer
      can: mcp251x: read-modify-write eflag only when needed

 drivers/net/can/mcp251x.c |   77 +++++++++++++++++++++++++++++++++++----------
 1 files changed, 60 insertions(+), 17 deletions(-)

^ permalink raw reply

* [PATCH 1/7] can: mcp251x: fix NOHZ local_softirq_pending 08 warning
From: Marc Kleine-Budde @ 2010-10-15 10:33 UTC (permalink / raw)
  To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Marc Kleine-Budde
In-Reply-To: <1287138845-20561-1-git-send-email-mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

This patch replaces netif_rx() with netif_rx_ni() which has to be used
from the threaded interrupt i.e. process context context.

Thanks to Christian Pellegrin for pointing at the right fix:
481a8199142c050b72bff8a1956a49fd0a75bbe0 by Oliver Hartkopp.

Signed-off-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/net/can/mcp251x.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
index b11a0cb..c06e023 100644
--- a/drivers/net/can/mcp251x.c
+++ b/drivers/net/can/mcp251x.c
@@ -451,7 +451,7 @@ static void mcp251x_hw_rx(struct spi_device *spi, int buf_idx)
 
 	priv->net->stats.rx_packets++;
 	priv->net->stats.rx_bytes += frame->can_dlc;
-	netif_rx(skb);
+	netif_rx_ni(skb);
 }
 
 static void mcp251x_hw_sleep(struct spi_device *spi)
@@ -676,7 +676,7 @@ static void mcp251x_error_skb(struct net_device *net, int can_id, int data1)
 	if (skb) {
 		frame->can_id = can_id;
 		frame->data[1] = data1;
-		netif_rx(skb);
+		netif_rx_ni(skb);
 	} else {
 		dev_err(&net->dev,
 			"cannot allocate error skb\n");
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 2/7] can: mcp251x: increase rx_errors on overflow, not only rx_over_errors
From: Marc Kleine-Budde @ 2010-10-15 10:34 UTC (permalink / raw)
  To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Marc Kleine-Budde
In-Reply-To: <1287138845-20561-1-git-send-email-mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

From: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/net/can/mcp251x.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
index c06e023..fdea752 100644
--- a/drivers/net/can/mcp251x.c
+++ b/drivers/net/can/mcp251x.c
@@ -816,10 +816,14 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id)
 		if (intf & CANINTF_ERRIF) {
 			/* Handle overflow counters */
 			if (eflag & (EFLG_RX0OVR | EFLG_RX1OVR)) {
-				if (eflag & EFLG_RX0OVR)
+				if (eflag & EFLG_RX0OVR) {
 					net->stats.rx_over_errors++;
-				if (eflag & EFLG_RX1OVR)
+					net->stats.rx_errors++;
+				}
+				if (eflag & EFLG_RX1OVR) {
 					net->stats.rx_over_errors++;
+					net->stats.rx_errors++;
+				}
 				can_id |= CAN_ERR_CRTL;
 				data1 |= CAN_ERR_CRTL_RX_OVERFLOW;
 			}
-- 
1.7.0.4

^ permalink raw reply related

* Re: [PATCH net-next 2/5] tipc: Simplify bearer shutdown logic
From: Neil Horman @ 2010-10-15 10:48 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: davem, netdev, allan.stephens
In-Reply-To: <20101014235825.GA5048@windriver.com>

On Thu, Oct 14, 2010 at 07:58:26PM -0400, Paul Gortmaker wrote:
> [Re: [PATCH net-next 2/5] tipc: Simplify bearer shutdown logic] On 13/10/2010 (Wed 10:39) Neil Horman wrote:
> 
> > On Tue, Oct 12, 2010 at 08:25:55PM -0400, Paul Gortmaker wrote:
> > > From: Allan Stephens <allan.stephens@windriver.com>
> > > 
> > > Disable all active bearers when TIPC is shut down without having to do
> > > a name-based search to locate each bearer object.
> > > 
> > It seems like you're doing a good deal more in this patch than just disabling
> > all active bearers without doing a name search.  The description is implemented
> > in the for loop of tipc_bearer_stop.  Whats the rest of it for?
> 
> It seems the original needlessly bloated out the patch size by
> swapping the order of tipc_bearer_find_interface & bearer_find
> in the file (now fixed) - and you are right, the locking change
> wasn't properly covered in the commit log.  The extra test you'd
> suggested tossing out is also now gone.
> 
> This change doesn't explicitly depend on any other changes,
> so if it is now OK, the option is there for it to be applied
> independently of the others that haven't been reworked yet.
> 
> Thanks,
> Paul.
> 
> 
> From 1771ad642cb076dbeb71e3533a25cb2f07df9cd8 Mon Sep 17 00:00:00 2001
> From: Allan Stephens <allan.stephens@windriver.com>
> Date: Sat, 4 Sep 2010 09:29:04 -0400
> Subject: [PATCH] tipc: Simplify bearer shutdown logic
> 
> Optimize processing in TIPC's bearer shutdown code, including:
> 
> 1. Remove an unnecessary check to see if TIPC bearer's can exist.
> 2. Don't release spinlocks before calling a media-specific disabling
> routine, since the routine can't sleep.
> 3. Make bearer_disable() operate directly on a struct bearer, instead
> of needlessly taking a name and then mapping that to the struct.
> 
> Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  net/tipc/bearer.c |   38 +++++++++++---------------------------
>  1 files changed, 11 insertions(+), 27 deletions(-)
> 
> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> index 9c10c6b..fd9c06c 100644
> --- a/net/tipc/bearer.c
> +++ b/net/tipc/bearer.c
> @@ -288,9 +288,6 @@ static struct bearer *bearer_find(const char *name)
>  	struct bearer *b_ptr;
>  	u32 i;
>  
> -	if (tipc_mode != TIPC_NET_MODE)
> -		return NULL;
> -
>  	for (i = 0, b_ptr = tipc_bearers; i < MAX_BEARERS; i++, b_ptr++) {
>  		if (b_ptr->active && (!strcmp(b_ptr->publ.name, name)))
>  			return b_ptr;
> @@ -630,30 +627,17 @@ int tipc_block_bearer(const char *name)
>   * Note: This routine assumes caller holds tipc_net_lock.
>   */
>  
> -static int bearer_disable(const char *name)
> +static int bearer_disable(struct bearer *b_ptr)
>  {
> -	struct bearer *b_ptr;
>  	struct link *l_ptr;
>  	struct link *temp_l_ptr;
>  
> -	b_ptr = bearer_find(name);
> -	if (!b_ptr) {
> -		warn("Attempt to disable unknown bearer <%s>\n", name);
> -		return -EINVAL;
> -	}
> -
> -	info("Disabling bearer <%s>\n", name);
> +	info("Disabling bearer <%s>\n", b_ptr->publ.name);
>  	tipc_disc_stop_link_req(b_ptr->link_req);
>  	spin_lock_bh(&b_ptr->publ.lock);
>  	b_ptr->link_req = NULL;
>  	b_ptr->publ.blocked = 1;
> -	if (b_ptr->media->disable_bearer) {
> -		spin_unlock_bh(&b_ptr->publ.lock);
> -		write_unlock_bh(&tipc_net_lock);
> -		b_ptr->media->disable_bearer(&b_ptr->publ);
> -		write_lock_bh(&tipc_net_lock);
> -		spin_lock_bh(&b_ptr->publ.lock);
> -	}
> +	b_ptr->media->disable_bearer(&b_ptr->publ);
>  	list_for_each_entry_safe(l_ptr, temp_l_ptr, &b_ptr->links, link_list) {
>  		tipc_link_delete(l_ptr);
>  	}
> @@ -664,10 +648,16 @@ static int bearer_disable(const char *name)
>  
>  int tipc_disable_bearer(const char *name)
>  {
> +	struct bearer *b_ptr;
>  	int res;
>  
>  	write_lock_bh(&tipc_net_lock);
> -	res = bearer_disable(name);
> +	b_ptr = bearer_find(name);
> +	if (b_ptr == NULL) {
> +		warn("Attempt to disable unknown bearer <%s>\n", name);
> +		res = -EINVAL;
> +	} else
> +		res = bearer_disable(b_ptr);
>  	write_unlock_bh(&tipc_net_lock);
>  	return res;
>  }
> @@ -680,13 +670,7 @@ void tipc_bearer_stop(void)
>  
>  	for (i = 0; i < MAX_BEARERS; i++) {
>  		if (tipc_bearers[i].active)
> -			tipc_bearers[i].publ.blocked = 1;
> -	}
> -	for (i = 0; i < MAX_BEARERS; i++) {
> -		if (tipc_bearers[i].active)
> -			bearer_disable(tipc_bearers[i].publ.name);
> +			bearer_disable(&tipc_bearers[i]);
>  	}
>  	media_count = 0;
>  }
> -
> -
> -- 
> 1.7.2.1
> 
> 

Yes, this looks much better, thank you.
Reviewed-by: Neil Horman <nhorman@tuxdriver.com>


^ permalink raw reply

* [PATCH V2 0/7] can: mcp251x: fix and optimize driver
From: Marc Kleine-Budde @ 2010-10-15 10:49 UTC (permalink / raw)
  To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA

Moin,

this series of patches improves the mcp251x driver. It first fixes the
local_softirq_pending problem. Then the amount of SPI transfers is reduced
in order to optimise the driver.

This series has been tested with a mcp2515 on i.MX35.

Changes since V1:
- Fix broken encoding in S-o-b

Please review, test and consider to apply.

regards, Marc

---

The following changes since commit cd2638a86c7b90e77ce623c09de2a26177f2a5c1:
  Carolyn Wyborny (1):
        igb: add check for fiber/serdes devices to igb_set_spd_dplx;

are available in the git repository at:

  git://git.pengutronix.de/git/mkl/linux-2.6.git can/mcp251x-for-net-next

Marc Kleine-Budde (4):
      can: mcp251x: fix NOHZ local_softirq_pending 08 warning
      can: mcp251x: write intf only when needed
      can: mcp251x: define helper functions mcp251x_is_2510, mcp251x_is_2515
      can: mcp251x: optimize 2515, rx int gets cleared automatically

Sascha Hauer (3):
      can: mcp251x: increase rx_errors on overflow, not only rx_over_errors
      can: mcp251x: allow to read two registers in one spi transfer
      can: mcp251x: read-modify-write eflag only when needed

 drivers/net/can/mcp251x.c |   77 +++++++++++++++++++++++++++++++++++----------
 1 files changed, 60 insertions(+), 17 deletions(-)

^ permalink raw reply

* [PATCH 1/7] can: mcp251x: fix NOHZ local_softirq_pending 08 warning
From: Marc Kleine-Budde @ 2010-10-15 10:49 UTC (permalink / raw)
  To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Marc Kleine-Budde
In-Reply-To: <1287139762-23356-1-git-send-email-mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

This patch replaces netif_rx() with netif_rx_ni() which has to be used
from the threaded interrupt i.e. process context context.

Thanks to Christian Pellegrin for pointing at the right fix:
481a8199142c050b72bff8a1956a49fd0a75bbe0 by Oliver Hartkopp.

Signed-off-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/net/can/mcp251x.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
index b11a0cb..c06e023 100644
--- a/drivers/net/can/mcp251x.c
+++ b/drivers/net/can/mcp251x.c
@@ -451,7 +451,7 @@ static void mcp251x_hw_rx(struct spi_device *spi, int buf_idx)
 
 	priv->net->stats.rx_packets++;
 	priv->net->stats.rx_bytes += frame->can_dlc;
-	netif_rx(skb);
+	netif_rx_ni(skb);
 }
 
 static void mcp251x_hw_sleep(struct spi_device *spi)
@@ -676,7 +676,7 @@ static void mcp251x_error_skb(struct net_device *net, int can_id, int data1)
 	if (skb) {
 		frame->can_id = can_id;
 		frame->data[1] = data1;
-		netif_rx(skb);
+		netif_rx_ni(skb);
 	} else {
 		dev_err(&net->dev,
 			"cannot allocate error skb\n");
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 2/7] can: mcp251x: increase rx_errors on overflow, not only rx_over_errors
From: Marc Kleine-Budde @ 2010-10-15 10:49 UTC (permalink / raw)
  To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Marc Kleine-Budde
In-Reply-To: <1287139762-23356-1-git-send-email-mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

From: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/net/can/mcp251x.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
index c06e023..fdea752 100644
--- a/drivers/net/can/mcp251x.c
+++ b/drivers/net/can/mcp251x.c
@@ -816,10 +816,14 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id)
 		if (intf & CANINTF_ERRIF) {
 			/* Handle overflow counters */
 			if (eflag & (EFLG_RX0OVR | EFLG_RX1OVR)) {
-				if (eflag & EFLG_RX0OVR)
+				if (eflag & EFLG_RX0OVR) {
 					net->stats.rx_over_errors++;
-				if (eflag & EFLG_RX1OVR)
+					net->stats.rx_errors++;
+				}
+				if (eflag & EFLG_RX1OVR) {
 					net->stats.rx_over_errors++;
+					net->stats.rx_errors++;
+				}
 				can_id |= CAN_ERR_CRTL;
 				data1 |= CAN_ERR_CRTL_RX_OVERFLOW;
 			}
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 3/7] can: mcp251x: allow to read two registers in one spi transfer
From: Marc Kleine-Budde @ 2010-10-15 10:49 UTC (permalink / raw)
  To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Marc Kleine-Budde,
	Uwe Kleine-König
In-Reply-To: <1287139762-23356-1-git-send-email-mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

From: Sascha Hauer <s.hauer@pengutronix.de>

This patch bases on work done earlier by David Jander.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Acked-by: David Jander <david@protonic.nl>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/mcp251x.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
index fdea752..9b3466a 100644
--- a/drivers/net/can/mcp251x.c
+++ b/drivers/net/can/mcp251x.c
@@ -319,6 +319,20 @@ static u8 mcp251x_read_reg(struct spi_device *spi, uint8_t reg)
 	return val;
 }
 
+static void mcp251x_read_2regs(struct spi_device *spi, uint8_t reg,
+		uint8_t *v1, uint8_t *v2)
+{
+	struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
+
+	priv->spi_tx_buf[0] = INSTRUCTION_READ;
+	priv->spi_tx_buf[1] = reg;
+
+	mcp251x_spi_trans(spi, 4);
+
+	*v1 = priv->spi_rx_buf[2];
+	*v2 = priv->spi_rx_buf[3];
+}
+
 static void mcp251x_write_reg(struct spi_device *spi, u8 reg, uint8_t val)
 {
 	struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
@@ -754,10 +768,11 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id)
 	mutex_lock(&priv->mcp_lock);
 	while (!priv->force_quit) {
 		enum can_state new_state;
-		u8 intf = mcp251x_read_reg(spi, CANINTF);
-		u8 eflag;
+		u8 intf, eflag;
 		int can_id = 0, data1 = 0;
 
+		mcp251x_read_2regs(spi, CANINTF, &intf, &eflag);
+
 		if (intf & CANINTF_RX0IF) {
 			mcp251x_hw_rx(spi, 0);
 			/* Free one buffer ASAP */
@@ -770,7 +785,6 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id)
 
 		mcp251x_write_bits(spi, CANINTF, intf, 0x00);
 
-		eflag = mcp251x_read_reg(spi, EFLG);
 		mcp251x_write_reg(spi, EFLG, 0x00);
 
 		/* Update can state */
-- 
1.7.0.4

_______________________________________________
Socketcan-core mailing list
Socketcan-core@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply related

* [PATCH 4/7] can: mcp251x: read-modify-write eflag only when needed
From: Marc Kleine-Budde @ 2010-10-15 10:49 UTC (permalink / raw)
  To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Marc Kleine-Budde
In-Reply-To: <1287139762-23356-1-git-send-email-mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

From: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Use read-modify-write instead of a simple write to change the register
contents, to close existing the race window between the original manual
read and write.

Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/net/can/mcp251x.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
index 9b3466a..7e2f951 100644
--- a/drivers/net/can/mcp251x.c
+++ b/drivers/net/can/mcp251x.c
@@ -785,7 +785,8 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id)
 
 		mcp251x_write_bits(spi, CANINTF, intf, 0x00);
 
-		mcp251x_write_reg(spi, EFLG, 0x00);
+		if (eflag)
+			mcp251x_write_bits(spi, EFLG, eflag, 0x00);
 
 		/* Update can state */
 		if (eflag & EFLG_TXBO) {
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 6/7] can: mcp251x: define helper functions mcp251x_is_2510, mcp251x_is_2515
From: Marc Kleine-Budde @ 2010-10-15 10:49 UTC (permalink / raw)
  To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Marc Kleine-Budde
In-Reply-To: <1287139762-23356-1-git-send-email-mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Signed-off-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/net/can/mcp251x.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
index f5e2edd..28281f9 100644
--- a/drivers/net/can/mcp251x.c
+++ b/drivers/net/can/mcp251x.c
@@ -252,6 +252,16 @@ struct mcp251x_priv {
 	int restart_tx;
 };
 
+#define MCP251X_IS(_model) \
+static inline int mcp251x_is_##_model(struct spi_device *spi) \
+{ \
+	struct mcp251x_platform_data *pdata = spi->dev.platform_data; \
+	return pdata->model == CAN_MCP251X_MCP##_model; \
+}
+
+MCP251X_IS(2510);
+MCP251X_IS(2515);
+
 static void mcp251x_clean(struct net_device *net)
 {
 	struct mcp251x_priv *priv = netdev_priv(net);
@@ -362,10 +372,9 @@ static void mcp251x_write_bits(struct spi_device *spi, u8 reg,
 static void mcp251x_hw_tx_frame(struct spi_device *spi, u8 *buf,
 				int len, int tx_buf_idx)
 {
-	struct mcp251x_platform_data *pdata = spi->dev.platform_data;
 	struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
 
-	if (pdata->model == CAN_MCP251X_MCP2510) {
+	if (mcp251x_is_2510(spi)) {
 		int i;
 
 		for (i = 1; i < TXBDAT_OFF + len; i++)
@@ -408,9 +417,8 @@ static void mcp251x_hw_rx_frame(struct spi_device *spi, u8 *buf,
 				int buf_idx)
 {
 	struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
-	struct mcp251x_platform_data *pdata = spi->dev.platform_data;
 
-	if (pdata->model == CAN_MCP251X_MCP2510) {
+	if (mcp251x_is_2510(spi)) {
 		int i, len;
 
 		for (i = 1; i < RXBDAT_OFF; i++)
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 7/7] can: mcp251x: optimize 2515, rx int gets cleared automatically
From: Marc Kleine-Budde @ 2010-10-15 10:49 UTC (permalink / raw)
  To: socketcan-core; +Cc: netdev, Marc Kleine-Budde
In-Reply-To: <1287139762-23356-1-git-send-email-mkl@pengutronix.de>

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/mcp251x.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
index 28281f9..a42c3fa 100644
--- a/drivers/net/can/mcp251x.c
+++ b/drivers/net/can/mcp251x.c
@@ -787,15 +787,20 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id)
 		/* receive buffer 0 */
 		if (intf & CANINTF_RX0IF) {
 			mcp251x_hw_rx(spi, 0);
-			/* Free one buffer ASAP */
-			mcp251x_write_bits(spi, CANINTF, intf & CANINTF_RX0IF,
-					   0x00);
+			/*
+			 * Free one buffer ASAP
+			 * (The MCP2515 does this automatically.)
+			 */
+			if (mcp251x_is_2510(spi))
+				mcp251x_write_bits(spi, CANINTF, CANINTF_RX0IF, 0x00);
 		}
 
 		/* receive buffer 1 */
 		if (intf & CANINTF_RX1IF) {
 			mcp251x_hw_rx(spi, 1);
-			clear_intf |= CANINTF_RX1IF;
+			/* the MCP2515 does this automatically */
+			if (mcp251x_is_2510(spi))
+				clear_intf |= CANINTF_RX1IF;
 		}
 
 		/* any error or tx interrupt we need to clear? */
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 5/7] can: mcp251x: write intf only when needed
From: Marc Kleine-Budde @ 2010-10-15 10:49 UTC (permalink / raw)
  To: socketcan-core; +Cc: netdev, Marc Kleine-Budde
In-Reply-To: <1287139762-23356-1-git-send-email-mkl@pengutronix.de>

This patch introduces a variable "clear_intf" that hold the bits that
should be cleared. Only read-modify-write register if "clear_intf"
is set.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/mcp251x.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
index 7e2f951..f5e2edd 100644
--- a/drivers/net/can/mcp251x.c
+++ b/drivers/net/can/mcp251x.c
@@ -125,6 +125,8 @@
 #  define CANINTF_TX0IF 0x04
 #  define CANINTF_RX1IF 0x02
 #  define CANINTF_RX0IF 0x01
+#  define CANINTF_ERR_TX \
+	(CANINTF_ERRIF | CANINTF_TX2IF | CANINTF_TX1IF | CANINTF_TX0IF)
 #define EFLG	      0x2d
 #  define EFLG_EWARN	0x01
 #  define EFLG_RXWAR	0x02
@@ -769,10 +771,12 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id)
 	while (!priv->force_quit) {
 		enum can_state new_state;
 		u8 intf, eflag;
+		u8 clear_intf = 0;
 		int can_id = 0, data1 = 0;
 
 		mcp251x_read_2regs(spi, CANINTF, &intf, &eflag);
 
+		/* receive buffer 0 */
 		if (intf & CANINTF_RX0IF) {
 			mcp251x_hw_rx(spi, 0);
 			/* Free one buffer ASAP */
@@ -780,10 +784,17 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id)
 					   0x00);
 		}
 
-		if (intf & CANINTF_RX1IF)
+		/* receive buffer 1 */
+		if (intf & CANINTF_RX1IF) {
 			mcp251x_hw_rx(spi, 1);
+			clear_intf |= CANINTF_RX1IF;
+		}
 
-		mcp251x_write_bits(spi, CANINTF, intf, 0x00);
+		/* any error or tx interrupt we need to clear? */
+		if (intf & CANINTF_ERR_TX)
+			clear_intf |= intf & CANINTF_ERR_TX;
+		if (clear_intf)
+			mcp251x_write_bits(spi, CANINTF, clear_intf, 0x00);
 
 		if (eflag)
 			mcp251x_write_bits(spi, EFLG, eflag, 0x00);
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic
From: Neil Horman @ 2010-10-15 11:00 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: davem, netdev, allan.stephens
In-Reply-To: <20101015011139.GB5048@windriver.com>

On Thu, Oct 14, 2010 at 09:11:51PM -0400, Paul Gortmaker wrote:
> [Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic] On 13/10/2010 (Wed 10:58) Neil Horman wrote:
> 
> > On Tue, Oct 12, 2010 at 08:25:56PM -0400, Paul Gortmaker wrote:
> > > From: Allan Stephens <allan.stephens@windriver.com>
> > > 
> > > Introduces "enabling" state during activation of a new TIPC bearer,
> > > which supplements the existing "disabled" and "enabled" states.
> > > This change allows the new bearer to be added without having to
> > > temporarily block the processing of incoming packets on existing
> > > bearers during the binding of the new bearer to its associated
> > > interface. It also makes it unnecessary to zero out the entire
> > > bearer structure at the start of activation.
> > > 
> 
> [...]
> 
> > > +	b_ptr->state = BEARER_ENABLING;
> > >  	strcpy(b_ptr->publ.name, name);
> > > +	b_ptr->priority = priority;
> > > +
> > > +	write_unlock_bh(&tipc_net_lock);
> > Why the 3rd state?  Doesn't seem needed. 
> 
> I'm a bit disappointed in myself for also not noticing that it
> was set but never tested for.  The following should give the
> same end result but without the obfuscation of an extra state.
> 
> This one also doesn't explicitly depend on any other changes,
> so if it is now OK, the option is there for it to be applied
> independently of the others that haven't been reworked yet.
> 
> Thanks,
> Paul.
> 
> 
> From 86d0d5c92439d0a3f5a0f165aa8bd842d377dae9 Mon Sep 17 00:00:00 2001
> From: Allan Stephens <allan.stephens@windriver.com>
> Date: Thu, 14 Oct 2010 16:09:23 -0400
> Subject: [PATCH] tipc: Optimizations to bearer enabling logic
> 
> Allow new bearers to be added without having to temporarily block
> the processing of incoming packets on existing bearers during the
> binding of the new bearer to its associated interface. Eliminates
> zeroing out of the new bearer structure at the start of activation,
> since it is already in that state.
> 
> Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  net/tipc/bearer.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> index fd9c06c..2ff8181 100644
> --- a/net/tipc/bearer.c
> +++ b/net/tipc/bearer.c
> @@ -556,14 +556,15 @@ restart:
>  	}
>  
>  	b_ptr = &tipc_bearers[bearer_id];
> -	memset(b_ptr, 0, sizeof(struct bearer));
> -
>  	strcpy(b_ptr->publ.name, name);
> +
> +	write_unlock_bh(&tipc_net_lock);
>  	res = m_ptr->enable_bearer(&b_ptr->publ);
>  	if (res) {
>  		warn("Bearer <%s> rejected, enable failure (%d)\n", name, -res);
> -		goto failed;
> +		return res;
>  	}
> +	write_lock_bh(&tipc_net_lock);
>  
This definately looks more concise, but I don't see why its necessecary to drop
the tipc_net_lock around the call to enable_bearer.  The only caler of
tipc_register_media sets the enable_bearer pointer to the enable_bearer
function, and  I' don't see any path through that function which would
potentially retake that lock.  In fact it seems dropping it might be dangerous,
if other paths (like from cfg_named_msg_event), tried to enable a bearer in
parallel with a user space directive from the netlink socket).  With out the
protection of tipc_net_lock, you could use the same eth_bearer twice, and
corrupt that array.

Neil

>  	b_ptr->identity = bearer_id;
>  	b_ptr->media = m_ptr;
> -- 
> 1.7.2.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: [PATCH net-next] tipc: cleanup function namespace
From: Neil Horman @ 2010-10-15 11:01 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Paul Gortmaker, David Miller, netdev, allan.stephens, Jon Maloy
In-Reply-To: <20101014151333.1446a90c@nehalam>

On Thu, Oct 14, 2010 at 03:13:33PM -0700, Stephen Hemminger wrote:
> On Thu, 14 Oct 2010 17:44:27 -0400
> Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> 
> > [Re: [PATCH net-next] tipc: cleanup function namespace] On 14/10/2010 (Thu 11:33) Stephen Hemminger wrote:
> > 
> > > On Thu, 14 Oct 2010 13:53:21 -0400
> > > Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> > > 
> > > > On 10-10-13 09:29 PM, Neil Horman wrote:
> > > > > On Wed, Oct 13, 2010 at 08:23:24PM -0400, Paul Gortmaker wrote:
> > > > >> On 10-10-13 07:20 PM, Stephen Hemminger wrote:
> > > > >>> Do some cleanups of TIPC based on make namespacecheck
> > > > >>>     1. Don't export unused symbols
> > > > >>>     2. Eliminate dead code
> > > > >>>     3. Make functions and variables local
> > > > >>>     4. Rename buf_acquire to tipc_buf_acquire since it is used in several files
> > > > >>>
> > > > >>> Compile tested only.
> > > > >>> This make break out of tree kernel modules that depend on TIPC routines.
> > > > >>
> > > > >> Hi Stephen,
> > > > >>
> > > > >> When I first started looking at TIPC code, I too came to the
> > > > >> same conclusion as you did and was about to do #1,2,3 -- but
> > > > >> then I was told that the exported symbols were part of an API
> > > > >> and might be in use by folks here and there as per this thread:
> > > > >>
> > > > >> http://www.mail-archive.com/netdev@vger.kernel.org/msg30208.html
> > > > >>
> > > > > I think its telling the the argument in the above thread for keeping the API
> > > > > were that users of it were out there and 'likely to contribute' in the future.
> > > > > That thread was 3 years ago.  They might be using the API from outside the
> > > > > kernel tree, but they're not planning on contributing.  As Christoph noted,
> > > > > they're freeloaders.  The community really doesn't need or want to maintain an
> > > > > API like that.  If these users are your customers, and removing the API is
> > > > > unacceptable, perhaps its time to move the entire TIPC module out of tree.
> > > > 
> > > > As I'd said -- I don't know what the use cases of these API users are,
> > > > and so as far as I know they aren't customers either.  For what it is
> > > > worth, know that I personally wouldn't try and use a business case to
> > > > justify a technically wrong decision here on netdev anyway.
> > > > 
> > > > I was just describing the history of the situation, and suggesting
> > > > one possible slower approach of phasing it out as a courtesy to those
> > > > users, in the same way that the kernel community has extended that
> > > > same courtesy with other things in feature-removal.txt
> > > > 
> > > > In the end, since Jon is OK with the removal, and is in the process of
> > > > communicating this to the API users he is aware of, I sure don't have
> > > > any reason to try and save the API.  If folks are good with having it
> > > > just go away overnight, then great -- I'll be just as happy to see it
> > > > disappear as you and Stephen.  So, a long winded way of saying...
> > > > 
> > > > Acked-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> > > 
> > > How about putting an entry in feature-removal.txt with a short (6 month)
> > > window?
> > 
> > I'm fine with that too.
> > 
> > P.
> > 
> > From 5a15a26de63a29fcb6cb7a7fb83b6d2fc63cbadb Mon Sep 17 00:00:00 2001
> > From: Paul Gortmaker <paul.gortmaker@windriver.com>
> > Date: Thu, 14 Oct 2010 17:29:08 -0400
> > Subject: [PATCH] TIPC: Document the demise of the Native API for March 2011
> > 
> > The native API in the TIPC code exists as a bunch of functions
> > and exported symbols that aren't actually used by any currently
> > in-tree kernel code/modules.
> > 
> > Since this code is anomalous to the general guiding principle that
> > the kernel should not be libc, coverage tools and people intending
> > to do general cleanups keep finding this code and suggesting that
> > it be removed.
> > 
> > It seems the right thing to do is to just finally delete it once
> > and for all, after giving a reasonable window for any existing
> > users to find alternative solutions to their custom use case(s).
> > 
> > Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> > ---
> >  Documentation/feature-removal-schedule.txt |   12 ++++++++++++
> >  1 files changed, 12 insertions(+), 0 deletions(-)
> > 
> > diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
> > index f456389..1def37e 100644
> > --- a/Documentation/feature-removal-schedule.txt
> > +++ b/Documentation/feature-removal-schedule.txt
> > @@ -573,3 +573,15 @@ Why:	Hareware scan is the prefer method for iwlwifi devices for
> >  Who:	Wey-Yi Guy <wey-yi.w.guy@intel.com>
> >  
> >  ----------------------------
> > +
> > +What:	TIPC: Delete all code and exported symbols specific to Native API
> > +When:	March 2011
> > +Why:	The TIPC Native API, as described here:
> > +	http://tipc.sourceforge.net/doc/tipc_1.7_prog_guide.html#native_api
> > +	is implemented by exporting a bunch of otherwise unused functions
> > +	for possible modular linkage by custom end-user code.  This goes
> > +	against the general concept that the kernel should not be libc.
> > +
> > +Who:	Paul Gortmaker <paul.gortmaker@windriver.com>
> > +	
> > +----------------------------
> 
> Acked-by: Stephen Hemminger <shemminger@vyatta.com>
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>


^ permalink raw reply

* openvswitch/flow WAS ( Re: [rfc] Merging the Open vSwitch datapath
From: jamal @ 2010-10-15 11:31 UTC (permalink / raw)
  To: Ben Pfaff, Jesse Gross; +Cc: netdev
In-Reply-To: <87k4n8ow1r.fsf@benpfaff.org>


Sorry, slightly off topic - and this old (catching up 
with netdev randomly going backwards)..

I was curious after seeing the exchange with Eric D. on
the tunnels and how you have your own tree so i glossed 
over what your project is doing.

It seems to me that you reinvented things that exist in 
Linux already such as bridging, tunnels and what really
caught my attention: ability to do flows (tc actions).
It is possible Linux is missing something you wanted or was
not efficient enough? 
[For example: I couldnt see anything you needed
on flow-action management that Linux couldnt do already
(with already very nice well structured netlink APIs)]

cheers,
jamal


^ permalink raw reply

* Re: Couple tc filter questions.
From: Jarek Poplawski @ 2010-10-15 11:44 UTC (permalink / raw)
  To: Jonathan Thibault; +Cc: netdev
In-Reply-To: <4CB792E0.5030704@navigue.com>

On 2010-10-15 01:31, Jonathan Thibault wrote:
> Since the lartc mailing list appears to be dead, I'll ask here and hope not to offend anyone.
> 
> 1- This page:
>   http://lartc.org/howto/lartc.qdisc.filters.html
> States: "Also, with HTB, you should attach all filters to the root!"
> 
> Why?  Is it still true?  My setup would be a lot easier with cascading filters.  If it's just a matter of there not being any efficiency gains from cascading filters, that's fine.  If there is a risk of things exploding randomly and without notice, I'd be keen to know.  Testing shows that cascading works okay, but I haven't tried under any serious load.

It's not true.

> 
> 2- Are filter flowid (classify) actions terminating?  Meaning if two consecutive filters would match the same packet, only the first match would ever apply and no further filter is evaluated?  Are there actions for which this isn't the case?  Intuitively and experimentally, I'd answer no but if anyone knowledgeable in the matter would care to expand on that topic I'd be grateful.  Especially considering cascading classes/filters.
> 
> Another area where termination isn't entirely clear is when using mirred and ifb devices.  I might want to send a copy of all my traffic to an ifb device, but then I would still want subsequent filters to match in the current qdisc.  In such a case, a filter that matches all traffic with a mirred action should probably not be terminating.
> 
> Maybe I'm thinking too much in terms of iptables here :P

Could you try doc/actions from iproute2 sources? Generally, anything
reasonable, not forbidden and working in tests is allowed. If still
problems please CC Jamal, the maintainer of tc classifiers (according
to linux MAINTAINERS).

Jarek P.

^ permalink raw reply

* [PATCH 1/6] r8169: check dma mapping failures
From: Stanislaw Gruszka @ 2010-10-15 12:15 UTC (permalink / raw)
  To: Francois Romieu, netdev; +Cc: Denis Kirjanov, Stanislaw Gruszka

Check possible dma mapping errors and do clean up if it happens,
when sending frames stop the tx queue.

Fix overwrap bug in rtl8169_tx_clear on the way.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
All patches in series was tested on RTL8111/8168B

 drivers/net/r8169.c |   69 ++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 46 insertions(+), 23 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index bc669a4..a6c4f90 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -4018,20 +4018,25 @@ static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
 
 	skb = __netdev_alloc_skb(dev, rx_buf_sz + pad, gfp);
 	if (!skb)
-		goto err_out;
+		goto err_out_0;
 
 	skb_reserve(skb, align ? ((pad - 1) & (unsigned long)skb->data) : pad);
 
 	mapping = dma_map_single(&pdev->dev, skb->data, rx_buf_sz,
 				 PCI_DMA_FROMDEVICE);
+	if (dma_mapping_error(&pdev->dev, mapping))
+		goto err_free_skb_1;
 
 	rtl8169_map_to_asic(desc, mapping, rx_buf_sz);
-out:
+
 	return skb;
 
-err_out:
+err_free_skb_1:
+	dev_kfree_skb(skb);
+
+err_out_0:
 	rtl8169_make_unusable_by_asic(desc);
-	goto out;
+	return NULL;
 }
 
 static void rtl8169_rx_clear(struct rtl8169_private *tp)
@@ -4115,12 +4120,12 @@ static void rtl8169_unmap_tx_skb(struct pci_dev *pdev, struct ring_info *tx_skb,
 	tx_skb->len = 0;
 }
 
-static void rtl8169_tx_clear(struct rtl8169_private *tp)
+static void rtl8169_tx_clear_range(struct rtl8169_private *tp, u32 start, int n)
 {
-	unsigned int i;
+	int i;
 
-	for (i = tp->dirty_tx; i < tp->dirty_tx + NUM_TX_DESC; i++) {
-		unsigned int entry = i % NUM_TX_DESC;
+	for (i = 0; i < n; i++) {
+		unsigned int entry = (start + i) % NUM_TX_DESC;
 		struct ring_info *tx_skb = tp->tx_skb + entry;
 		unsigned int len = tx_skb->len;
 
@@ -4136,6 +4141,11 @@ static void rtl8169_tx_clear(struct rtl8169_private *tp)
 			tp->dev->stats.tx_dropped++;
 		}
 	}
+}
+
+static void rtl8169_tx_clear(struct rtl8169_private *tp)
+{
+	rtl8169_tx_clear_range(tp, tp->dirty_tx, NUM_TX_DESC);
 	tp->cur_tx = tp->dirty_tx = 0;
 }
 
@@ -4254,6 +4264,8 @@ static int rtl8169_xmit_frags(struct rtl8169_private *tp, struct sk_buff *skb,
 		addr = ((void *) page_address(frag->page)) + frag->page_offset;
 		mapping = dma_map_single(&tp->pci_dev->dev, addr, len,
 					 PCI_DMA_TODEVICE);
+		if (unlikely(dma_mapping_error(&tp->pci_dev->dev, mapping)))
+			goto err_out;
 
 		/* anti gcc 2.95.3 bugware (sic) */
 		status = opts1 | len | (RingEnd * !((entry + 1) % NUM_TX_DESC));
@@ -4270,6 +4282,10 @@ static int rtl8169_xmit_frags(struct rtl8169_private *tp, struct sk_buff *skb,
 	}
 
 	return cur_frag;
+
+err_out:
+	rtl8169_tx_clear_range(tp, tp->cur_tx, cur_frag);
+	return -EIO;
 }
 
 static inline u32 rtl8169_tso_csum(struct sk_buff *skb, struct net_device *dev)
@@ -4296,40 +4312,44 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 				      struct net_device *dev)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
-	unsigned int frags, entry = tp->cur_tx % NUM_TX_DESC;
+	unsigned int entry = tp->cur_tx % NUM_TX_DESC;
 	struct TxDesc *txd = tp->TxDescArray + entry;
 	void __iomem *ioaddr = tp->mmio_addr;
 	dma_addr_t mapping;
 	u32 status, len;
 	u32 opts1;
+	int frags;
 
 	if (unlikely(TX_BUFFS_AVAIL(tp) < skb_shinfo(skb)->nr_frags)) {
 		netif_err(tp, drv, dev, "BUG! Tx Ring full when queue awake!\n");
-		goto err_stop;
+		goto err_stop_0;
 	}
 
 	if (unlikely(le32_to_cpu(txd->opts1) & DescOwn))
-		goto err_stop;
+		goto err_stop_0;
+
+	len = skb_headlen(skb);
+	mapping = dma_map_single(&tp->pci_dev->dev, skb->data, len,
+				 PCI_DMA_TODEVICE);
+	if (unlikely(dma_mapping_error(&tp->pci_dev->dev, mapping)))
+		goto err_stop_0;
+
+	tp->tx_skb[entry].len = len;
+	txd->addr = cpu_to_le64(mapping);
+	txd->opts2 = cpu_to_le32(rtl8169_tx_vlan_tag(tp, skb));
 
 	opts1 = DescOwn | rtl8169_tso_csum(skb, dev);
 
 	frags = rtl8169_xmit_frags(tp, skb, opts1);
-	if (frags) {
-		len = skb_headlen(skb);
+	if (frags < 0)
+		goto err_dma_1;
+	else if (frags)
 		opts1 |= FirstFrag;
-	} else {
-		len = skb->len;
+	else {
 		opts1 |= FirstFrag | LastFrag;
 		tp->tx_skb[entry].skb = skb;
 	}
 
-	mapping = dma_map_single(&tp->pci_dev->dev, skb->data, len,
-				 PCI_DMA_TODEVICE);
-
-	tp->tx_skb[entry].len = len;
-	txd->addr = cpu_to_le64(mapping);
-	txd->opts2 = cpu_to_le32(rtl8169_tx_vlan_tag(tp, skb));
-
 	wmb();
 
 	/* anti gcc 2.95.3 bugware (sic) */
@@ -4351,7 +4371,10 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 
 	return NETDEV_TX_OK;
 
-err_stop:
+err_dma_1:
+	rtl8169_unmap_tx_skb(tp->pci_dev, tp->tx_skb + entry, txd);
+
+err_stop_0:
 	netif_stop_queue(dev);
 	dev->stats.tx_dropped++;
 	return NETDEV_TX_BUSY;
-- 
1.7.1


^ permalink raw reply related

* [PATCH 2/6] r8169: reduce number of functions arguments
From: Stanislaw Gruszka @ 2010-10-15 12:15 UTC (permalink / raw)
  To: Francois Romieu, netdev; +Cc: Denis Kirjanov, Stanislaw Gruszka
In-Reply-To: <1287144922-3297-1-git-send-email-sgruszka@redhat.com>

We don't need to pass arguments on stack since we have them in per
device private structure.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/r8169.c |   30 ++++++++++++------------------
 1 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index a6c4f90..86be06c 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -4005,29 +4005,26 @@ static inline void rtl8169_map_to_asic(struct RxDesc *desc, dma_addr_t mapping,
 	rtl8169_mark_to_asic(desc, rx_buf_sz);
 }
 
-static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
-					    struct net_device *dev,
-					    struct RxDesc *desc, int rx_buf_sz,
-					    unsigned int align, gfp_t gfp)
+static struct sk_buff *rtl8169_alloc_rx_skb(struct rtl8169_private *tp,
+					    struct RxDesc *desc, gfp_t gfp)
 {
 	struct sk_buff *skb;
 	dma_addr_t mapping;
-	unsigned int pad;
+	unsigned int align = tp->align;
+	unsigned int pad = align ? align : NET_IP_ALIGN;
 
-	pad = align ? align : NET_IP_ALIGN;
-
-	skb = __netdev_alloc_skb(dev, rx_buf_sz + pad, gfp);
+	skb = __netdev_alloc_skb(tp->dev, tp->rx_buf_sz + pad, gfp);
 	if (!skb)
 		goto err_out_0;
 
 	skb_reserve(skb, align ? ((pad - 1) & (unsigned long)skb->data) : pad);
 
-	mapping = dma_map_single(&pdev->dev, skb->data, rx_buf_sz,
+	mapping = dma_map_single(&tp->pci_dev->dev, skb->data, tp->rx_buf_sz,
 				 PCI_DMA_FROMDEVICE);
-	if (dma_mapping_error(&pdev->dev, mapping))
+	if (dma_mapping_error(&tp->pci_dev->dev, mapping))
 		goto err_free_skb_1;
 
-	rtl8169_map_to_asic(desc, mapping, rx_buf_sz);
+	rtl8169_map_to_asic(desc, mapping, tp->rx_buf_sz);
 
 	return skb;
 
@@ -4051,8 +4048,7 @@ static void rtl8169_rx_clear(struct rtl8169_private *tp)
 	}
 }
 
-static u32 rtl8169_rx_fill(struct rtl8169_private *tp, struct net_device *dev,
-			   u32 start, u32 end, gfp_t gfp)
+static u32 rtl8169_rx_fill(struct rtl8169_private *tp, u32 start, u32 end, gfp_t gfp)
 {
 	u32 cur;
 
@@ -4065,9 +4061,7 @@ static u32 rtl8169_rx_fill(struct rtl8169_private *tp, struct net_device *dev,
 		if (tp->Rx_skbuff[i])
 			continue;
 
-		skb = rtl8169_alloc_rx_skb(tp->pci_dev, dev,
-					   tp->RxDescArray + i,
-					   tp->rx_buf_sz, tp->align, gfp);
+		skb = rtl8169_alloc_rx_skb(tp, tp->RxDescArray + i, gfp);
 		if (!skb)
 			break;
 
@@ -4095,7 +4089,7 @@ static int rtl8169_init_ring(struct net_device *dev)
 	memset(tp->tx_skb, 0x0, NUM_TX_DESC * sizeof(struct ring_info));
 	memset(tp->Rx_skbuff, 0x0, NUM_RX_DESC * sizeof(struct sk_buff *));
 
-	if (rtl8169_rx_fill(tp, dev, 0, NUM_RX_DESC, GFP_KERNEL) != NUM_RX_DESC)
+	if (rtl8169_rx_fill(tp, 0, NUM_RX_DESC, GFP_KERNEL) != NUM_RX_DESC)
 		goto err_out;
 
 	rtl8169_mark_as_last_descriptor(tp->RxDescArray + NUM_RX_DESC - 1);
@@ -4615,7 +4609,7 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
 	count = cur_rx - tp->cur_rx;
 	tp->cur_rx = cur_rx;
 
-	delta = rtl8169_rx_fill(tp, dev, tp->dirty_rx, tp->cur_rx, GFP_ATOMIC);
+	delta = rtl8169_rx_fill(tp, tp->dirty_rx, tp->cur_rx, GFP_ATOMIC);
 	if (!delta && count)
 		netif_info(tp, intr, dev, "no Rx buffer allocated\n");
 	tp->dirty_rx += delta;
-- 
1.7.1


^ permalink raw reply related

* [PATCH 3/6] r8169: replace PCI_DMA_{TO,FROM}DEVICE to DMA_{TO,FROM}_DEVICE
From: Stanislaw Gruszka @ 2010-10-15 12:15 UTC (permalink / raw)
  To: Francois Romieu, netdev; +Cc: Denis Kirjanov, Stanislaw Gruszka
In-Reply-To: <1287144922-3297-1-git-send-email-sgruszka@redhat.com>

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/r8169.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 86be06c..1eafe9b 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -3984,7 +3984,7 @@ static void rtl8169_free_rx_skb(struct rtl8169_private *tp,
 	struct pci_dev *pdev = tp->pci_dev;
 
 	dma_unmap_single(&pdev->dev, le64_to_cpu(desc->addr), tp->rx_buf_sz,
-			 PCI_DMA_FROMDEVICE);
+			 DMA_FROM_DEVICE);
 	dev_kfree_skb(*sk_buff);
 	*sk_buff = NULL;
 	rtl8169_make_unusable_by_asic(desc);
@@ -4020,7 +4020,7 @@ static struct sk_buff *rtl8169_alloc_rx_skb(struct rtl8169_private *tp,
 	skb_reserve(skb, align ? ((pad - 1) & (unsigned long)skb->data) : pad);
 
 	mapping = dma_map_single(&tp->pci_dev->dev, skb->data, tp->rx_buf_sz,
-				 PCI_DMA_FROMDEVICE);
+				 DMA_FROM_DEVICE);
 	if (dma_mapping_error(&tp->pci_dev->dev, mapping))
 		goto err_free_skb_1;
 
@@ -4107,7 +4107,7 @@ static void rtl8169_unmap_tx_skb(struct pci_dev *pdev, struct ring_info *tx_skb,
 	unsigned int len = tx_skb->len;
 
 	dma_unmap_single(&pdev->dev, le64_to_cpu(desc->addr), len,
-			 PCI_DMA_TODEVICE);
+			 DMA_TO_DEVICE);
 	desc->opts1 = 0x00;
 	desc->opts2 = 0x00;
 	desc->addr = 0x00;
@@ -4257,7 +4257,7 @@ static int rtl8169_xmit_frags(struct rtl8169_private *tp, struct sk_buff *skb,
 		len = frag->size;
 		addr = ((void *) page_address(frag->page)) + frag->page_offset;
 		mapping = dma_map_single(&tp->pci_dev->dev, addr, len,
-					 PCI_DMA_TODEVICE);
+					 DMA_TO_DEVICE);
 		if (unlikely(dma_mapping_error(&tp->pci_dev->dev, mapping)))
 			goto err_out;
 
@@ -4324,7 +4324,7 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 
 	len = skb_headlen(skb);
 	mapping = dma_map_single(&tp->pci_dev->dev, skb->data, len,
-				 PCI_DMA_TODEVICE);
+				 DMA_TO_DEVICE);
 	if (unlikely(dma_mapping_error(&tp->pci_dev->dev, mapping)))
 		goto err_stop_0;
 
@@ -4505,7 +4505,7 @@ static inline bool rtl8169_try_rx_copy(struct sk_buff **sk_buff,
 		goto out;
 
 	dma_sync_single_for_cpu(&tp->pci_dev->dev, addr, pkt_size,
-				PCI_DMA_FROMDEVICE);
+				DMA_FROM_DEVICE);
 	skb_copy_from_linear_data(*sk_buff, skb->data, pkt_size);
 	*sk_buff = skb;
 	done = true;
@@ -4575,11 +4575,11 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
 
 			if (rtl8169_try_rx_copy(&skb, tp, pkt_size, addr)) {
 				dma_sync_single_for_device(&pdev->dev, addr,
-					pkt_size, PCI_DMA_FROMDEVICE);
+					pkt_size, DMA_FROM_DEVICE);
 				rtl8169_mark_to_asic(desc, tp->rx_buf_sz);
 			} else {
 				dma_unmap_single(&pdev->dev, addr, tp->rx_buf_sz,
-						 PCI_DMA_FROMDEVICE);
+						 DMA_FROM_DEVICE);
 				tp->Rx_skbuff[entry] = NULL;
 			}
 
-- 
1.7.1


^ permalink raw reply related

* [PATCH 4/6] r8169: introduce some more local variables
From: Stanislaw Gruszka @ 2010-10-15 12:15 UTC (permalink / raw)
  To: Francois Romieu, netdev; +Cc: Denis Kirjanov, Stanislaw Gruszka
In-Reply-To: <1287144922-3297-1-git-send-email-sgruszka@redhat.com>

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/r8169.c |   50 ++++++++++++++++++++++++++------------------------
 1 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 1eafe9b..f79ddb2 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -3981,9 +3981,9 @@ static inline void rtl8169_make_unusable_by_asic(struct RxDesc *desc)
 static void rtl8169_free_rx_skb(struct rtl8169_private *tp,
 				struct sk_buff **sk_buff, struct RxDesc *desc)
 {
-	struct pci_dev *pdev = tp->pci_dev;
+	struct device *d = &tp->pci_dev->dev;
 
-	dma_unmap_single(&pdev->dev, le64_to_cpu(desc->addr), tp->rx_buf_sz,
+	dma_unmap_single(d, le64_to_cpu(desc->addr), tp->rx_buf_sz,
 			 DMA_FROM_DEVICE);
 	dev_kfree_skb(*sk_buff);
 	*sk_buff = NULL;
@@ -4010,21 +4010,22 @@ static struct sk_buff *rtl8169_alloc_rx_skb(struct rtl8169_private *tp,
 {
 	struct sk_buff *skb;
 	dma_addr_t mapping;
+	struct device *d = &tp->pci_dev->dev;
+	unsigned int rx_buf_sz = tp->rx_buf_sz;
 	unsigned int align = tp->align;
 	unsigned int pad = align ? align : NET_IP_ALIGN;
 
-	skb = __netdev_alloc_skb(tp->dev, tp->rx_buf_sz + pad, gfp);
+	skb = __netdev_alloc_skb(tp->dev, rx_buf_sz + pad, gfp);
 	if (!skb)
 		goto err_out_0;
 
 	skb_reserve(skb, align ? ((pad - 1) & (unsigned long)skb->data) : pad);
 
-	mapping = dma_map_single(&tp->pci_dev->dev, skb->data, tp->rx_buf_sz,
-				 DMA_FROM_DEVICE);
-	if (dma_mapping_error(&tp->pci_dev->dev, mapping))
+	mapping = dma_map_single(d, skb->data, rx_buf_sz, DMA_FROM_DEVICE);
+	if (dma_mapping_error(d, mapping))
 		goto err_free_skb_1;
 
-	rtl8169_map_to_asic(desc, mapping, tp->rx_buf_sz);
+	rtl8169_map_to_asic(desc, mapping, rx_buf_sz);
 
 	return skb;
 
@@ -4101,13 +4102,13 @@ err_out:
 	return -ENOMEM;
 }
 
-static void rtl8169_unmap_tx_skb(struct pci_dev *pdev, struct ring_info *tx_skb,
+static void rtl8169_unmap_tx_skb(struct device *d, struct ring_info *tx_skb,
 				 struct TxDesc *desc)
 {
 	unsigned int len = tx_skb->len;
 
-	dma_unmap_single(&pdev->dev, le64_to_cpu(desc->addr), len,
-			 DMA_TO_DEVICE);
+	dma_unmap_single(d, le64_to_cpu(desc->addr), len, DMA_TO_DEVICE);
+
 	desc->opts1 = 0x00;
 	desc->opts2 = 0x00;
 	desc->addr = 0x00;
@@ -4117,6 +4118,7 @@ static void rtl8169_unmap_tx_skb(struct pci_dev *pdev, struct ring_info *tx_skb,
 static void rtl8169_tx_clear_range(struct rtl8169_private *tp, u32 start, int n)
 {
 	int i;
+	struct device *d = &tp->pci_dev->dev;
 
 	for (i = 0; i < n; i++) {
 		unsigned int entry = (start + i) % NUM_TX_DESC;
@@ -4126,8 +4128,7 @@ static void rtl8169_tx_clear_range(struct rtl8169_private *tp, u32 start, int n)
 		if (len) {
 			struct sk_buff *skb = tx_skb->skb;
 
-			rtl8169_unmap_tx_skb(tp->pci_dev, tx_skb,
-					     tp->TxDescArray + entry);
+			rtl8169_unmap_tx_skb(d, tx_skb, tp->TxDescArray + entry);
 			if (skb) {
 				dev_kfree_skb(skb);
 				tx_skb->skb = NULL;
@@ -4243,6 +4244,7 @@ static int rtl8169_xmit_frags(struct rtl8169_private *tp, struct sk_buff *skb,
 	struct skb_shared_info *info = skb_shinfo(skb);
 	unsigned int cur_frag, entry;
 	struct TxDesc * uninitialized_var(txd);
+	struct device *d = &tp->pci_dev->dev;
 
 	entry = tp->cur_tx;
 	for (cur_frag = 0; cur_frag < info->nr_frags; cur_frag++) {
@@ -4256,9 +4258,8 @@ static int rtl8169_xmit_frags(struct rtl8169_private *tp, struct sk_buff *skb,
 		txd = tp->TxDescArray + entry;
 		len = frag->size;
 		addr = ((void *) page_address(frag->page)) + frag->page_offset;
-		mapping = dma_map_single(&tp->pci_dev->dev, addr, len,
-					 DMA_TO_DEVICE);
-		if (unlikely(dma_mapping_error(&tp->pci_dev->dev, mapping)))
+		mapping = dma_map_single(d, addr, len, DMA_TO_DEVICE);
+		if (unlikely(dma_mapping_error(d, mapping)))
 			goto err_out;
 
 		/* anti gcc 2.95.3 bugware (sic) */
@@ -4309,6 +4310,7 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 	unsigned int entry = tp->cur_tx % NUM_TX_DESC;
 	struct TxDesc *txd = tp->TxDescArray + entry;
 	void __iomem *ioaddr = tp->mmio_addr;
+	struct device *d = &tp->pci_dev->dev;
 	dma_addr_t mapping;
 	u32 status, len;
 	u32 opts1;
@@ -4323,9 +4325,8 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 		goto err_stop_0;
 
 	len = skb_headlen(skb);
-	mapping = dma_map_single(&tp->pci_dev->dev, skb->data, len,
-				 DMA_TO_DEVICE);
-	if (unlikely(dma_mapping_error(&tp->pci_dev->dev, mapping)))
+	mapping = dma_map_single(d, skb->data, len, DMA_TO_DEVICE);
+	if (unlikely(dma_mapping_error(d, mapping)))
 		goto err_stop_0;
 
 	tp->tx_skb[entry].len = len;
@@ -4366,7 +4367,7 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 	return NETDEV_TX_OK;
 
 err_dma_1:
-	rtl8169_unmap_tx_skb(tp->pci_dev, tp->tx_skb + entry, txd);
+	rtl8169_unmap_tx_skb(d, tp->tx_skb + entry, txd);
 
 err_stop_0:
 	netif_stop_queue(dev);
@@ -4444,7 +4445,8 @@ static void rtl8169_tx_interrupt(struct net_device *dev,
 		dev->stats.tx_bytes += len;
 		dev->stats.tx_packets++;
 
-		rtl8169_unmap_tx_skb(tp->pci_dev, tx_skb, tp->TxDescArray + entry);
+		rtl8169_unmap_tx_skb(&tp->pci_dev->dev, tx_skb,
+				     tp->TxDescArray + entry);
 
 		if (status & LastFrag) {
 			dev_kfree_skb(tx_skb->skb);
@@ -4559,7 +4561,7 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
 			struct sk_buff *skb = tp->Rx_skbuff[entry];
 			dma_addr_t addr = le64_to_cpu(desc->addr);
 			int pkt_size = (status & 0x00001FFF) - 4;
-			struct pci_dev *pdev = tp->pci_dev;
+			struct device *d = &tp->pci_dev->dev;
 
 			/*
 			 * The driver does not support incoming fragmented
@@ -4574,11 +4576,11 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
 			}
 
 			if (rtl8169_try_rx_copy(&skb, tp, pkt_size, addr)) {
-				dma_sync_single_for_device(&pdev->dev, addr,
-					pkt_size, DMA_FROM_DEVICE);
+				dma_sync_single_for_device(d, addr, pkt_size,
+							   DMA_FROM_DEVICE);
 				rtl8169_mark_to_asic(desc, tp->rx_buf_sz);
 			} else {
-				dma_unmap_single(&pdev->dev, addr, tp->rx_buf_sz,
+				dma_unmap_single(d, addr, tp->rx_buf_sz,
 						 DMA_FROM_DEVICE);
 				tp->Rx_skbuff[entry] = NULL;
 			}
-- 
1.7.1


^ permalink raw reply related

* [PATCH 5/6] r8169: do not account fragments as packets
From: Stanislaw Gruszka @ 2010-10-15 12:15 UTC (permalink / raw)
  To: Francois Romieu, netdev; +Cc: Denis Kirjanov, Stanislaw Gruszka
In-Reply-To: <1287144922-3297-1-git-send-email-sgruszka@redhat.com>

Only increase tx_{packets,dropped} statistics when transmit or drop
full skb, not just fragment.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/r8169.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index f79ddb2..0ef49b4 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -4130,10 +4130,10 @@ static void rtl8169_tx_clear_range(struct rtl8169_private *tp, u32 start, int n)
 
 			rtl8169_unmap_tx_skb(d, tx_skb, tp->TxDescArray + entry);
 			if (skb) {
+				tp->dev->stats.tx_dropped++;
 				dev_kfree_skb(skb);
 				tx_skb->skb = NULL;
 			}
-			tp->dev->stats.tx_dropped++;
 		}
 	}
 }
@@ -4443,12 +4443,12 @@ static void rtl8169_tx_interrupt(struct net_device *dev,
 			break;
 
 		dev->stats.tx_bytes += len;
-		dev->stats.tx_packets++;
 
 		rtl8169_unmap_tx_skb(&tp->pci_dev->dev, tx_skb,
 				     tp->TxDescArray + entry);
 
 		if (status & LastFrag) {
+			dev->stats.tx_packets++;
 			dev_kfree_skb(tx_skb->skb);
 			tx_skb->skb = NULL;
 		}
-- 
1.7.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox