Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH net-next 0/5] ionic: struct cleanups
@ 2020-08-31 23:35 Shannon Nelson
  2020-08-31 23:35 ` [PATCH net-next 1/5] ionic: clean up page handling code Shannon Nelson
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Shannon Nelson @ 2020-08-31 23:35 UTC (permalink / raw)
  To: netdev, davem; +Cc: Shannon Nelson

This patchset has a few changes for better cacheline use,
to cleanup a page handling API, and to streamline the
Adminq/Notifyq queue handling.

Shannon Nelson (5):
  ionic: clean up page handling code
  ionic: smaller coalesce default
  ionic: struct reorder for faster access
  ionic: clean up desc_info and cq_info structs
  ionic: clean adminq service routine

 drivers/net/ethernet/pensando/ionic/ionic.h   |  3 -
 .../net/ethernet/pensando/ionic/ionic_dev.c   | 33 +-------
 .../net/ethernet/pensando/ionic/ionic_dev.h   | 26 +++---
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 44 +++++-----
 .../net/ethernet/pensando/ionic/ionic_lif.h   | 14 ++--
 .../net/ethernet/pensando/ionic/ionic_main.c  | 26 ------
 .../net/ethernet/pensando/ionic/ionic_txrx.c  | 81 +++++++++++--------
 7 files changed, 92 insertions(+), 135 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next 1/5] ionic: clean up page handling code
  2020-08-31 23:35 [PATCH net-next 0/5] ionic: struct cleanups Shannon Nelson
@ 2020-08-31 23:35 ` Shannon Nelson
  2020-09-01  0:14   ` David Miller
  2020-08-31 23:35 ` [PATCH net-next 2/5] ionic: smaller coalesce default Shannon Nelson
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Shannon Nelson @ 2020-08-31 23:35 UTC (permalink / raw)
  To: netdev, davem; +Cc: Shannon Nelson, Neel Patel

The internal page handling can be cleaned up by passing our
local page struct rather than dma addresses, and by putting
more of the mgmt code into the alloc and free routines.

Signed-off-by: Neel Patel <neel@pensando.io>
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 .../net/ethernet/pensando/ionic/ionic_txrx.c  | 73 +++++++++++--------
 1 file changed, 42 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index c3291decd4c3..efc02b366e73 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -100,6 +100,8 @@ static struct sk_buff *ionic_rx_frags(struct ionic_queue *q,
 		frag_len = min(len, (u16)PAGE_SIZE);
 		len -= frag_len;
 
+		dma_sync_single_for_cpu(dev, dma_unmap_addr(page_info, dma_addr),
+					len, DMA_FROM_DEVICE);
 		dma_unmap_page(dev, dma_unmap_addr(page_info, dma_addr),
 			       PAGE_SIZE, DMA_FROM_DEVICE);
 		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
@@ -266,40 +268,49 @@ void ionic_rx_flush(struct ionic_cq *cq)
 				   work_done, IONIC_INTR_CRED_RESET_COALESCE);
 }
 
