netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net-next 00/12] net: get rid of __napi_complete()
@ 2017-02-04 23:24 Eric Dumazet
  2017-02-04 23:24 ` [PATCH v2 net-next 01/12] 8139too: use napi_complete_done() Eric Dumazet
                   ` (12 more replies)
  0 siblings, 13 replies; 19+ messages in thread
From: Eric Dumazet @ 2017-02-04 23:24 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

This patch series removes __napi_complete() calls, in an effort
to make NAPI API simpler and generalize GRO and napi_complete_done()

Eric Dumazet (12):
  8139too: use napi_complete_done()
  8139cp: use napi_complete_done()
  epic100: use napi_complete_done()
  amd8111e: add GRO support
  pcnet32: use napi_complete_done()
  ep93xx_eth: add GRO support
  skge: use napi_complete_done()
  ks8695net: add GRO support
  qla3xxx: add GRO support
  ibm/emac: use napi_complete_done()
  aeroflex/greth: use napi_complete_done()
  net: remove __napi_complete()

 drivers/net/ethernet/aeroflex/greth.c    |   2 +-
 drivers/net/ethernet/amd/amd8111e.c      | 164 ++++++++++++++-----------------
 drivers/net/ethernet/amd/pcnet32.c       |  11 +--
 drivers/net/ethernet/cirrus/ep93xx_eth.c |  29 ++----
 drivers/net/ethernet/ibm/emac/mal.c      |  18 ++--
 drivers/net/ethernet/marvell/skge.c      |   8 +-
 drivers/net/ethernet/micrel/ks8695net.c  |  11 +--
 drivers/net/ethernet/qlogic/qla3xxx.c    |  31 +++---
 drivers/net/ethernet/realtek/8139cp.c    |  11 +--
 drivers/net/ethernet/realtek/8139too.c   |   8 +-
 drivers/net/ethernet/smsc/epic100.c      |  31 ++----
 include/linux/netdevice.h                |   1 -
 net/core/dev.c                           |  24 +----
 13 files changed, 127 insertions(+), 222 deletions(-)

-- 
2.11.0.483.g087da7b7c-goog

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

* [PATCH v2 net-next 01/12] 8139too: use napi_complete_done()
  2017-02-04 23:24 [PATCH v2 net-next 00/12] net: get rid of __napi_complete() Eric Dumazet
@ 2017-02-04 23:24 ` Eric Dumazet
  2017-02-04 23:24 ` [PATCH v2 net-next 02/12] 8139cp: " Eric Dumazet
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Eric Dumazet @ 2017-02-04 23:24 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

Use napi_complete_done() instead of __napi_complete() to :

1) Get support of gro_flush_timeout if opt-in
2) Not rearm interrupts for busy-polling users.
3) use standard NAPI API.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/ethernet/realtek/8139too.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/realtek/8139too.c b/drivers/net/ethernet/realtek/8139too.c
index 5ad59c6d29a46a036a6b1d70e77ae288d9fe..89631753e79962d91456d93b71929af76891 100644
--- a/drivers/net/ethernet/realtek/8139too.c
+++ b/drivers/net/ethernet/realtek/8139too.c
@@ -2135,14 +2135,10 @@ static int rtl8139_poll(struct napi_struct *napi, int budget)
 	if (likely(RTL_R16(IntrStatus) & RxAckBits))
 		work_done += rtl8139_rx(dev, tp, budget);
 
-	if (work_done < budget) {
+	if (work_done < budget && napi_complete_done(napi, work_done)) {
 		unsigned long flags;
-		/*
-		 * Order is important since data can get interrupted
-		 * again when we think we are done.
-		 */
+
 		spin_lock_irqsave(&tp->lock, flags);
-		__napi_complete(napi);
 		RTL_W16_F(IntrMask, rtl8139_intr_mask);
 		spin_unlock_irqrestore(&tp->lock, flags);
 	}
-- 
2.11.0.483.g087da7b7c-goog

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

* [PATCH v2 net-next 02/12] 8139cp: use napi_complete_done()
  2017-02-04 23:24 [PATCH v2 net-next 00/12] net: get rid of __napi_complete() Eric Dumazet
  2017-02-04 23:24 ` [PATCH v2 net-next 01/12] 8139too: use napi_complete_done() Eric Dumazet
@ 2017-02-04 23:24 ` Eric Dumazet
  2017-02-04 23:24 ` [PATCH v2 net-next 03/12] epic100: " Eric Dumazet
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Eric Dumazet @ 2017-02-04 23:24 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

Use napi_complete_done() instead of __napi_complete() to :

1) Get support of gro_flush_timeout if opt-in
2) Not rearm interrupts for busy-polling users.
3) use standard NAPI API.
4) Eventually get rid of napi_gro_flush() in the future.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/ethernet/realtek/8139cp.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
index 0b3cd58093d5ecf00abef6eda1386f13ac5e..672f6b696069ad8b47989ecb1dd03d3a81fd 100644
--- a/drivers/net/ethernet/realtek/8139cp.c
+++ b/drivers/net/ethernet/realtek/8139cp.c
@@ -465,10 +465,8 @@ static int cp_rx_poll(struct napi_struct *napi, int budget)
 	struct cp_private *cp = container_of(napi, struct cp_private, napi);
 	struct net_device *dev = cp->dev;
 	unsigned int rx_tail = cp->rx_tail;
-	int rx;
+	int rx = 0;
 
-	rx = 0;
-rx_status_loop:
 	cpw16(IntrStatus, cp_rx_intr_mask);
 
 	while (rx < budget) {
@@ -556,15 +554,10 @@ static int cp_rx_poll(struct napi_struct *napi, int budget)
 	/* if we did not reach work limit, then we're done with
 	 * this round of polling
 	 */
-	if (rx < budget) {
+	if (rx < budget && napi_complete_done(napi, rx)) {
 		unsigned long flags;
 
-		if (cpr16(IntrStatus) & cp_rx_intr_mask)
-			goto rx_status_loop;
-
-		napi_gro_flush(napi, false);
 		spin_lock_irqsave(&cp->lock, flags);
-		__napi_complete(napi);
 		cpw16_f(IntrMask, cp_intr_mask);
 		spin_unlock_irqrestore(&cp->lock, flags);
 	}
-- 
2.11.0.483.g087da7b7c-goog

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

* [PATCH v2 net-next 03/12] epic100: use napi_complete_done()
  2017-02-04 23:24 [PATCH v2 net-next 00/12] net: get rid of __napi_complete() Eric Dumazet
  2017-02-04 23:24 ` [PATCH v2 net-next 01/12] 8139too: use napi_complete_done() Eric Dumazet
  2017-02-04 23:24 ` [PATCH v2 net-next 02/12] 8139cp: " Eric Dumazet
@ 2017-02-04 23:24 ` Eric Dumazet
  2017-02-04 23:24 ` [PATCH v2 net-next 04/12] amd8111e: add GRO support Eric Dumazet
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Eric Dumazet @ 2017-02-04 23:24 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

Use napi_complete_done() instead of __napi_complete() to :

1) Get support of gro_flush_timeout if opt-in
2) Not rearm interrupts for busy-polling users.
3) use standard NAPI API.
4) get rid of baroque code and ease maintenance.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/ethernet/smsc/epic100.c | 31 +++++++------------------------
 1 file changed, 7 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/smsc/epic100.c b/drivers/net/ethernet/smsc/epic100.c
index 55a95e1d69d68b45ec85a8648e85d24e3673..5f2737189c724eea48ffdd8d8a48f520a209 100644
--- a/drivers/net/ethernet/smsc/epic100.c
+++ b/drivers/net/ethernet/smsc/epic100.c
@@ -264,7 +264,6 @@ struct epic_private {
 	spinlock_t lock;				/* Group with Tx control cache line. */
 	spinlock_t napi_lock;
 	struct napi_struct napi;
-	unsigned int reschedule_in_poll;
 	unsigned int cur_tx, dirty_tx;
 
 	unsigned int cur_rx, dirty_rx;
@@ -400,7 +399,6 @@ static int epic_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	spin_lock_init(&ep->lock);
 	spin_lock_init(&ep->napi_lock);
-	ep->reschedule_in_poll = 0;
 
 	/* Bring the chip out of low-power mode. */
 	ew32(GENCTL, 0x4200);
@@ -1086,13 +1084,12 @@ static irqreturn_t epic_interrupt(int irq, void *dev_instance)
 
 	handled = 1;
 
-	if ((status & EpicNapiEvent) && !ep->reschedule_in_poll) {
+	if (status & EpicNapiEvent) {
 		spin_lock(&ep->napi_lock);
 		if (napi_schedule_prep(&ep->napi)) {
 			epic_napi_irq_off(dev, ep);
 			__napi_schedule(&ep->napi);
-		} else
-			ep->reschedule_in_poll++;
+		}
 		spin_unlock(&ep->napi_lock);
 	}
 	status &= ~EpicNapiEvent;
@@ -1248,37 +1245,23 @@ static int epic_poll(struct napi_struct *napi, int budget)
 {
 	struct epic_private *ep = container_of(napi, struct epic_private, napi);
 	struct net_device *dev = ep->mii.dev;
-	int work_done = 0;
 	void __iomem *ioaddr = ep->ioaddr;
-
-rx_action:
+	int work_done;
 
 	epic_tx(dev, ep);
 
-	work_done += epic_rx(dev, budget);
+	work_done = epic_rx(dev, budget);
 
 	epic_rx_err(dev, ep);
 
-	if (work_done < budget) {
+	if (work_done < budget && napi_complete_done(napi, work_done)) {
 		unsigned long flags;
-		int more;
-
-		/* A bit baroque but it avoids a (space hungry) spin_unlock */
 
 		spin_lock_irqsave(&ep->napi_lock, flags);
 
-		more = ep->reschedule_in_poll;
-		if (!more) {
-			__napi_complete(napi);
-			ew32(INTSTAT, EpicNapiEvent);
-			epic_napi_irq_on(dev, ep);
-		} else
-			ep->reschedule_in_poll--;
-
+		ew32(INTSTAT, EpicNapiEvent);
+		epic_napi_irq_on(dev, ep);
 		spin_unlock_irqrestore(&ep->napi_lock, flags);
-
-		if (more)
-			goto rx_action;
 	}
 
 	return work_done;
-- 
2.11.0.483.g087da7b7c-goog

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

* [PATCH v2 net-next 04/12] amd8111e: add GRO support
  2017-02-04 23:24 [PATCH v2 net-next 00/12] net: get rid of __napi_complete() Eric Dumazet
                   ` (2 preceding siblings ...)
  2017-02-04 23:24 ` [PATCH v2 net-next 03/12] epic100: " Eric Dumazet
@ 2017-02-04 23:24 ` Eric Dumazet
  2017-02-04 23:24 ` [PATCH v2 net-next 05/12] pcnet32: use napi_complete_done() Eric Dumazet
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Eric Dumazet @ 2017-02-04 23:24 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

Use napi_complete_done() instead of __napi_complete() to :

1) Get support of gro_flush_timeout if opt-in
2) Not rearm interrupts for busy-polling users.
3) use standard NAPI API.
4) get rid of baroque code and ease maintenance.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/ethernet/amd/amd8111e.c | 164 ++++++++++++++++--------------------
 1 file changed, 72 insertions(+), 92 deletions(-)

diff --git a/drivers/net/ethernet/amd/amd8111e.c b/drivers/net/ethernet/amd/amd8111e.c
index 9595f1bc535b73306720e4d0005fe58d2025..7b5df562f30f5625cdc2e1e5b6d59bff6919 100644
--- a/drivers/net/ethernet/amd/amd8111e.c
+++ b/drivers/net/ethernet/amd/amd8111e.c
@@ -695,125 +695,105 @@ static int amd8111e_rx_poll(struct napi_struct *napi, int budget)
 	void __iomem *mmio = lp->mmio;
 	struct sk_buff *skb,*new_skb;
 	int min_pkt_len, status;
-	unsigned int intr0;
 	int num_rx_pkt = 0;
 	short pkt_len;
 #if AMD8111E_VLAN_TAG_USED
 	short vtag;
 #endif
-	int rx_pkt_limit = budget;
-	unsigned long flags;
 
-	if (rx_pkt_limit <= 0)
-		goto rx_not_empty;
+	while (num_rx_pkt < budget) {
+		status = le16_to_cpu(lp->rx_ring[rx_index].rx_flags);
+		if (status & OWN_BIT)
+			break;
 
-	do{
-		/* process receive packets until we use the quota.
-		 * If we own the next entry, it's a new packet. Send it up.
+		/* There is a tricky error noted by John Murphy,
+		 * <murf@perftech.com> to Russ Nelson: Even with
+		 * full-sized * buffers it's possible for a
+		 * jabber packet to use two buffers, with only
+		 * the last correctly noting the error.
 		 */
-		while(1) {
-			status = le16_to_cpu(lp->rx_ring[rx_index].rx_flags);
-			if (status & OWN_BIT)
-				break;
-
-			/* There is a tricky error noted by John Murphy,
-			 * <murf@perftech.com> to Russ Nelson: Even with
-			 * full-sized * buffers it's possible for a
-			 * jabber packet to use two buffers, with only
-			 * the last correctly noting the error.
-			 */
-			if(status & ERR_BIT) {
-				/* resetting flags */
-				lp->rx_ring[rx_index].rx_flags &= RESET_RX_FLAGS;
-				goto err_next_pkt;
-			}
-			/* check for STP and ENP */
-			if(!((status & STP_BIT) && (status & ENP_BIT))){
-				/* resetting flags */
-				lp->rx_ring[rx_index].rx_flags &= RESET_RX_FLAGS;
-				goto err_next_pkt;
-			}
-			pkt_len = le16_to_cpu(lp->rx_ring[rx_index].msg_count) - 4;
+		if (status & ERR_BIT) {
+			/* resetting flags */
+			lp->rx_ring[rx_index].rx_flags &= RESET_RX_FLAGS;
+			goto err_next_pkt;
+		}
+		/* check for STP and ENP */
+		if (!((status & STP_BIT) && (status & ENP_BIT))){
+			/* resetting flags */
+			lp->rx_ring[rx_index].rx_flags &= RESET_RX_FLAGS;
+			goto err_next_pkt;
+		}
+		pkt_len = le16_to_cpu(lp->rx_ring[rx_index].msg_count) - 4;
 
 #if AMD8111E_VLAN_TAG_USED
-			vtag = status & TT_MASK;
-			/*MAC will strip vlan tag*/
-			if (vtag != 0)
-				min_pkt_len =MIN_PKT_LEN - 4;
+		vtag = status & TT_MASK;
+		/* MAC will strip vlan tag */
+		if (vtag != 0)
+			min_pkt_len = MIN_PKT_LEN - 4;
 			else
 #endif
-				min_pkt_len =MIN_PKT_LEN;
+			min_pkt_len = MIN_PKT_LEN;
 
-			if (pkt_len < min_pkt_len) {
-				lp->rx_ring[rx_index].rx_flags &= RESET_RX_FLAGS;
-				lp->drv_rx_errors++;
-				goto err_next_pkt;
-			}
-			if(--rx_pkt_limit < 0)
-				goto rx_not_empty;
-			new_skb = netdev_alloc_skb(dev, lp->rx_buff_len);
-			if (!new_skb) {
-				/* if allocation fail,
-				 * ignore that pkt and go to next one
-				 */
-				lp->rx_ring[rx_index].rx_flags &= RESET_RX_FLAGS;
-				lp->drv_rx_errors++;
-				goto err_next_pkt;
-			}
+		if (pkt_len < min_pkt_len) {
+			lp->rx_ring[rx_index].rx_flags &= RESET_RX_FLAGS;
+			lp->drv_rx_errors++;
+			goto err_next_pkt;
+		}
+		new_skb = netdev_alloc_skb(dev, lp->rx_buff_len);
+		if (!new_skb) {
+			/* if allocation fail,
+			 * ignore that pkt and go to next one
+			 */
+			lp->rx_ring[rx_index].rx_flags &= RESET_RX_FLAGS;
+			lp->drv_rx_errors++;
+			goto err_next_pkt;
+		}
 
-			skb_reserve(new_skb, 2);
-			skb = lp->rx_skbuff[rx_index];
-			pci_unmap_single(lp->pci_dev,lp->rx_dma_addr[rx_index],
-					 lp->rx_buff_len-2, PCI_DMA_FROMDEVICE);
-			skb_put(skb, pkt_len);
-			lp->rx_skbuff[rx_index] = new_skb;
-			lp->rx_dma_addr[rx_index] = pci_map_single(lp->pci_dev,
-								   new_skb->data,
-								   lp->rx_buff_len-2,
-								   PCI_DMA_FROMDEVICE);
+		skb_reserve(new_skb, 2);
+		skb = lp->rx_skbuff[rx_index];
+		pci_unmap_single(lp->pci_dev,lp->rx_dma_addr[rx_index],
+				 lp->rx_buff_len-2, PCI_DMA_FROMDEVICE);
+		skb_put(skb, pkt_len);
+		lp->rx_skbuff[rx_index] = new_skb;
+		lp->rx_dma_addr[rx_index] = pci_map_single(lp->pci_dev,
+							   new_skb->data,
+							   lp->rx_buff_len-2,
+							   PCI_DMA_FROMDEVICE);
 
-			skb->protocol = eth_type_trans(skb, dev);
+		skb->protocol = eth_type_trans(skb, dev);
 
 #if AMD8111E_VLAN_TAG_USED
-			if (vtag == TT_VLAN_TAGGED){
-				u16 vlan_tag = le16_to_cpu(lp->rx_ring[rx_index].tag_ctrl_info);
-				__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);
-			}
-#endif
-			netif_receive_skb(skb);
-			/*COAL update rx coalescing parameters*/
-			lp->coal_conf.rx_packets++;
-			lp->coal_conf.rx_bytes += pkt_len;
-			num_rx_pkt++;
-
-		err_next_pkt:
-			lp->rx_ring[rx_index].buff_phy_addr
-				= cpu_to_le32(lp->rx_dma_addr[rx_index]);
-			lp->rx_ring[rx_index].buff_count =
-				cpu_to_le16(lp->rx_buff_len-2);
-			wmb();
-			lp->rx_ring[rx_index].rx_flags |= cpu_to_le16(OWN_BIT);
-			rx_index = (++lp->rx_idx) & RX_RING_DR_MOD_MASK;
+		if (vtag == TT_VLAN_TAGGED){
+			u16 vlan_tag = le16_to_cpu(lp->rx_ring[rx_index].tag_ctrl_info);
+			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);
 		}
-		/* Check the interrupt status register for more packets in the
-		 * mean time. Process them since we have not used up our quota.
-		 */
-		intr0 = readl(mmio + INT0);
-		/*Ack receive packets */
-		writel(intr0 & RINT0,mmio + INT0);
+#endif
+		napi_gro_receive(napi, skb);
+		/* COAL update rx coalescing parameters */
+		lp->coal_conf.rx_packets++;
+		lp->coal_conf.rx_bytes += pkt_len;
+		num_rx_pkt++;
+
+err_next_pkt:
+		lp->rx_ring[rx_index].buff_phy_addr
+			= cpu_to_le32(lp->rx_dma_addr[rx_index]);
+		lp->rx_ring[rx_index].buff_count =
+			cpu_to_le16(lp->rx_buff_len-2);
+		wmb();
+		lp->rx_ring[rx_index].rx_flags |= cpu_to_le16(OWN_BIT);
+		rx_index = (++lp->rx_idx) & RX_RING_DR_MOD_MASK;
+	}
 
-	} while(intr0 & RINT0);
+	if (num_rx_pkt < budget && napi_complete_done(napi, num_rx_pkt)) {
+		unsigned long flags;
 
-	if (rx_pkt_limit > 0) {
 		/* Receive descriptor is empty now */
 		spin_lock_irqsave(&lp->lock, flags);
-		__napi_complete(napi);
 		writel(VAL0|RINTEN0, mmio + INTEN0);
 		writel(VAL2 | RDMD0, mmio + CMD0);
 		spin_unlock_irqrestore(&lp->lock, flags);
 	}
 
-rx_not_empty:
 	return num_rx_pkt;
 }
 
-- 
2.11.0.483.g087da7b7c-goog

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

* [PATCH v2 net-next 05/12] pcnet32: use napi_complete_done()
  2017-02-04 23:24 [PATCH v2 net-next 00/12] net: get rid of __napi_complete() Eric Dumazet
                   ` (3 preceding siblings ...)
  2017-02-04 23:24 ` [PATCH v2 net-next 04/12] amd8111e: add GRO support Eric Dumazet
@ 2017-02-04 23:24 ` Eric Dumazet
  2017-02-04 23:24 ` [PATCH v2 net-next 06/12] ep93xx_eth: add GRO support Eric Dumazet
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Eric Dumazet @ 2017-02-04 23:24 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

Use napi_complete_done() instead of __napi_complete() to :

1) Get support of gro_flush_timeout if opt-in
2) Not rearm interrupts for busy-polling users.
3) use standard NAPI API.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/ethernet/amd/pcnet32.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/amd/pcnet32.c b/drivers/net/ethernet/amd/pcnet32.c
index 41e58cca8feed284391d91f7469eaf4b79e5..a8a22c0d688a359fa977ffaf03da06bfc698 100644
--- a/drivers/net/ethernet/amd/pcnet32.c
+++ b/drivers/net/ethernet/amd/pcnet32.c
@@ -1350,13 +1350,8 @@ static int pcnet32_poll(struct napi_struct *napi, int budget)
 		pcnet32_restart(dev, CSR0_START);
 		netif_wake_queue(dev);
 	}
-	spin_unlock_irqrestore(&lp->lock, flags);
-
-	if (work_done < budget) {
-		spin_lock_irqsave(&lp->lock, flags);
-
-		__napi_complete(napi);
 
+	if (work_done < budget && napi_complete_done(napi, work_done)) {
 		/* clear interrupt masks */
 		val = lp->a->read_csr(ioaddr, CSR3);
 		val &= 0x00ff;
@@ -1364,9 +1359,9 @@ static int pcnet32_poll(struct napi_struct *napi, int budget)
 
 		/* Set interrupt enable. */
 		lp->a->write_csr(ioaddr, CSR0, CSR0_INTEN);
-
-		spin_unlock_irqrestore(&lp->lock, flags);
 	}
+
+	spin_unlock_irqrestore(&lp->lock, flags);
 	return work_done;
 }
 
-- 
2.11.0.483.g087da7b7c-goog

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

* [PATCH v2 net-next 06/12] ep93xx_eth: add GRO support
  2017-02-04 23:24 [PATCH v2 net-next 00/12] net: get rid of __napi_complete() Eric Dumazet
                   ` (4 preceding siblings ...)
  2017-02-04 23:24 ` [PATCH v2 net-next 05/12] pcnet32: use napi_complete_done() Eric Dumazet
@ 2017-02-04 23:24 ` Eric Dumazet
  2017-05-15 10:31   ` Lennert Buytenhek
  2017-02-04 23:24 ` [PATCH v2 net-next 07/12] skge: use napi_complete_done() Eric Dumazet
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 19+ messages in thread
From: Eric Dumazet @ 2017-02-04 23:24 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

Use napi_complete_done() instead of __napi_complete() to :

1) Get support of gro_flush_timeout if opt-in
2) Not rearm interrupts for busy-polling users.
3) use standard NAPI API.
4) get rid of baroque code and ease maintenance.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/ethernet/cirrus/ep93xx_eth.c | 29 ++++++-----------------------
 1 file changed, 6 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/cirrus/ep93xx_eth.c b/drivers/net/ethernet/cirrus/ep93xx_eth.c