-static struct page *ionic_rx_page_alloc(struct ionic_queue *q,
-					dma_addr_t *dma_addr)
+static int ionic_rx_page_alloc(struct ionic_queue *q,
+			       struct ionic_page_info *page_info)
 {
 	struct ionic_lif *lif = q->lif;
 	struct ionic_rx_stats *stats;
 	struct net_device *netdev;
 	struct device *dev;
-	struct page *page;
 
 	netdev = lif->netdev;
 	dev = lif->ionic->dev;
 	stats = q_to_rx_stats(q);
-	page = alloc_page(GFP_ATOMIC);
-	if (unlikely(!page)) {
-		net_err_ratelimited("%s: Page alloc failed on %s!\n",
+
+	if (unlikely(!page_info)) {
+		net_err_ratelimited("%s: %s invalid page_info in alloc\n",
+				    netdev->name, q->name);
+		return -EINVAL;
+	}
+
+	page_info->page = dev_alloc_page();
+	if (unlikely(!page_info->page)) {
+		net_err_ratelimited("%s: %s page alloc failed\n",
 				    netdev->name, q->name);
 		stats->alloc_err++;
-		return NULL;
+		return -ENOMEM;
 	}
 
-	*dma_addr = dma_map_page(dev, page, 0, PAGE_SIZE, DMA_FROM_DEVICE);
-	if (unlikely(dma_mapping_error(dev, *dma_addr))) {
-		__free_page(page);
-		net_err_ratelimited("%s: DMA single map failed on %s!\n",
+	page_info->dma_addr = dma_map_page(dev, page_info->page, 0, PAGE_SIZE,
+					   DMA_FROM_DEVICE);
+	if (unlikely(dma_mapping_error(dev, page_info->dma_addr))) {
+		put_page(page_info->page);
+		page_info->dma_addr = 0;
+		page_info->page = NULL;
+		net_err_ratelimited("%s: %s dma map failed\n",
 				    netdev->name, q->name);
 		stats->dma_map_err++;
-		return NULL;
+		return -EIO;
 	}
 
-	return page;
+	return 0;
 }
 
-static void ionic_rx_page_free(struct ionic_queue *q, struct page *page,
-			       dma_addr_t dma_addr)
+static void ionic_rx_page_free(struct ionic_queue *q,
+			       struct ionic_page_info *page_info)
 {
 	struct ionic_lif *lif = q->lif;
 	struct net_device *netdev;
@@ -308,15 +319,23 @@ static void ionic_rx_page_free(struct ionic_queue *q, struct page *page,
 	netdev = lif->netdev;
 	dev = lif->ionic->dev;
 
-	if (unlikely(!page)) {
-		net_err_ratelimited("%s: Trying to free unallocated buffer on %s!\n",
+	if (unlikely(!page_info)) {
+		net_err_ratelimited("%s: %s invalid page_info in free\n",
 				    netdev->name, q->name);
 		return;
 	}
 
-	dma_unmap_page(dev, dma_addr, PAGE_SIZE, DMA_FROM_DEVICE);
+	if (unlikely(!page_info->page)) {
+		net_err_ratelimited("%s: %s invalid page in free\n",
+				    netdev->name, q->name);
+		return;
+	}
 
-	__free_page(page);
+	dma_unmap_page(dev, page_info->dma_addr, PAGE_SIZE, DMA_FROM_DEVICE);
+
+	put_page(page_info->page);
+	page_info->dma_addr = 0;
+	page_info->page = NULL;
 }
 
 void ionic_rx_fill(struct ionic_queue *q)
@@ -352,8 +371,7 @@ void ionic_rx_fill(struct ionic_queue *q)
 		desc->opcode = (nfrags > 1) ? IONIC_RXQ_DESC_OPCODE_SG :
 					      IONIC_RXQ_DESC_OPCODE_SIMPLE;
 		desc_info->npages = nfrags;
-		page_info->page = ionic_rx_page_alloc(q, &page_info->dma_addr);
-		if (unlikely(!page_info->page)) {
+		if (unlikely(ionic_rx_page_alloc(q, page_info))) {
 			desc->addr = 0;
 			desc->len = 0;
 			return;
@@ -370,8 +388,7 @@ void ionic_rx_fill(struct ionic_queue *q)
 				continue;
 
 			sg_elem = &sg_desc->elems[j];
-			page_info->page = ionic_rx_page_alloc(q, &page_info->dma_addr);
-			if (unlikely(!page_info->page)) {
+			if (unlikely(ionic_rx_page_alloc(q, page_info))) {
 				sg_elem->addr = 0;
 				sg_elem->len = 0;
 				return;
@@ -409,14 +426,8 @@ void ionic_rx_empty(struct ionic_queue *q)
 		desc->addr = 0;
 		desc->len = 0;
 
-		for (i = 0; i < desc_info->npages; i++) {
-			if (likely(desc_info->pages[i].page)) {
-				ionic_rx_page_free(q, desc_info->pages[i].page,
-						   desc_info->pages[i].dma_addr);
-				desc_info->pages[i].page = NULL;
-				desc_info->pages[i].dma_addr = 0;
-			}
-		}
+		for (i = 0; i < desc_info->npages; i++)
+			ionic_rx_page_free(q, &desc_info->pages[i]);
 
 		desc_info->cb_arg = NULL;
 		idx = (idx + 1) & (q->num_descs - 1);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next 2/5] ionic: smaller coalesce default
  2020-08-31 23:35 [PATCH net-next 0/5] ionic: struct cleanups Shannon Nelson
  2020-08-31 23:35 ` [PATCH net-next 1/5] ionic: clean up page handling code Shannon Nelson
@ 2020-08-31 23:35 ` Shannon Nelson
  2020-08-31 23:50   ` Jakub Kicinski
  2020-08-31 23:35 ` [PATCH net-next 3/5] ionic: struct reorder for faster access Shannon Nelson
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Shannon Nelson @ 2020-08-31 23:35 UTC (permalink / raw)
  To: netdev, davem; +Cc: Shannon Nelson

We've found that a smaller default value for interrupt coalescing
works better for latency without hurting general operations.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/net/ethernet/pensando/ionic/ionic_dev.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index 9e2ac2b8a082..2b2eb5f2a0e5 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -16,7 +16,7 @@
 #define IONIC_DEF_TXRX_DESC		4096
 #define IONIC_LIFS_MAX			1024
 #define IONIC_WATCHDOG_SECS		5
-#define IONIC_ITR_COAL_USEC_DEFAULT	64
+#define IONIC_ITR_COAL_USEC_DEFAULT	8
 
 #define IONIC_DEV_CMD_REG_VERSION	1
 #define IONIC_DEV_INFO_REG_COUNT	32
-- 
2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next 3/5] ionic: struct reorder for faster access
  2020-08-31 23:35 [PATCH net-next 0/5] ionic: struct cleanups Shannon Nelson
  2020-08-31 23:35 ` [PATCH net-next 1/5] ionic: clean up page handling code Shannon Nelson
  2020-08-31 23:35 ` [PATCH net-next 2/5] ionic: smaller coalesce default Shannon Nelson
@ 2020-08-31 23:35 ` Shannon Nelson
  2020-08-31 23:35 ` [PATCH net-next 4/5] ionic: clean up desc_info and cq_info structs Shannon Nelson
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Shannon Nelson @ 2020-08-31 23:35 UTC (permalink / raw)
  To: netdev, davem; +Cc: Shannon Nelson

Move a few active struct fields to the front of the struct
for a little better cache use and performance.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 .../net/ethernet/pensando/ionic/ionic_dev.h    | 18 +++++++++---------
 .../net/ethernet/pensando/ionic/ionic_lif.h    | 14 +++++++-------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index 2b2eb5f2a0e5..87debc512755 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -199,16 +199,17 @@ struct ionic_desc_info {
 
 struct ionic_queue {
 	struct device *dev;
-	u64 dbell_count;
-	u64 drop;
-	u64 stop;
-	u64 wake;
 	struct ionic_lif *lif;
 	struct ionic_desc_info *info;
-	struct ionic_dev *idev;
 	u16 head_idx;
 	u16 tail_idx;
 	unsigned int index;
+	unsigned int num_descs;
+	u64 dbell_count;
+	u64 stop;
+	u64 wake;
+	u64 drop;
+	struct ionic_dev *idev;
 	unsigned int type;
 	unsigned int hw_index;
 	unsigned int hw_type;
@@ -226,7 +227,6 @@ struct ionic_queue {
 	};
 	dma_addr_t base_pa;
 	dma_addr_t sg_base_pa;
-	unsigned int num_descs;
 	unsigned int desc_size;
 	unsigned int sg_desc_size;
 	unsigned int pid;
@@ -246,8 +246,6 @@ struct ionic_intr_info {
 };
 
 struct ionic_cq {
-	void *base;
-	dma_addr_t base_pa;
 	struct ionic_lif *lif;
 	struct ionic_cq_info *info;
 	struct ionic_queue *bound_q;
@@ -255,8 +253,10 @@ struct ionic_cq {
 	u16 tail_idx;
 	bool done_color;
 	unsigned int num_descs;
-	u64 compl_count;
 	unsigned int desc_size;
+	u64 compl_count;
+	void *base;
+	dma_addr_t base_pa;
 };
 
 struct ionic;
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
index e1e6ff1a0918..11ea9e0c6a4a 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
@@ -16,32 +16,32 @@
 #define IONIC_TX_BUDGET_DEFAULT		256
 
 struct ionic_tx_stats {
-	u64 dma_map_err;
 	u64 pkts;
 	u64 bytes;
-	u64 clean;
-	u64 linearize;
 	u64 csum_none;
 	u64 csum;
-	u64 crc32_csum;
 	u64 tso;
 	u64 tso_bytes;
 	u64 frags;
 	u64 vlan_inserted;
+	u64 clean;
+	u64 linearize;
+	u64 crc32_csum;
 	u64 sg_cntr[IONIC_MAX_NUM_SG_CNTR];
+	u64 dma_map_err;
 };
 
 struct ionic_rx_stats {
-	u64 dma_map_err;
-	u64 alloc_err;
 	u64 pkts;
 	u64 bytes;
 	u64 csum_none;
 	u64 csum_complete;
-	u64 csum_error;
 	u64 buffers_posted;
 	u64 dropped;
 	u64 vlan_stripped;
+	u64 csum_error;
+	u64 dma_map_err;
+	u64 alloc_err;
 };
 
 #define IONIC_QCQ_F_INITED		BIT(0)
-- 
2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next 4/5] ionic: clean up desc_info and cq_info structs
  2020-08-31 23:35 [PATCH net-next 0/5] ionic: struct cleanups Shannon Nelson
                   ` (2 preceding siblings ...)
  2020-08-31 23:35 ` [PATCH net-next 3/5] ionic: struct reorder for faster access Shannon Nelson
@ 2020-08-31 23:35 ` Shannon Nelson
  2020-08-31 23:35 ` [PATCH net-next 5/5] ionic: clean adminq service routine Shannon Nelson
  2020-09-01  0:47 ` [PATCH net-next 0/5] ionic: struct cleanups Florian Fainelli
  5 siblings, 0 replies; 13+ messages in thread
From: Shannon Nelson @ 2020-08-31 23:35 UTC (permalink / raw)
  To: netdev, davem; +Cc: Shannon Nelson, Neel Patel

Remove some unnecessary struct fields and related code.

Signed-off-by: Neel Patel <neel@pensando.io>
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 .../net/ethernet/pensando/ionic/ionic_dev.c   | 33 ++-----------------
 .../net/ethernet/pensando/ionic/ionic_dev.h   |  6 ----
 .../net/ethernet/pensando/ionic/ionic_txrx.c  |  8 +++--
 3 files changed, 8 insertions(+), 39 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.c b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
index 3645673b4b18..6068f51a11d9 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
@@ -467,9 +467,7 @@ int ionic_cq_init(struct ionic_lif *lif, struct ionic_cq *cq,
 		  struct ionic_intr_info *intr,
 		  unsigned int num_descs, size_t desc_size)
 {
-	struct ionic_cq_info *cur;
 	unsigned int ring_size;
-	unsigned int i;
 
 	if (desc_size == 0 || !is_power_of_2(num_descs))
 		return -EINVAL;
@@ -485,19 +483,6 @@ int ionic_cq_init(struct ionic_lif *lif, struct ionic_cq *cq,
 	cq->tail_idx = 0;
 	cq->done_color = 1;
 
-	cur = cq->info;
-
-	for (i = 0; i < num_descs; i++) {
-		if (i + 1 == num_descs) {
-			cur->next = cq->info;
-			cur->last = true;
-		} else {
-			cur->next = cur + 1;
-		}
-		cur->index = i;
-		cur++;
-	}
-
 	return 0;
 }
 
@@ -551,9 +536,7 @@ int ionic_q_init(struct ionic_lif *lif, struct ionic_dev *idev,
 		 unsigned int num_descs, size_t desc_size,
 		 size_t sg_desc_size, unsigned int pid)
 {
-	struct ionic_desc_info *cur;
 	unsigned int ring_size;
-	unsigned int i;
 
 	if (desc_size == 0 || !is_power_of_2(num_descs))
 		return -EINVAL;
@@ -574,18 +557,6 @@ int ionic_q_init(struct ionic_lif *lif, struct ionic_dev *idev,
 
 	snprintf(q->name, sizeof(q->name), "L%d-%s%u", lif->index, name, index);
 
-	cur = q->info;
-
-	for (i = 0; i < num_descs; i++) {
-		if (i + 1 == num_descs)
-			cur->next = q->info;
-		else
-			cur->next = cur + 1;
-		cur->index = i;
-		cur->left = num_descs - i;
-		cur++;
-	}
-
 	return 0;
 }
 
@@ -652,6 +623,7 @@ void ionic_q_service(struct ionic_queue *q, struct ionic_cq_info *cq_info,
 	struct ionic_desc_info *desc_info;
 	ionic_desc_cb cb;
 	void *cb_arg;
+	u16 index;
 
 	/* check for empty queue */
 	if (q->tail_idx == q->head_idx)
@@ -665,6 +637,7 @@ void ionic_q_service(struct ionic_queue *q, struct ionic_cq_info *cq_info,
 
 	do {
 		desc_info = &q->info[q->tail_idx];
+		index = q->tail_idx;
 		q->tail_idx = (q->tail_idx + 1) & (q->num_descs - 1);
 
 		cb = desc_info->cb;
@@ -675,5 +648,5 @@ void ionic_q_service(struct ionic_queue *q, struct ionic_cq_info *cq_info,
 
 		if (cb)
 			cb(q, desc_info, cq_info, cb_arg);
-	} while (desc_info->index != stop_index);
+	} while (index != stop_index);
 }
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index 87debc512755..21c3ce9ee446 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -156,9 +156,6 @@ struct ionic_cq_info {
 		struct ionic_admin_comp *admincq;
 		struct ionic_notifyq_event *notifyq;
 	};
-	struct ionic_cq_info *next;
-	unsigned int index;
-	bool last;
 };
 
 struct ionic_queue;
@@ -186,9 +183,6 @@ struct ionic_desc_info {
 		struct ionic_txq_sg_desc *txq_sg_desc;
 		struct ionic_rxq_sg_desc *rxq_sgl_desc;
 	};
-	struct ionic_desc_info *next;
-	unsigned int index;
-	unsigned int left;
 	unsigned int npages;
 	struct ionic_page_info pages[IONIC_RX_MAX_SG_ELEMS + 1];
 	ionic_desc_cb cb;
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index efc02b366e73..8ba14792616f 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -240,10 +240,10 @@ static bool ionic_rx_service(struct ionic_cq *cq, struct ionic_cq_info *cq_info)
 	if (q->tail_idx == q->head_idx)
 		return false;
 
-	desc_info = &q->info[q->tail_idx];
-	if (desc_info->index != le16_to_cpu(comp->comp_index))
+	if (q->tail_idx != le16_to_cpu(comp->comp_index))
 		return false;
 
+	desc_info = &q->info[q->tail_idx];
 	q->tail_idx = (q->tail_idx + 1) & (q->num_descs - 1);
 
 	/* clean the related q entry, only one per qc completion */
@@ -637,6 +637,7 @@ static bool ionic_tx_service(struct ionic_cq *cq, struct ionic_cq_info *cq_info)
 	struct ionic_txq_comp *comp = cq_info->cq_desc;
 	struct ionic_queue *q = cq->bound_q;
 	struct ionic_desc_info *desc_info;
+	u16 index;
 
 	if (!color_match(comp->color, cq->done_color))
 		return false;
@@ -646,11 +647,12 @@ static bool ionic_tx_service(struct ionic_cq *cq, struct ionic_cq_info *cq_info)
 	 */
 	do {
 		desc_info = &q->info[q->tail_idx];
+		index = q->tail_idx;
 		q->tail_idx = (q->tail_idx + 1) & (q->num_descs - 1);
 		ionic_tx_clean(q, desc_info, cq_info, desc_info->cb_arg);
 		desc_info->cb = NULL;
 		desc_info->cb_arg = NULL;
-	} while (desc_info->index != le16_to_cpu(comp->comp_index));
+	} while (index != le16_to_cpu(comp->comp_index));
 
 	return true;
 }
-- 
2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next 5/5] ionic: clean adminq service routine
  2020-08-31 23:35 [PATCH net-next 0/5] ionic: struct cleanups Shannon Nelson
                   ` (3 preceding siblings ...)
  2020-08-31 23:35 ` [PATCH net-next 4/5] ionic: clean up desc_info and cq_info structs Shannon Nelson
@ 2020-08-31 23:35 ` Shannon Nelson
  2020-09-01  0:47 ` [PATCH net-next 0/5] ionic: struct cleanups Florian Fainelli
  5 siblings, 0 replies; 13+ messages in thread
From: Shannon Nelson @ 2020-08-31 23:35 UTC (permalink / raw)
  To: netdev, davem; +Cc: Shannon Nelson, Neel Patel

The only thing calling ionic_napi any more is the adminq
processing, so combine and simplify.

Signed-off-by: Neel Patel <neel@pensando.io>
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/net/ethernet/pensando/ionic/ionic.h   |  3 --
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 44 +++++++++++--------
 .../net/ethernet/pensando/ionic/ionic_main.c  | 26 -----------
 3 files changed, 25 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
index f699ed19eb4f..084a924431d5 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic.h
@@ -64,9 +64,6 @@ struct ionic_admin_ctx {
 	union ionic_adminq_comp comp;
 };
 
-int ionic_napi(struct napi_struct *napi, int budget, ionic_cq_cb cb,
-	       ionic_cq_done_cb done_cb, void *done_arg);
-
 int ionic_adminq_post_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx);
 int ionic_dev_cmd_wait(struct ionic *ionic, unsigned long max_wait);
 int ionic_set_dma_mask(struct ionic *ionic);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index eeaa73650986..ee683cb142a8 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -800,21 +800,6 @@ static bool ionic_notifyq_service(struct ionic_cq *cq,
 	return true;
 }
 
-static int ionic_notifyq_clean(struct ionic_lif *lif, int budget)
-{
-	struct ionic_dev *idev = &lif->ionic->idev;
-	struct ionic_cq *cq = &lif->notifyqcq->cq;
-	u32 work_done;
-
-	work_done = ionic_cq_service(cq, budget, ionic_notifyq_service,
-				     NULL, NULL);
-	if (work_done)
-		ionic_intr_credits(idev->intr_ctrl, cq->bound_intr->index,
-				   work_done, IONIC_INTR_CRED_RESET_COALESCE);
-
-	return work_done;
-}
-
 static bool ionic_adminq_service(struct ionic_cq *cq,
 				 struct ionic_cq_info *cq_info)
 {
@@ -830,15 +815,36 @@ static bool ionic_adminq_service(struct ionic_cq *cq,
 
 static int ionic_adminq_napi(struct napi_struct *napi, int budget)
 {
+	struct ionic_intr_info *intr = napi_to_cq(napi)->bound_intr;
 	struct ionic_lif *lif = napi_to_cq(napi)->lif;
+	struct ionic_dev *idev = &lif->ionic->idev;
+	unsigned int flags = 0;
 	int n_work = 0;
 	int a_work = 0;
+	int work_done;
+
+	if (lif->notifyqcq && lif->notifyqcq->flags & IONIC_QCQ_F_INITED)
+		n_work = ionic_cq_service(&lif->notifyqcq->cq, budget,
+					  ionic_notifyq_service, NULL, NULL);
 
-	if (likely(lif->notifyqcq && lif->notifyqcq->flags & IONIC_QCQ_F_INITED))
-		n_work = ionic_notifyq_clean(lif, budget);
-	a_work = ionic_napi(napi, budget, ionic_adminq_service, NULL, NULL);
+	if (lif->adminqcq && lif->adminqcq->flags & IONIC_QCQ_F_INITED)
+		a_work = ionic_cq_service(&lif->adminqcq->cq, budget,
+					  ionic_adminq_service, NULL, NULL);
+
+	work_done = max(n_work, a_work);
+	if (work_done < budget && napi_complete_done(napi, work_done)) {
+		flags |= IONIC_INTR_CRED_UNMASK;
+		DEBUG_STATS_INTR_REARM(intr);
+	}
 
-	return max(n_work, a_work);
+	if (work_done || flags) {
+		flags |= IONIC_INTR_CRED_RESET_COALESCE;
+		ionic_intr_credits(idev->intr_ctrl,
+				   intr->index,
+				   n_work + a_work, flags);
+	}
+
+	return work_done;
 }
 
 void ionic_get_stats64(struct net_device *netdev,
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 2b72a51be1d0..cfb90bf605fe 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -305,32 +305,6 @@ int ionic_adminq_post_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx)
 	return ionic_adminq_check_err(lif, ctx, (remaining == 0));
 }
 
-int ionic_napi(struct napi_struct *napi, int budget, ionic_cq_cb cb,
-	       ionic_cq_done_cb done_cb, void *done_arg)
-{
-	struct ionic_qcq *qcq = napi_to_qcq(napi);
-	struct ionic_cq *cq = &qcq->cq;
-	u32 work_done, flags = 0;
-
-	work_done = ionic_cq_service(cq, budget, cb, done_cb, done_arg);
-
-	if (work_done < budget && napi_complete_done(napi, work_done)) {
-		flags |= IONIC_INTR_CRED_UNMASK;
-		DEBUG_STATS_INTR_REARM(cq->bound_intr);
-	}
-
-	if (work_done || flags) {
-		flags |= IONIC_INTR_CRED_RESET_COALESCE;
-		ionic_intr_credits(cq->lif->ionic->idev.intr_ctrl,
-				   cq->bound_intr->index,
-				   work_done, flags);
-	}
-
-	DEBUG_STATS_NAPI_POLL(qcq, work_done);
-
-	return work_done;
-}
-
 static void ionic_dev_cmd_clean(struct ionic *ionic)
 {
 	union ionic_dev_cmd_regs *regs = ionic->idev.dev_cmd_regs;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 2/5] ionic: smaller coalesce default
  2020-08-31 23:35 ` [PATCH net-next 2/5] ionic: smaller coalesce default Shannon Nelson
@ 2020-08-31 23:50   ` Jakub Kicinski
  2020-09-01  0:16     ` David Miller
  0 siblings, 1 reply; 13+ messages in thread
From: Jakub Kicinski @ 2020-08-31 23:50 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev, davem

On Mon, 31 Aug 2020 16:35:55 -0700 Shannon Nelson wrote:
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> index 9e2ac2b8a082..2b2eb5f2a0e5 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> @@ -16,7 +16,7 @@
>  #define IONIC_DEF_TXRX_DESC		4096
>  #define IONIC_LIFS_MAX			1024
>  #define IONIC_WATCHDOG_SECS		5
> -#define IONIC_ITR_COAL_USEC_DEFAULT	64
> +#define IONIC_ITR_COAL_USEC_DEFAULT	8

8 us interrupt coalescing does not hurt general operations?! No way.

It's your customers who'll get hurt here, so your call, but I seriously
doubt this. Unless the unit is not usec?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 1/5] ionic: clean up page handling code
  2020-08-31 23:35 ` [PATCH net-next 1/5] ionic: clean up page handling code Shannon Nelson
@ 2020-09-01  0:14   ` David Miller
  2020-09-01  4:19     ` Shannon Nelson
  0 siblings, 1 reply; 13+ messages in thread
From: David Miller @ 2020-09-01  0:14 UTC (permalink / raw)
  To: snelson; +Cc: netdev, neel

From: Shannon Nelson <snelson@pensando.io>
Date: Mon, 31 Aug 2020 16:35:54 -0700

> @@ -100,6 +100,8 @@ static struct sk_buff *ionic_rx_frags(struct ionic_queue *q,
>  		frag_len = min(len, (u16)PAGE_SIZE);
>  		len -= frag_len;
>  
> +		dma_sync_single_for_cpu(dev, dma_unmap_addr(page_info, dma_addr),
> +					len, DMA_FROM_DEVICE);
>  		dma_unmap_page(dev, dma_unmap_addr(page_info, dma_addr),
>  			       PAGE_SIZE, DMA_FROM_DEVICE);
>  		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,

The unmap operation performs a sync, if necessary, for you.

That's the pattern of usage:

	map();
	device read/write memory
	unmap();

That's it, no more, no less.

The time to use sync is when you want to maintain the mapping and keep
using it.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 2/5] ionic: smaller coalesce default
  2020-08-31 23:50   ` Jakub Kicinski
@ 2020-09-01  0:16     ` David Miller
  2020-09-01  4:20       ` Shannon Nelson
  0 siblings, 1 reply; 13+ messages in thread
From: David Miller @ 2020-09-01  0:16 UTC (permalink / raw)
  To: kuba; +Cc: snelson, netdev

From: Jakub Kicinski <kuba@kernel.org>
Date: Mon, 31 Aug 2020 16:50:54 -0700

> On Mon, 31 Aug 2020 16:35:55 -0700 Shannon Nelson wrote:
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
>> index 9e2ac2b8a082..2b2eb5f2a0e5 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
>> @@ -16,7 +16,7 @@
>>  #define IONIC_DEF_TXRX_DESC		4096
>>  #define IONIC_LIFS_MAX			1024
>>  #define IONIC_WATCHDOG_SECS		5
>> -#define IONIC_ITR_COAL_USEC_DEFAULT	64
>> +#define IONIC_ITR_COAL_USEC_DEFAULT	8
> 
> 8 us interrupt coalescing does not hurt general operations?! No way.
> 
> It's your customers who'll get hurt here, so your call, but I seriously
> doubt this. Unless the unit is not usec?

Agreed, 8usec is really really low.  You won't get much coalescing during
bulk transfers with a value like that, eliminating the gain from coalescing
in the first place.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 0/5] ionic: struct cleanups
  2020-08-31 23:35 [PATCH net-next 0/5] ionic: struct cleanups Shannon Nelson
                   ` (4 preceding siblings ...)
  2020-08-31 23:35 ` [PATCH net-next 5/5] ionic: clean adminq service routine Shannon Nelson
@ 2020-09-01  0:47 ` Florian Fainelli
  2020-09-01  4:21   ` Shannon Nelson
  5 siblings, 1 reply; 13+ messages in thread
From: Florian Fainelli @ 2020-09-01  0:47 UTC (permalink / raw)
  To: Shannon Nelson, netdev, davem



On 8/31/2020 4:35 PM, Shannon Nelson wrote:
> This patchset has a few changes for better cacheline use,
> to cleanup a page handling API, and to streamline the
> Adminq/Notifyq queue handling.

Some other non technical changes, the changes that Neel in the 
Signed-off-by should also have a From that matches his Signed-off-by, or 
you should use Co-developped-by, or some variant of it.

> 
> Shannon Nelson (5):
>    ionic: clean up page handling code
>    ionic: smaller coalesce default
>    ionic: struct reorder for faster access
>    ionic: clean up desc_info and cq_info structs
>    ionic: clean adminq service routine
> 
>   drivers/net/ethernet/pensando/ionic/ionic.h   |  3 -
>   .../net/ethernet/pensando/ionic/ionic_dev.c   | 33 +-------
>   .../net/ethernet/pensando/ionic/ionic_dev.h   | 26 +++---
>   .../net/ethernet/pensando/ionic/ionic_lif.c   | 44 +++++-----
>   .../net/ethernet/pensando/ionic/ionic_lif.h   | 14 ++--
>   .../net/ethernet/pensando/ionic/ionic_main.c  | 26 ------
>   .../net/ethernet/pensando/ionic/ionic_txrx.c  | 81 +++++++++++--------
>   7 files changed, 92 insertions(+), 135 deletions(-)
> 

-- 
Florian

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 1/5] ionic: clean up page handling code
  2020-09-01  0:14   ` David Miller
@ 2020-09-01  4:19     ` Shannon Nelson
  0 siblings, 0 replies; 13+ messages in thread
From: Shannon Nelson @ 2020-09-01  4:19 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, neel



On 8/31/20 5:14 PM, David Miller wrote:
> From: Shannon Nelson <snelson@pensando.io>
> Date: Mon, 31 Aug 2020 16:35:54 -0700
>
>> @@ -100,6 +100,8 @@ static struct sk_buff *ionic_rx_frags(struct ionic_queue *q,
>>   		frag_len = min(len, (u16)PAGE_SIZE);
>>   		len -= frag_len;
>>   
>> +		dma_sync_single_for_cpu(dev, dma_unmap_addr(page_info, dma_addr),
>> +					len, DMA_FROM_DEVICE);
>>   		dma_unmap_page(dev, dma_unmap_addr(page_info, dma_addr),
>>   			       PAGE_SIZE, DMA_FROM_DEVICE);
>>   		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
> The unmap operation performs a sync, if necessary, for you.
>
> That's the pattern of usage:
>
> 	map();
> 	device read/write memory
> 	unmap();
>
> That's it, no more, no less.
>
> The time to use sync is when you want to maintain the mapping and keep
> using it.

Thanks, I'll drop that part.
sln

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 2/5] ionic: smaller coalesce default
  2020-09-01  0:16     ` David Miller
@ 2020-09-01  4:20       ` Shannon Nelson
  0 siblings, 0 replies; 13+ messages in thread
From: Shannon Nelson @ 2020-09-01  4:20 UTC (permalink / raw)
  To: David Miller, kuba; +Cc: netdev

On 8/31/20 5:16 PM, David Miller wrote:
> From: Jakub Kicinski <kuba@kernel.org>
> Date: Mon, 31 Aug 2020 16:50:54 -0700
>
>> On Mon, 31 Aug 2020 16:35:55 -0700 Shannon Nelson wrote:
>>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
>>> index 9e2ac2b8a082..2b2eb5f2a0e5 100644
>>> --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
>>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
>>> @@ -16,7 +16,7 @@
>>>   #define IONIC_DEF_TXRX_DESC		4096
>>>   #define IONIC_LIFS_MAX			1024
>>>   #define IONIC_WATCHDOG_SECS		5
>>> -#define IONIC_ITR_COAL_USEC_DEFAULT	64
>>> +#define IONIC_ITR_COAL_USEC_DEFAULT	8
>> 8 us interrupt coalescing does not hurt general operations?! No way.
>>
>> It's your customers who'll get hurt here, so your call, but I seriously
>> doubt this. Unless the unit is not usec?
> Agreed, 8usec is really really low.  You won't get much coalescing during
> bulk transfers with a value like that, eliminating the gain from coalescing
> in the first place.

Thanks.  I'll drop this patch and come back to this issue when we get a 
chance to add adaptive coalescing.

sln



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 0/5] ionic: struct cleanups
  2020-09-01  0:47 ` [PATCH net-next 0/5] ionic: struct cleanups Florian Fainelli
@ 2020-09-01  4:21   ` Shannon Nelson
  0 siblings, 0 replies; 13+ messages in thread
From: Shannon Nelson @ 2020-09-01  4:21 UTC (permalink / raw)
  To: Florian Fainelli, netdev, davem

On 8/31/20 5:47 PM, Florian Fainelli wrote:
>
> On 8/31/2020 4:35 PM, Shannon Nelson wrote:
>> This patchset has a few changes for better cacheline use,
>> to cleanup a page handling API, and to streamline the
>> Adminq/Notifyq queue handling.
>
> Some other non technical changes, the changes that Neel in the 
> Signed-off-by should also have a From that matches his Signed-off-by, 
> or you should use Co-developped-by, or some variant of it.

Thanks.  For now, I'll go back to using co-developed.
sln



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-09-01  4:22 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-31 23:35 [PATCH net-next 0/5] ionic: struct cleanups Shannon Nelson
2020-08-31 23:35 ` [PATCH net-next 1/5] ionic: clean up page handling code Shannon Nelson
2020-09-01  0:14   ` David Miller
2020-09-01  4:19     ` Shannon Nelson
2020-08-31 23:35 ` [PATCH net-next 2/5] ionic: smaller coalesce default Shannon Nelson
2020-08-31 23:50   ` Jakub Kicinski
2020-09-01  0:16     ` David Miller
2020-09-01  4:20       ` Shannon Nelson
2020-08-31 23:35 ` [PATCH net-next 3/5] ionic: struct reorder for faster access Shannon Nelson
2020-08-31 23:35 ` [PATCH net-next 4/5] ionic: clean up desc_info and cq_info structs Shannon Nelson
2020-08-31 23:35 ` [PATCH net-next 5/5] ionic: clean adminq service routine Shannon Nelson
2020-09-01  0:47 ` [PATCH net-next 0/5] ionic: struct cleanups Florian Fainelli
2020-09-01  4:21   ` Shannon Nelson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).