index 396c88678eabfec556536ca4a81f86224563..7a7c02f1f8b955f28206855f7084630cf8b3 100644
--- a/drivers/net/ethernet/cirrus/ep93xx_eth.c
+++ b/drivers/net/ethernet/cirrus/ep93xx_eth.c
@@ -228,9 +228,10 @@ static void ep93xx_mdio_write(struct net_device *dev, int phy_id, int reg, int d
 		pr_info("mdio write timed out\n");
 }
 
-static int ep93xx_rx(struct net_device *dev, int processed, int budget)
+static int ep93xx_rx(struct net_device *dev, int budget)
 {
 	struct ep93xx_priv *ep = netdev_priv(dev);
+	int processed = 0;
 
 	while (processed < budget) {
 		int entry;
@@ -294,7 +295,7 @@ static int ep93xx_rx(struct net_device *dev, int processed, int budget)
 			skb_put(skb, length);
 			skb->protocol = eth_type_trans(skb, dev);
 
-			netif_receive_skb(skb);
+			napi_gro_receive(&ep->napi, skb);
 
 			dev->stats.rx_packets++;
 			dev->stats.rx_bytes += length;
@@ -310,35 +311,17 @@ static int ep93xx_rx(struct net_device *dev, int processed, int budget)
 	return processed;
 }
 
-static int ep93xx_have_more_rx(struct ep93xx_priv *ep)
-{
-	struct ep93xx_rstat *rstat = ep->descs->rstat + ep->rx_pointer;
-	return !!((rstat->rstat0 & RSTAT0_RFP) && (rstat->rstat1 & RSTAT1_RFP));
-}
-
 static int ep93xx_poll(struct napi_struct *napi, int budget)
 {
 	struct ep93xx_priv *ep = container_of(napi, struct ep93xx_priv, napi);
 	struct net_device *dev = ep->dev;
-	int rx = 0;
-
-poll_some_more:
-	rx = ep93xx_rx(dev, rx, budget);
-	if (rx < budget) {
-		int more = 0;
+	int rx;
 
+	rx = ep93xx_rx(dev, budget);
+	if (rx < budget && napi_complete_done(napi, rx)) {
 		spin_lock_irq(&ep->rx_lock);
-		__napi_complete(napi);
 		wrl(ep, REG_INTEN, REG_INTEN_TX | REG_INTEN_RX);
-		if (ep93xx_have_more_rx(ep)) {
-			wrl(ep, REG_INTEN, REG_INTEN_TX);
-			wrl(ep, REG_INTSTSP, REG_INTSTS_RX);
-			more = 1;
-		}
 		spin_unlock_irq(&ep->rx_lock);
-
-		if (more && napi_reschedule(napi))
-			goto poll_some_more;
 	}
 
 	if (rx) {
-- 
2.11.0.483.g087da7b7c-goog

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

* [PATCH v2 net-next 07/12] skge: use napi_complete_done()
  2017-02-04 23:24 [PATCH v2 net-next 00/12] net: get rid of __napi_complete() Eric Dumazet
                   ` (5 preceding siblings ...)
  2017-02-04 23:24 ` [PATCH v2 net-next 06/12] ep93xx_eth: add GRO support Eric Dumazet
@ 2017-02-04 23:24 ` Eric Dumazet
  2017-02-04 23:24 ` [PATCH v2 net-next 08/12] ks8695net: add GRO support Eric Dumazet
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Eric Dumazet @ 2017-02-04 23:24 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

Use napi_complete_done() instead of __napi_complete() to :

1) Get support of gro_flush_timeout if opt-in
2) Not rearm interrupts for busy-polling users.
3) use standard NAPI API and get rid of napi_gro_flush()

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/ethernet/marvell/skge.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/marvell/skge.c b/drivers/net/ethernet/marvell/skge.c
index 81106b73e468b3d542fbbef94503278103c7..edb95271a4f2f784d740b3201364a5e45de1 100644
--- a/drivers/net/ethernet/marvell/skge.c
+++ b/drivers/net/ethernet/marvell/skge.c
@@ -3201,7 +3201,7 @@ static void skge_tx_done(struct net_device *dev)
 	}
 }
 
-static int skge_poll(struct napi_struct *napi, int to_do)
+static int skge_poll(struct napi_struct *napi, int budget)
 {
 	struct skge_port *skge = container_of(napi, struct skge_port, napi);
 	struct net_device *dev = skge->netdev;
@@ -3214,7 +3214,7 @@ static int skge_poll(struct napi_struct *napi, int to_do)
 
 	skge_write8(hw, Q_ADDR(rxqaddr[skge->port], Q_CSR), CSR_IRQ_CL_F);
 
-	for (e = ring->to_clean; prefetch(e->next), work_done < to_do; e = e->next) {
+	for (e = ring->to_clean; prefetch(e->next), work_done < budget; e = e->next) {
 		struct skge_rx_desc *rd = e->desc;
 		struct sk_buff *skb;
 		u32 control;
@@ -3236,12 +3236,10 @@ static int skge_poll(struct napi_struct *napi, int to_do)
 	wmb();
 	skge_write8(hw, Q_ADDR(rxqaddr[skge->port], Q_CSR), CSR_START);
 
-	if (work_done < to_do) {
+	if (work_done < budget && napi_complete_done(napi, work_done)) {
 		unsigned long flags;
 
-		napi_gro_flush(napi, false);
 		spin_lock_irqsave(&hw->hw_lock, flags);
-		__napi_complete(napi);
 		hw->intr_mask |= napimask[skge->port];
 		skge_write32(hw, B0_IMSK, hw->intr_mask);
 		skge_read32(hw, B0_IMSK);
-- 
2.11.0.483.g087da7b7c-goog

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

* [PATCH v2 net-next 08/12] ks8695net: add GRO support
  2017-02-04 23:24 [PATCH v2 net-next 00/12] net: get rid of __napi_complete() Eric Dumazet
                   ` (6 preceding siblings ...)
  2017-02-04 23:24 ` [PATCH v2 net-next 07/12] skge: use napi_complete_done() Eric Dumazet
@ 2017-02-04 23:24 ` Eric Dumazet
  2017-02-04 23:24 ` [PATCH v2 net-next 09/12] qla3xxx: " Eric Dumazet
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Eric Dumazet @ 2017-02-04 23:24 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

Use napi_complete_done() instead of __napi_complete() to :

1) Get support of gro_flush_timeout if opt-in
2) Not rearm interrupts for busy-polling users.
3) use standard NAPI API.

Note that rx_lock seems to be useless, NAPI logic should
not need this extra care.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/ethernet/micrel/ks8695net.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/micrel/ks8695net.c b/drivers/net/ethernet/micrel/ks8695net.c
index 20cb85bc0c5f8660a259dfb8dfd7213df713..d2106159473f24f576da2d965704476d1576 100644
--- a/drivers/net/ethernet/micrel/ks8695net.c
+++ b/drivers/net/ethernet/micrel/ks8695net.c
@@ -519,7 +519,7 @@ static int ks8695_rx(struct ks8695_priv *ksp, int budget)
 			/* Relinquish the SKB to the network layer */
 			skb_put(skb, pktlen);
 			skb->protocol = eth_type_trans(skb, ndev);
-			netif_receive_skb(skb);
+			napi_gro_receive(&ksp->napi, skb);
 
 			/* Record stats */
 			ndev->stats.rx_packets++;
@@ -561,18 +561,17 @@ static int ks8695_rx(struct ks8695_priv *ksp, int budget)
 static int ks8695_poll(struct napi_struct *napi, int budget)
 {
 	struct ks8695_priv *ksp = container_of(napi, struct ks8695_priv, napi);
-	unsigned long  work_done;
-
 	unsigned long isr = readl(KS8695_IRQ_VA + KS8695_INTEN);
 	unsigned long mask_bit = 1 << ks8695_get_rx_enable_bit(ksp);
+	int work_done;
 
 	work_done = ks8695_rx(ksp, budget);
 
-	if (work_done < budget) {
+	if (work_done < budget && napi_complete_done(napi, work_done)) {
 		unsigned long flags;
+
 		spin_lock_irqsave(&ksp->rx_lock, flags);
-		__napi_complete(napi);
-		/*enable rx interrupt*/
+		/* enable rx interrupt */
 		writel(isr | mask_bit, KS8695_IRQ_VA + KS8695_INTEN);
 		spin_unlock_irqrestore(&ksp->rx_lock, flags);
 	}
-- 
2.11.0.483.g087da7b7c-goog

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

* [PATCH v2 net-next 09/12] qla3xxx: add GRO support
  2017-02-04 23:24 [PATCH v2 net-next 00/12] net: get rid of __napi_complete() Eric Dumazet
                   ` (7 preceding siblings ...)
  2017-02-04 23:24 ` [PATCH v2 net-next 08/12] ks8695net: add GRO support Eric Dumazet
@ 2017-02-04 23:24 ` Eric Dumazet
  2017-02-04 23:25 ` [PATCH v2 net-next 10/12] ibm/emac: use napi_complete_done() Eric Dumazet
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Eric Dumazet @ 2017-02-04 23:24 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

Use napi_complete_done() instead of __napi_complete() to :

1) Get support of gro_flush_timeout if opt-in
2) Not rearm interrupts for busy-polling users.
3) use standard NAPI API.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/ethernet/qlogic/qla3xxx.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qla3xxx.c b/drivers/net/ethernet/qlogic/qla3xxx.c
index 5c100ab86c0046ec72a011e5f806d6557ac7..ea38236f1ced5c2d40c5bb344defc32142e7 100644
--- a/drivers/net/ethernet/qlogic/qla3xxx.c
+++ b/drivers/net/ethernet/qlogic/qla3xxx.c
@@ -2025,7 +2025,7 @@ static void ql_process_mac_rx_intr(struct ql3_adapter *qdev,
 	skb_checksum_none_assert(skb);
 	skb->protocol = eth_type_trans(skb, qdev->ndev);
 
-	netif_receive_skb(skb);
+	napi_gro_receive(&qdev->napi, skb);
 	lrg_buf_cb2->skb = NULL;
 
 	if (qdev->device_id == QL3022_DEVICE_ID)
@@ -2095,7 +2095,7 @@ static void ql_process_macip_rx_intr(struct ql3_adapter *qdev,
 	}
 	skb2->protocol = eth_type_trans(skb2, qdev->ndev);
 
-	netif_receive_skb(skb2);
+	napi_gro_receive(&qdev->napi, skb2);
 	ndev->stats.rx_packets++;
 	ndev->stats.rx_bytes += length;
 	lrg_buf_cb2->skb = NULL;
@@ -2105,8 +2105,7 @@ static void ql_process_macip_rx_intr(struct ql3_adapter *qdev,
 	ql_release_to_lrg_buf_free_list(qdev, lrg_buf_cb2);
 }
 
-static int ql_tx_rx_clean(struct ql3_adapter *qdev,
-			  int *tx_cleaned, int *rx_cleaned, int work_to_do)
+static int ql_tx_rx_clean(struct ql3_adapter *qdev, int budget)
 {
 	struct net_rsp_iocb *net_rsp;
 	struct net_device *ndev = qdev->ndev;
@@ -2114,7 +2113,7 @@ static int ql_tx_rx_clean(struct ql3_adapter *qdev,
 
 	/* While there are entries in the completion queue. */
 	while ((le32_to_cpu(*(qdev->prsp_producer_index)) !=
-		qdev->rsp_consumer_index) && (work_done < work_to_do)) {
+		qdev->rsp_consumer_index) && (work_done < budget)) {
 
 		net_rsp = qdev->rsp_current;
 		rmb();
@@ -2130,21 +2129,20 @@ static int ql_tx_rx_clean(struct ql3_adapter *qdev,
 		case OPCODE_OB_MAC_IOCB_FN2:
 			ql_process_mac_tx_intr(qdev, (struct ob_mac_iocb_rsp *)
 					       net_rsp);
-			(*tx_cleaned)++;
 			break;
 
 		case OPCODE_IB_MAC_IOCB:
 		case OPCODE_IB_3032_MAC_IOCB:
 			ql_process_mac_rx_intr(qdev, (struct ib_mac_iocb_rsp *)
 					       net_rsp);
-			(*rx_cleaned)++;
+			work_done++;
 			break;
 
 		case OPCODE_IB_IP_IOCB:
 		case OPCODE_IB_3032_IP_IOCB:
 			ql_process_macip_rx_intr(qdev, (struct ib_ip_iocb_rsp *)
 						 net_rsp);
-			(*rx_cleaned)++;
+			work_done++;
 			break;
 		default: {
 			u32 *tmp = (u32 *)net_rsp;
@@ -2169,7 +2167,6 @@ static int ql_tx_rx_clean(struct ql3_adapter *qdev,
 			qdev->rsp_current++;
 		}
 
-		work_done = *tx_cleaned + *rx_cleaned;
 	}
 
 	return work_done;
@@ -2178,25 +2175,25 @@ static int ql_tx_rx_clean(struct ql3_adapter *qdev,
 static int ql_poll(struct napi_struct *napi, int budget)
 {
 	struct ql3_adapter *qdev = container_of(napi, struct ql3_adapter, napi);
-	int rx_cleaned = 0, tx_cleaned = 0;
-	unsigned long hw_flags;
 	struct ql3xxx_port_registers __iomem *port_regs =
 		qdev->mem_map_registers;
+	int work_done;
 
-	ql_tx_rx_clean(qdev, &tx_cleaned, &rx_cleaned, budget);
+	work_done = ql_tx_rx_clean(qdev, budget);
 
-	if (tx_cleaned + rx_cleaned != budget) {
-		spin_lock_irqsave(&qdev->hw_lock, hw_flags);
-		__napi_complete(napi);
+	if (work_done < budget && napi_complete_done(napi, work_done)) {
+		unsigned long flags;
+
+		spin_lock_irqsave(&qdev->hw_lock, flags);
 		ql_update_small_bufq_prod_index(qdev);
 		ql_update_lrg_bufq_prod_index(qdev);
 		writel(qdev->rsp_consumer_index,
 			    &port_regs->CommonRegs.rspQConsumerIndex);
-		spin_unlock_irqrestore(&qdev->hw_lock, hw_flags);
+		spin_unlock_irqrestore(&qdev->hw_lock, flags);
 
 		ql_enable_interrupts(qdev);
 	}
-	return tx_cleaned + rx_cleaned;
+	return work_done;
 }
 
 static irqreturn_t ql3xxx_isr(int irq, void *dev_id)
-- 
2.11.0.483.g087da7b7c-goog

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

* [PATCH v2 net-next 10/12] ibm/emac: use napi_complete_done()
  2017-02-04 23:24 [PATCH v2 net-next 00/12] net: get rid of __napi_complete() Eric Dumazet
                   ` (8 preceding siblings ...)
  2017-02-04 23:24 ` [PATCH v2 net-next 09/12] qla3xxx: " Eric Dumazet
@ 2017-02-04 23:25 ` Eric Dumazet
  2017-02-04 23:25 ` [PATCH v2 net-next 11/12] aeroflex/greth: " Eric Dumazet
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Eric Dumazet @ 2017-02-04 23:25 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

Use napi_complete_done() instead of __napi_complete()

We plan to remove __napi_complete() to reduce NAPI complexity.
---
 drivers/net/ethernet/ibm/emac/mal.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/mal.c b/drivers/net/ethernet/ibm/emac/mal.c
index aaf6fec566b5175a834c74cee494ebcf13e1..cd3227b088b73f51b68951401eefc7e2d1e1 100644
--- a/drivers/net/ethernet/ibm/emac/mal.c
+++ b/drivers/net/ethernet/ibm/emac/mal.c
@@ -421,20 +421,20 @@ static int mal_poll(struct napi_struct *napi, int budget)
 		int n;
 		if (unlikely(test_bit(MAL_COMMAC_POLL_DISABLED, &mc->flags)))
 			continue;
-		n = mc->ops->poll_rx(mc->dev, budget);
+		n = mc->ops->poll_rx(mc->dev, budget - received);
 		if (n) {
 			received += n;
-			budget -= n;
-			if (budget <= 0)
-				goto more_work; // XXX What if this is the last one ?
+			if (received >= budget)
+				return budget;
 		}
 	}
 
-	/* We need to disable IRQs to protect from RXDE IRQ here */
-	spin_lock_irqsave(&mal->lock, flags);
-	__napi_complete(napi);
-	mal_enable_eob_irq(mal);
-	spin_unlock_irqrestore(&mal->lock, flags);
+	if (napi_complete_done(napi, received)) {
+		/* We need to disable IRQs to protect from RXDE IRQ here */
+		spin_lock_irqsave(&mal->lock, flags);
+		mal_enable_eob_irq(mal);
+		spin_unlock_irqrestore(&mal->lock, flags);
+	}
 
 	/* Check for "rotting" packet(s) */
 	list_for_each(l, &mal->poll_list) {
-- 
2.11.0.483.g087da7b7c-goog

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

* [PATCH v2 net-next 11/12] aeroflex/greth: use napi_complete_done()
  2017-02-04 23:24 [PATCH v2 net-next 00/12] net: get rid of __napi_complete() Eric Dumazet
                   ` (9 preceding siblings ...)
  2017-02-04 23:25 ` [PATCH v2 net-next 10/12] ibm/emac: use napi_complete_done() Eric Dumazet
@ 2017-02-04 23:25 ` Eric Dumazet
  2017-02-04 23:25 ` [PATCH v2 net-next 12/12] net: remove __napi_complete() Eric Dumazet
  2017-02-05 22:16 ` [PATCH v2 net-next 00/12] net: get rid of __napi_complete() David Miller
  12 siblings, 0 replies; 19+ messages in thread
From: Eric Dumazet @ 2017-02-04 23:25 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

We plan to remove __napi_complete() soon,
this driver is the last user.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/ethernet/aeroflex/greth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/aeroflex/greth.c b/drivers/net/ethernet/aeroflex/greth.c
index 93def92f999703d234b44508725f45863ef3..9f7422ada704e9484b79d336ece46a054766 100644
--- a/drivers/net/ethernet/aeroflex/greth.c
+++ b/drivers/net/ethernet/aeroflex/greth.c
@@ -1008,7 +1008,7 @@ static int greth_poll(struct napi_struct *napi, int budget)
 			spin_unlock_irqrestore(&greth->devlock, flags);
 			goto restart_txrx_poll;
 		} else {
-			__napi_complete(napi);
+			napi_complete_done(napi, work_done);
 			spin_unlock_irqrestore(&greth->devlock, flags);
 		}
 	}
-- 
2.11.0.483.g087da7b7c-goog

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

* [PATCH v2 net-next 12/12] net: remove __napi_complete()
  2017-02-04 23:24 [PATCH v2 net-next 00/12] net: get rid of __napi_complete() Eric Dumazet
                   ` (10 preceding siblings ...)
  2017-02-04 23:25 ` [PATCH v2 net-next 11/12] aeroflex/greth: " Eric Dumazet
@ 2017-02-04 23:25 ` Eric Dumazet
  2017-02-05 22:16 ` [PATCH v2 net-next 00/12] net: get rid of __napi_complete() David Miller
  12 siblings, 0 replies; 19+ messages in thread
From: Eric Dumazet @ 2017-02-04 23:25 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

All __napi_complete() callers have been converted to
use the more standard napi_complete_done(),
we can now remove this NAPI method for good.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/netdevice.h |  1 -
 net/core/dev.c            | 24 +++---------------------
 2 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 6f18b509fb2f473029561d85646713c149df..014fbe256d55f843b72595e881d646dd5762 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -463,7 +463,6 @@ static inline bool napi_reschedule(struct napi_struct *napi)
 	return false;
 }
 
-bool __napi_complete(struct napi_struct *n);
 bool napi_complete_done(struct napi_struct *n, int work_done);
 /**
  *	napi_complete - NAPI processing complete
diff --git a/net/core/dev.c b/net/core/dev.c
index 42ba0379575a28525052a36620bfb30d38af..404d2e6d5d3233ad95edee18581276841573 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4883,23 +4883,6 @@ void __napi_schedule_irqoff(struct napi_struct *n)
 }
 EXPORT_SYMBOL(__napi_schedule_irqoff);
 
-bool __napi_complete(struct napi_struct *n)
-{
-	BUG_ON(!test_bit(NAPI_STATE_SCHED, &n->state));
-
-	/* Some drivers call us directly, instead of calling
-	 * napi_complete_done().
-	 */
-	if (unlikely(test_bit(NAPI_STATE_IN_BUSY_POLL, &n->state)))
-		return false;
-
-	list_del_init(&n->poll_list);
-	smp_mb__before_atomic();
-	clear_bit(NAPI_STATE_SCHED, &n->state);
-	return true;
-}
-EXPORT_SYMBOL(__napi_complete);
-
 bool napi_complete_done(struct napi_struct *n, int work_done)
 {
 	unsigned long flags;
@@ -4926,14 +4909,13 @@ bool napi_complete_done(struct napi_struct *n, int work_done)
 		else
 			napi_gro_flush(n, false);
 	}
-	if (likely(list_empty(&n->poll_list))) {
-		WARN_ON_ONCE(!test_and_clear_bit(NAPI_STATE_SCHED, &n->state));
-	} else {
+	if (unlikely(!list_empty(&n->poll_list))) {
 		/* If n->poll_list is not empty, we need to mask irqs */
 		local_irq_save(flags);
-		__napi_complete(n);
+		list_del_init(&n->poll_list);
 		local_irq_restore(flags);
 	}
+	WARN_ON_ONCE(!test_and_clear_bit(NAPI_STATE_SCHED, &n->state));
 	return true;
 }
 EXPORT_SYMBOL(napi_complete_done);
-- 
2.11.0.483.g087da7b7c-goog

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

* Re: [PATCH v2 net-next 00/12] net: get rid of __napi_complete()
  2017-02-04 23:24 [PATCH v2 net-next 00/12] net: get rid of __napi_complete() Eric Dumazet
                   ` (11 preceding siblings ...)
  2017-02-04 23:25 ` [PATCH v2 net-next 12/12] net: remove __napi_complete() Eric Dumazet
@ 2017-02-05 22:16 ` David Miller
  12 siblings, 0 replies; 19+ messages in thread
From: David Miller @ 2017-02-05 22:16 UTC (permalink / raw)
  To: edumazet; +Cc: netdev, eric.dumazet

From: Eric Dumazet <edumazet@google.com>
Date: Sat,  4 Feb 2017 15:24:50 -0800

> This patch series removes __napi_complete() calls, in an effort
> to make NAPI API simpler and generalize GRO and napi_complete_done()

Nice cleanup, series applied, thanks Eric.

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

* Re: [PATCH v2 net-next 06/12] ep93xx_eth: add GRO support
  2017-02-04 23:24 ` [PATCH v2 net-next 06/12] ep93xx_eth: add GRO support Eric Dumazet
@ 2017-05-15 10:31   ` Lennert Buytenhek
  2017-05-15 20:43     ` Ryan Mallon
  0 siblings, 1 reply; 19+ messages in thread
From: Lennert Buytenhek @ 2017-05-15 10:31 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S. Miller, netdev, Eric Dumazet, Ryan Mallon,
	Hartley Sweeten

On Sat, Feb 04, 2017 at 03:24:56PM -0800, Eric Dumazet wrote:

> Use napi_complete_done() instead of __napi_complete() to :
> 
> 1) Get support of gro_flush_timeout if opt-in
> 2) Not rearm interrupts for busy-polling users.
> 3) use standard NAPI API.
> 4) get rid of baroque code and ease maintenance.
>
> [...]
>
> @@ -310,35 +311,17 @@ static int ep93xx_rx(struct net_device *dev, int processed, int budget)
>  	return processed;
>  }
>  
> -static int ep93xx_have_more_rx(struct ep93xx_priv *ep)
> -{
> -	struct ep93xx_rstat *rstat = ep->descs->rstat + ep->rx_pointer;
> -	return !!((rstat->rstat0 & RSTAT0_RFP) && (rstat->rstat1 & RSTAT1_RFP));
> -}
> -
>  static int ep93xx_poll(struct napi_struct *napi, int budget)
>  {
>  	struct ep93xx_priv *ep = container_of(napi, struct ep93xx_priv, napi);
>  	struct net_device *dev = ep->dev;
> -	int rx = 0;
> -
> -poll_some_more:
> -	rx = ep93xx_rx(dev, rx, budget);
> -	if (rx < budget) {
> -		int more = 0;
> +	int rx;
>  
> +	rx = ep93xx_rx(dev, budget);
> +	if (rx < budget && napi_complete_done(napi, rx)) {
>  		spin_lock_irq(&ep->rx_lock);
> -		__napi_complete(napi);
>  		wrl(ep, REG_INTEN, REG_INTEN_TX | REG_INTEN_RX);
> -		if (ep93xx_have_more_rx(ep)) {
> -			wrl(ep, REG_INTEN, REG_INTEN_TX);
> -			wrl(ep, REG_INTSTSP, REG_INTSTS_RX);
> -			more = 1;
> -		}
>  		spin_unlock_irq(&ep->rx_lock);
> -
> -		if (more && napi_reschedule(napi))
> -			goto poll_some_more;
>  	}
>  
>  	if (rx) {

This code was the way it was because the ep93xx hardware is somewhat
braindead.  If I remember correctly (but it's been a while since I wrote
this code):

1. ep93xx netdev IRQs are edge-triggered, so if you re-enable IRQs
   while there was still work to be done, you will not get another IRQ.

2. Disabling an interrupt source in the interrupt mask register will
   cause its interrupt status bit to always return zero, so you cannot
   check whether an interrupt status is pending without having the
   interrupt source enabled.

(I'll admit that a comment explaining this would have been in order.)

I don't know if we really care about this hardware anymore (I don't),
but the ep93xx platform is still listed as being maintained in the
MAINTAINERS file -- adding Ryan and Hartley.

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

* Re: [PATCH v2 net-next 06/12] ep93xx_eth: add GRO support
  2017-05-15 10:31   ` Lennert Buytenhek
@ 2017-05-15 20:43     ` Ryan Mallon
  2017-05-15 21:02       ` Alexander Sverdlin
  0 siblings, 1 reply; 19+ messages in thread
From: Ryan Mallon @ 2017-05-15 20:43 UTC (permalink / raw)
  To: Lennert Buytenhek, Eric Dumazet
  Cc: David S. Miller, netdev, Eric Dumazet, Hartley Sweeten,
	alexander.sverdlin



On 15/05/17 20:31, Lennert Buytenhek wrote:
> On Sat, Feb 04, 2017 at 03:24:56PM -0800, Eric Dumazet wrote:
> 
>> Use napi_complete_done() instead of __napi_complete() to :
>>
>> 1) Get support of gro_flush_timeout if opt-in
>> 2) Not rearm interrupts for busy-polling users.
>> 3) use standard NAPI API.
>> 4) get rid of baroque code and ease maintenance.
>>
>> [...]
>>
>> @@ -310,35 +311,17 @@ static int ep93xx_rx(struct net_device *dev, int processed, int budget)
>>  	return processed;
>>  }
>>  
>> -static int ep93xx_have_more_rx(struct ep93xx_priv *ep)
>> -{
>> -	struct ep93xx_rstat *rstat = ep->descs->rstat + ep->rx_pointer;
>> -	return !!((rstat->rstat0 & RSTAT0_RFP) && (rstat->rstat1 & RSTAT1_RFP));
>> -}
>> -
>>  static int ep93xx_poll(struct napi_struct *napi, int budget)
>>  {
>>  	struct ep93xx_priv *ep = container_of(napi, struct ep93xx_priv, napi);
>>  	struct net_device *dev = ep->dev;
>> -	int rx = 0;
>> -
>> -poll_some_more:
>> -	rx = ep93xx_rx(dev, rx, budget);
>> -	if (rx < budget) {
>> -		int more = 0;
>> +	int rx;
>>  
>> +	rx = ep93xx_rx(dev, budget);
>> +	if (rx < budget && napi_complete_done(napi, rx)) {
>>  		spin_lock_irq(&ep->rx_lock);
>> -		__napi_complete(napi);
>>  		wrl(ep, REG_INTEN, REG_INTEN_TX | REG_INTEN_RX);
>> -		if (ep93xx_have_more_rx(ep)) {
>> -			wrl(ep, REG_INTEN, REG_INTEN_TX);
>> -			wrl(ep, REG_INTSTSP, REG_INTSTS_RX);
>> -			more = 1;
>> -		}
>>  		spin_unlock_irq(&ep->rx_lock);
>> -
>> -		if (more && napi_reschedule(napi))
>> -			goto poll_some_more;
>>  	}
>>  
>>  	if (rx) {
> 
> This code was the way it was because the ep93xx hardware is somewhat
> braindead.  If I remember correctly (but it's been a while since I wrote
> this code):
> 
> 1. ep93xx netdev IRQs are edge-triggered, so if you re-enable IRQs
>    while there was still work to be done, you will not get another IRQ.
> 
> 2. Disabling an interrupt source in the interrupt mask register will
>    cause its interrupt status bit to always return zero, so you cannot
>    check whether an interrupt status is pending without having the
>    interrupt source enabled.
> 
> (I'll admit that a comment explaining this would have been in order.)
> 
> I don't know if we really care about this hardware anymore (I don't),
> but the ep93xx platform is still listed as being maintained in the
> MAINTAINERS file -- adding Ryan and Hartley.

I no longer have any ep93xx hardware to test with, and I never looked at
the Ethernet, so don't know the details. I think there are still a
handful of users. Adding Alexander who sent an ADC support series this
week, who might be able to test this?

~Ryan

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

* Re: [PATCH v2 net-next 06/12] ep93xx_eth: add GRO support
  2017-05-15 20:43     ` Ryan Mallon
@ 2017-05-15 21:02       ` Alexander Sverdlin
  2017-05-16 20:41         ` Alexander Sverdlin
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Sverdlin @ 2017-05-15 21:02 UTC (permalink / raw)
  To: Ryan Mallon, Lennert Buytenhek, Eric Dumazet
  Cc: David S. Miller, netdev, Eric Dumazet, Hartley Sweeten

Hi!

On Mon May 15 22:43:20 2017 Ryan Mallon <rmallon@gmail.com> wrote:
> > I don't know if we really care about this hardware anymore (I don't),
> > but the ep93xx platform is still listed as being maintained in the
> > MAINTAINERS file -- adding Ryan and Hartley.
> 
> I no longer have any ep93xx hardware to test with, and I never looked at
> the Ethernet, so don't know the details. I think there are still a
> handful of users. Adding Alexander who sent an ADC support series this
> week, who might be able to test this?

Yes, I very much care about ep93xx code being functional :)
I'll test the patches tomorrow.

Alexander.

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

* Re: [PATCH v2 net-next 06/12] ep93xx_eth: add GRO support
  2017-05-15 21:02       ` Alexander Sverdlin
@ 2017-05-16 20:41         ` Alexander Sverdlin
  2017-05-16 20:48           ` Eric Dumazet
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Sverdlin @ 2017-05-16 20:41 UTC (permalink / raw)
  To: Ryan Mallon, Lennert Buytenhek, Eric Dumazet
  Cc: David S. Miller, netdev, Eric Dumazet, Hartley Sweeten

Hello all,

On 15/05/17 23:02, Alexander Sverdlin wrote:
>>> I don't know if we really care about this hardware anymore (I don't),
>>> but the ep93xx platform is still listed as being maintained in the
>>> MAINTAINERS file -- adding Ryan and Hartley.
>> I no longer have any ep93xx hardware to test with, and I never looked at
>> the Ethernet, so don't know the details. I think there are still a
>> handful of users. Adding Alexander who sent an ADC support series this
>> week, who might be able to test this?
> Yes, I very much care about ep93xx code being functional :)
> I'll test the patches tomorrow.

it turns out I've used this patch two weeks long already in 4.11; but I've spent
a couple of hours now torturing the new driver and was not able to provoke
any inadequate behavior. It either receives all packets in time or not at all.
If IRQs would be edge-triggered, I'd expect some stale packets, which do not
arrive at first, but then appear with the packets coming next. This is not
the case. I've used pktgen module for this, with minimal packets and
different bursts.

netperf shows 45Mbit/s on UDP_STREAM test, which is also fair amount for
200MHz CPU.

So, I see no problems with the change.

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

* Re: [PATCH v2 net-next 06/12] ep93xx_eth: add GRO support
  2017-05-16 20:41         ` Alexander Sverdlin
@ 2017-05-16 20:48           ` Eric Dumazet
  0 siblings, 0 replies; 19+ messages in thread
From: Eric Dumazet @ 2017-05-16 20:48 UTC (permalink / raw)
  To: Alexander Sverdlin
  Cc: Ryan Mallon, Lennert Buytenhek, David S. Miller, netdev,
	Eric Dumazet, Hartley Sweeten

On Tue, May 16, 2017 at 1:41 PM, Alexander Sverdlin
<alexander.sverdlin@gmail.com> wrote:

> it turns out I've used this patch two weeks long already in 4.11; but I've spent
> a couple of hours now torturing the new driver and was not able to provoke
> any inadequate behavior. It either receives all packets in time or not at all.
> If IRQs would be edge-triggered, I'd expect some stale packets, which do not
> arrive at first, but then appear with the packets coming next. This is not
> the case. I've used pktgen module for this, with minimal packets and
> different bursts.
>
> netperf shows 45Mbit/s on UDP_STREAM test, which is also fair amount for
> 200MHz CPU.
>
> So, I see no problems with the change.
>

Thanks a lot for testing !

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

end of thread, other threads:[~2017-05-16 20:48 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-04 23:24 [PATCH v2 net-next 00/12] net: get rid of __napi_complete() Eric Dumazet
2017-02-04 23:24 ` [PATCH v2 net-next 01/12] 8139too: use napi_complete_done() Eric Dumazet
2017-02-04 23:24 ` [PATCH v2 net-next 02/12] 8139cp: " Eric Dumazet
2017-02-04 23:24 ` [PATCH v2 net-next 03/12] epic100: " Eric Dumazet
2017-02-04 23:24 ` [PATCH v2 net-next 04/12] amd8111e: add GRO support Eric Dumazet
2017-02-04 23:24 ` [PATCH v2 net-next 05/12] pcnet32: use napi_complete_done() Eric Dumazet
2017-02-04 23:24 ` [PATCH v2 net-next 06/12] ep93xx_eth: add GRO support Eric Dumazet
2017-05-15 10:31   ` Lennert Buytenhek
2017-05-15 20:43     ` Ryan Mallon
2017-05-15 21:02       ` Alexander Sverdlin
2017-05-16 20:41         ` Alexander Sverdlin
2017-05-16 20:48           ` Eric Dumazet
2017-02-04 23:24 ` [PATCH v2 net-next 07/12] skge: use napi_complete_done() Eric Dumazet
2017-02-04 23:24 ` [PATCH v2 net-next 08/12] ks8695net: add GRO support Eric Dumazet
2017-02-04 23:24 ` [PATCH v2 net-next 09/12] qla3xxx: " Eric Dumazet
2017-02-04 23:25 ` [PATCH v2 net-next 10/12] ibm/emac: use napi_complete_done() Eric Dumazet
2017-02-04 23:25 ` [PATCH v2 net-next 11/12] aeroflex/greth: " Eric Dumazet
2017-02-04 23:25 ` [PATCH v2 net-next 12/12] net: remove __napi_complete() Eric Dumazet
2017-02-05 22:16 ` [PATCH v2 net-next 00/12] net: get rid of __napi_complete() David Miller

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