Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] net: In unregister_netdevice_notifier unregister the netdevices.
From: David Miller @ 2012-04-13 15:05 UTC (permalink / raw)
  To: ebiederm
  Cc: eric.dumazet, ericvh, davej, linux-kernel, netdev, levinsasha928,
	remi.denis-courmont
In-Reply-To: <m1aa2otjf4.fsf_-_@fess.ebiederm.org>

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Fri, 06 Apr 2012 18:33:35 -0700

>>From a51fda849355b2ffb27cdb5d66fa1a96e7f47335 Mon Sep 17 00:00:00 2001
> From: Eric W. Biederman <ebiederm@xmission.com>
> Date: Sun, 25 Dec 2011 19:39:52 -0800
> Subject: 

Yikes, I had to edit this turd out :-)  Otherwise with this empty
Subject, GIT uses the first line of your commit log body as the
first line.

> We already synthesize events in register_netdevice_notifier and synthesizing
> events in unregister_netdevice_notifier allows to us remove the need for
> special case cleanup code.
> 
> This change should be safe as it adds no new cases for existing callers
> of unregiser_netdevice_notifier to handle.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>

Applied.

^ permalink raw reply

* Re: [PATCH 2/2] phonet: Sort out initiailziation and cleanup code.
From: David Miller @ 2012-04-13 15:05 UTC (permalink / raw)
  To: ebiederm
  Cc: eric.dumazet, ericvh, davej, linux-kernel, netdev, levinsasha928,
	remi.denis-courmont
In-Reply-To: <m162dctjbo.fsf_-_@fess.ebiederm.org>

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Fri, 06 Apr 2012 18:35:39 -0700

> 
> Recently an oops was reported in phonet if there was a failure during
> network namespace creation.
 ...
> After investigation it turns out there were two issues.
> 1) Phonet was not implementing network devices but was using register_pernet_device
>    instead of register_pernet_subsys.
> 
>    This was allowing there to be cases when phonenet was not initialized and
>    the phonet net_generic was not set for a network namespace when network
>    device events were being reported on the netdevice_notifier for a network
>    namespace leading to the oops above.
> 
> 2) phonet_exit_net was implementing a confusing and special case of handling all
>    network devices from going away that it was hard to see was correct, and would
>    only occur when the phonet module was removed.
> 
>    Now that unregister_netdevice_notifier has been modified to synthesize unregistration
>    events for the network devices that are extant when called this confusing special
>    case in phonet_exit_net is no longer needed.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>

Also applied, thanks Eric.

^ permalink raw reply

* Re: [PATCH]Fix misplaces parenthesis in virtio_net.c
From: David Miller @ 2012-04-13 15:05 UTC (permalink / raw)
  To: just.for.lkml; +Cc: rick.jones2, netdev, linux-kernel
In-Reply-To: <20120409171415.4c49c96a@googlemail.com>

From: Torsten Kaiser <just.for.lkml@googlemail.com>
Date: Mon, 9 Apr 2012 17:14:15 +0200

> Commit 2e57b79ccef1ff1422fdf45a9b28fe60f8f084f7 misplaced its
> parenthesis and now tx_fifo_errors will only be incremented if an
> ENOMEM error is not written to the syslog.
> 
> Correct the parenthesis and indentation to the original goal of
> counting all non ENOMEM errors and ratelimiting only the messages.
> 
> Signed-of-by: Torsten Kaiser <just.for.lkml@googlemail.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH 0/2] l2tp: l2tp_ip socket fixes
From: David Miller @ 2012-04-13 15:06 UTC (permalink / raw)
  To: jchapman; +Cc: netdev
In-Reply-To: <1334052643-10910-1-git-send-email-jchapman@katalix.com>

From: James Chapman <jchapman@katalix.com>
Date: Tue, 10 Apr 2012 11:10:41 +0100

> A couple of small fixes for the l2tp_ip socket support. This is for
> L2TPv3 IP link encapsulation.
> 
> [PATCH 1/2] l2tp: fix refcount leak in l2tp_ip sockets
> [PATCH 2/2] l2tp: don't overwrite source address in l2tp_ip_bind()

Both applied, although I hope patch #2 doesn't unintentionally break
things for someone.  It's not unreasonable to think that someone may
be depending upon that saddr being set to INADDR_ANY in that case
you're changing.

^ permalink raw reply

* Re: [PATCH] net/key/af_key.c: add missing kfree_skb
From: David Miller @ 2012-04-13 15:05 UTC (permalink / raw)
  To: Julia.Lawall; +Cc: kernel-janitors, netdev, linux-kernel
In-Reply-To: <1333960870-19735-1-git-send-email-Julia.Lawall@lip6.fr>

From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Mon,  9 Apr 2012 10:41:10 +0200

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> At the point of this error-handling code, alloc_skb has succeded, so free
> the resulting skb by jumping to the err label.
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Applied.

^ permalink raw reply

* Re: [PATCH net 0/3] CAIF Bug-fixes.
From: David Miller @ 2012-04-13 15:07 UTC (permalink / raw)
  To: sjur.brandeland; +Cc: netdev, sjurbren
In-Reply-To: <1334254689-2843-1-git-send-email-sjur.brandeland@stericsson.com>

From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Date: Thu, 12 Apr 2012 20:18:06 +0200

> Some simple bug fixes: plugging memory leaks and fixing the
> log-level from err to dbg. I suppose these should go into v3.4.

All applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next 0/4] New CAIF features
From: David Miller @ 2012-04-13 15:37 UTC (permalink / raw)
  To: sjur.brandeland; +Cc: netdev, sjurbren
In-Reply-To: <1334255247-3372-1-git-send-email-sjur.brandeland@stericsson.com>

From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Date: Thu, 12 Apr 2012 20:27:23 +0200

> Add feature to aggregate CAIF HSI frames into one large HSI-transfer.
> The triggers for starting a HSI transfer are:
> Max transfer size reached, timeout or reception of low-latency traffic. 
> As long as none of these triggers are met, CAIF-frames will be aggregated.
>  
> In order to be able to support aggregation and detect low-latency traffic,
> CAIF packets are assigned a TC class.
>  
> In addition we are now allowing other kernel modules to use the HSI interface
> while the CAIF interface is closed. The CAIF-HSI interface is changed so that
> reference to the HSI interface is held only when CAIF-HSI is in state UP.
> Initialization/de-initialization of the HSI HW is performed when the interface
> is opened/closed instead of at module load time.
> 
> NOTE: This patch-set depends on the patch "caif-hsi: Free flip_buffer at shutdown",
> sent in the Bug-fix patch-set.

All applied, thanks for letting me know explicitly about the dependency.

^ permalink raw reply

* [PATCH 0/9] atl1: add napi to Atheros L1 NIC
From: Tony Zelenoff @ 2012-04-13 16:09 UTC (permalink / raw)
  To: davem; +Cc: antonz, netdev, jcliburn, csnook, khorenko

Added NAPI interface to the driver of Atheros L1 NIC.
RX speed reported by iperf in TCP mode (with and without -N
switch) improved by ~7 Mbps. In UDP mode the speed up is
improved by ~4 Mbps.
Also hard RX/TX rings processing with memory allocation in
interrupt handlers is not very good idea. So, fixed this.

Note: thouse patches must be applied after patch, that fix
kernel panic in this driver.
Its subject is: "atl1: fix kernel panic in case of DMA errors"

Tony Zelenoff (9):
  atl1: handle rx in separate condition
  atl1: make driver napi compatible
  atl1: add napi process of tx interrupts
  atl1: use defined functions to disable irq
  atl1: make function to set imr of card
  atl1: add value to check ability of reenabling IRQs
  atl1: enable errors and link ints when rx/tx scheduled
  atl1: do not process interrupts in cycle in handler
  atl1: do not drop rx/tx interrupts before they are scheduled

 drivers/net/ethernet/atheros/atlx/atl1.c |  171 +++++++++++++++++++-----------
 drivers/net/ethernet/atheros/atlx/atl1.h |   17 +++-
 drivers/net/ethernet/atheros/atlx/atlx.c |   15 ++-
 3 files changed, 135 insertions(+), 68 deletions(-)

^ permalink raw reply

* [PATCH 1/9] atl1: handle rx in separate condition
From: Tony Zelenoff @ 2012-04-13 16:09 UTC (permalink / raw)
  To: davem; +Cc: antonz, netdev, jcliburn, csnook, khorenko
In-Reply-To: <1334333394-38404-1-git-send-email-antonz@parallels.com>

Remove rx from unlikely optimization in case of rx is very
likely thing for network card. This also reduce code a bit.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index c926857..6438239 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -2502,18 +2502,19 @@ static irqreturn_t atl1_intr(int irq, void *data)
 		if (status & ISR_CMB_TX)
 			atl1_intr_tx(adapter);
 
+		/* rx event */
+		if (status & ISR_CMB_RX)
+			alt1_intr_rx(adapter);
+
 		/* rx exception */
 		if (unlikely(status & (ISR_RXF_OV | ISR_RFD_UNRUN |
 			ISR_RRD_OV | ISR_HOST_RFD_UNRUN |
-			ISR_HOST_RRD_OV | ISR_CMB_RX))) {
-			if (status & (ISR_RXF_OV | ISR_RFD_UNRUN |
-				ISR_RRD_OV | ISR_HOST_RFD_UNRUN |
-				ISR_HOST_RRD_OV))
-				if (netif_msg_intr(adapter))
-					dev_printk(KERN_DEBUG,
-						&adapter->pdev->dev,
-						"rx exception, ISR = 0x%x\n",
-						status);
+			ISR_HOST_RRD_OV))) {
+			if (netif_msg_intr(adapter))
+				dev_printk(KERN_DEBUG,
+					&adapter->pdev->dev,
+					"rx exception, ISR = 0x%x\n",
+					status);
 			atl1_intr_rx(adapter);
 		}
 
-- 
1.7.1

^ permalink raw reply related

* [PATCH 4/9] atl1: use defined functions to disable irq
From: Tony Zelenoff @ 2012-04-13 16:09 UTC (permalink / raw)
  To: davem; +Cc: antonz, netdev, jcliburn, csnook, khorenko
In-Reply-To: <1334333394-38404-1-git-send-email-antonz@parallels.com>

Looks like direct writes to IMR register is not good idea,
because there are exist functions to make this work.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index a8935df..4fcdf57 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -266,7 +266,7 @@ static s32 atl1_reset_hw(struct atl1_hw *hw)
 	 * interrupts & Clear any pending interrupt events
 	 */
 	/*
-	 * iowrite32(0, hw->hw_addr + REG_IMR);
+	 * atlx_irq_disable(adapter);
 	 * iowrite32(0xffffffff, hw->hw_addr + REG_ISR);
 	 */
 
@@ -2512,7 +2512,7 @@ static irqreturn_t atl1_intr(int irq, void *data)
 				dev_printk(KERN_DEBUG, &adapter->pdev->dev,
 					"pcie phy link down %x\n", status);
 			if (netif_running(adapter->netdev)) {	/* reset MAC */
-				iowrite32(0, adapter->hw.hw_addr + REG_IMR);
+				atlx_irq_disable(adapter);
 				schedule_work(&adapter->reset_dev_task);
 				return IRQ_HANDLED;
 			}
@@ -2524,7 +2524,7 @@ static irqreturn_t atl1_intr(int irq, void *data)
 				dev_printk(KERN_DEBUG, &adapter->pdev->dev,
 					"pcie DMA r/w error (status = 0x%x)\n",
 					status);
-			iowrite32(0, adapter->hw.hw_addr + REG_IMR);
+			atlx_irq_disable(adapter);
 			schedule_work(&adapter->reset_dev_task);
 			return IRQ_HANDLED;
 		}
-- 
1.7.1

^ permalink raw reply related

* [PATCH 2/9] atl1: make driver napi compatible
From: Tony Zelenoff @ 2012-04-13 16:09 UTC (permalink / raw)
  To: davem; +Cc: antonz, netdev, jcliburn, csnook, khorenko
In-Reply-To: <1334333394-38404-1-git-send-email-antonz@parallels.com>

This is first step, here there is no fine interrupt
disabling which cause TX/ERR interrupts stalling when
RX scheduled ints processed.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c |   46 ++++++++++++++++++++++++++----
 drivers/net/ethernet/atheros/atlx/atl1.h |    1 +
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index 6438239..d39c1b9 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -1917,7 +1917,7 @@ next:
 	return num_alloc;
 }
 
-static void atl1_intr_rx(struct atl1_adapter *adapter)
+static int atl1_intr_rx(struct atl1_adapter *adapter, int budget)
 {
 	int i, count;
 	u16 length;
@@ -1933,7 +1933,7 @@ static void atl1_intr_rx(struct atl1_adapter *adapter)
 
 	rrd_next_to_clean = atomic_read(&rrd_ring->next_to_clean);
 
-	while (1) {
+	while (count <= budget) {
 		rrd = ATL1_RRD_DESC(rrd_ring, rrd_next_to_clean);
 		i = 1;
 		if (likely(rrd->xsz.valid)) {	/* packet valid */
@@ -2032,7 +2032,7 @@ rrd_ok:
 
 			__vlan_hwaccel_put_tag(skb, vlan_tag);
 		}
-		netif_rx(skb);
+		netif_receive_skb(skb);
 
 		/* let protocol layer free skb */
 		buffer_info->skb = NULL;
@@ -2065,6 +2065,8 @@ rrd_ok:
 		iowrite32(value, adapter->hw.hw_addr + REG_MAILBOX);
 		spin_unlock(&adapter->mb_lock);
 	}
+
+	return count;
 }
 
 static void atl1_intr_tx(struct atl1_adapter *adapter)
@@ -2439,6 +2441,33 @@ static netdev_tx_t atl1_xmit_frame(struct sk_buff *skb,
 	return NETDEV_TX_OK;
 }
 
+static int atl1_rx_clean(struct napi_struct *napi, int budget)
+{
+	struct atl1_adapter *adapter = container_of(napi, struct atl1_adapter, napi);
+	int work_done = atl1_intr_rx(adapter, budget);
+
+	/* Let's come again to process some more packets */
+	if (work_done >= budget)
+		return work_done;
+
+	napi_complete(napi);
+	/* re-enable Interrupt */
+	iowrite32(ISR_DIS_SMB | ISR_DIS_DMA, adapter->hw.hw_addr + REG_ISR);
+	return work_done;
+}
+
+static inline int atl1_sched_rx(struct atl1_adapter* adapter)
+{
+	if (likely(napi_schedule_prep(&adapter->napi))) {
+		__napi_schedule(&adapter->napi);
+		return 1;
+	}
+
+	dev_printk(KERN_ERR, &adapter->pdev->dev,
+		   "rx: INTs must be disabled!");
+	return 0;
+}
+
 /*
  * atl1_intr - Interrupt Handler
  * @irq: interrupt number
@@ -2503,8 +2532,9 @@ static irqreturn_t atl1_intr(int irq, void *data)
 			atl1_intr_tx(adapter);
 
 		/* rx event */
-		if (status & ISR_CMB_RX)
-			alt1_intr_rx(adapter);
+		if (status & ISR_CMB_RX && atl1_sched_rx(adapter))
+			/* Go away with INTs disabled */
+			return IRQ_HANDLED;
 
 		/* rx exception */
 		if (unlikely(status & (ISR_RXF_OV | ISR_RFD_UNRUN |
@@ -2515,7 +2545,8 @@ static irqreturn_t atl1_intr(int irq, void *data)
 					&adapter->pdev->dev,
 					"rx exception, ISR = 0x%x\n",
 					status);
-			atl1_intr_rx(adapter);
+			if (atl1_sched_rx(adapter))
+				return IRQ_HANDLED;
 		}
 
 		if (--max_ints < 0)
@@ -2600,6 +2631,7 @@ static s32 atl1_up(struct atl1_adapter *adapter)
 	if (unlikely(err))
 		goto err_up;
 
+	napi_enable(&adapter->napi);
 	atlx_irq_enable(adapter);
 	atl1_check_link(adapter);
 	netif_start_queue(netdev);
@@ -2616,6 +2648,7 @@ static void atl1_down(struct atl1_adapter *adapter)
 {
 	struct net_device *netdev = adapter->netdev;
 
+	napi_disable(&adapter->napi);
 	netif_stop_queue(netdev);
 	del_timer_sync(&adapter->phy_config_timer);
 	adapter->phy_timer_pending = false;
@@ -2972,6 +3005,7 @@ static int __devinit atl1_probe(struct pci_dev *pdev,
 
 	netdev->netdev_ops = &atl1_netdev_ops;
 	netdev->watchdog_timeo = 5 * HZ;
+	netif_napi_add(netdev, &adapter->napi, atl1_rx_clean, 64);
 
 	netdev->ethtool_ops = &atl1_ethtool_ops;
 	adapter->bd_number = cards_found;
diff --git a/drivers/net/ethernet/atheros/atlx/atl1.h b/drivers/net/ethernet/atheros/atlx/atl1.h
index e04bf4d..b55084c 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.h
+++ b/drivers/net/ethernet/atheros/atlx/atl1.h
@@ -758,6 +758,7 @@ struct atl1_adapter {
 	u16 link_speed;
 	u16 link_duplex;
 	spinlock_t lock;
+	struct napi_struct napi;
 	struct work_struct reset_dev_task;
 	struct work_struct link_chg_task;
 
-- 
1.7.1

^ permalink raw reply related

* [PATCH 3/9] atl1: add napi process of tx interrupts
From: Tony Zelenoff @ 2012-04-13 16:09 UTC (permalink / raw)
  To: davem; +Cc: antonz, netdev, jcliburn, csnook, khorenko
In-Reply-To: <1334333394-38404-1-git-send-email-antonz@parallels.com>

Make the tx ints processing same as rx ones via napi.
The idea got from e1000. The interrupt disabling is
still not fine grained.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index d39c1b9..a8935df 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -2069,12 +2069,13 @@ rrd_ok:
 	return count;
 }
 
-static void atl1_intr_tx(struct atl1_adapter *adapter)
+static int atl1_intr_tx(struct atl1_adapter *adapter)
 {
 	struct atl1_tpd_ring *tpd_ring = &adapter->tpd_ring;
 	struct atl1_buffer *buffer_info;
 	u16 sw_tpd_next_to_clean;
 	u16 cmb_tpd_next_to_clean;
+	int count = 0;
 
 	sw_tpd_next_to_clean = atomic_read(&tpd_ring->next_to_clean);
 	cmb_tpd_next_to_clean = le16_to_cpu(adapter->cmb.cmb->tpd_cons_idx);
@@ -2094,12 +2095,16 @@ static void atl1_intr_tx(struct atl1_adapter *adapter)
 
 		if (++sw_tpd_next_to_clean == tpd_ring->count)
 			sw_tpd_next_to_clean = 0;
+
+		count++;
 	}
 	atomic_set(&tpd_ring->next_to_clean, sw_tpd_next_to_clean);
 
 	if (netif_queue_stopped(adapter->netdev) &&
 	    netif_carrier_ok(adapter->netdev))
 		netif_wake_queue(adapter->netdev);
+
+	return count;
 }
 
 static u16 atl1_tpd_avail(struct atl1_tpd_ring *tpd_ring)
@@ -2441,11 +2446,14 @@ static netdev_tx_t atl1_xmit_frame(struct sk_buff *skb,
 	return NETDEV_TX_OK;
 }
 
-static int atl1_rx_clean(struct napi_struct *napi, int budget)
+static int atl1_rings_clean(struct napi_struct *napi, int budget)
 {
 	struct atl1_adapter *adapter = container_of(napi, struct atl1_adapter, napi);
 	int work_done = atl1_intr_rx(adapter, budget);
 
+	if (atl1_intr_tx(adapter))
+		work_done = budget;
+
 	/* Let's come again to process some more packets */
 	if (work_done >= budget)
 		return work_done;
@@ -2456,7 +2464,7 @@ static int atl1_rx_clean(struct napi_struct *napi, int budget)
 	return work_done;
 }
 
-static inline int atl1_sched_rx(struct atl1_adapter* adapter)
+static inline int atl1_sched_rings_clean(struct atl1_adapter* adapter)
 {
 	if (likely(napi_schedule_prep(&adapter->napi))) {
 		__napi_schedule(&adapter->napi);
@@ -2527,12 +2535,9 @@ static irqreturn_t atl1_intr(int irq, void *data)
 			atl1_check_for_link(adapter);
 		}
 
-		/* transmit event */
-		if (status & ISR_CMB_TX)
-			atl1_intr_tx(adapter);
-
-		/* rx event */
-		if (status & ISR_CMB_RX && atl1_sched_rx(adapter))
+		/* transmit or receive event */
+		if (status & (ISR_CMB_TX | ISR_CMB_RX) &&
+		    atl1_sched_rings_clean(adapter))
 			/* Go away with INTs disabled */
 			return IRQ_HANDLED;
 
@@ -2545,7 +2550,7 @@ static irqreturn_t atl1_intr(int irq, void *data)
 					&adapter->pdev->dev,
 					"rx exception, ISR = 0x%x\n",
 					status);
-			if (atl1_sched_rx(adapter))
+			if (atl1_sched_rings_clean(adapter))
 				return IRQ_HANDLED;
 		}
 
@@ -3005,7 +3010,7 @@ static int __devinit atl1_probe(struct pci_dev *pdev,
 
 	netdev->netdev_ops = &atl1_netdev_ops;
 	netdev->watchdog_timeo = 5 * HZ;
-	netif_napi_add(netdev, &adapter->napi, atl1_rx_clean, 64);
+	netif_napi_add(netdev, &adapter->napi, atl1_rings_clean, 64);
 
 	netdev->ethtool_ops = &atl1_ethtool_ops;
 	adapter->bd_number = cards_found;
-- 
1.7.1

^ permalink raw reply related

* [PATCH 5/9] atl1: make function to set imr of card
From: Tony Zelenoff @ 2012-04-13 16:09 UTC (permalink / raw)
  To: davem; +Cc: antonz, netdev, jcliburn, csnook, khorenko
In-Reply-To: <1334333394-38404-1-git-send-email-antonz@parallels.com>

This function should be used later to set/remove proper
bits in imr to disable only rx ints.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atlx.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atlx.c b/drivers/net/ethernet/atheros/atlx/atlx.c
index c9e9dc5..6c4a706 100644
--- a/drivers/net/ethernet/atheros/atlx/atlx.c
+++ b/drivers/net/ethernet/atheros/atlx/atlx.c
@@ -155,14 +155,20 @@ static void atlx_set_multi(struct net_device *netdev)
 	}
 }
 
+static inline void atlx_imr_set(struct atlx_adapter *adapter,
+				unsigned int imr)
+{
+	iowrite32(imr, adapter->hw.hw_addr + REG_IMR);
+	ioread32(adapter->hw.hw_addr + REG_IMR);
+}
+
 /*
  * atlx_irq_enable - Enable default interrupt generation settings
  * @adapter: board private structure
  */
 static void atlx_irq_enable(struct atlx_adapter *adapter)
 {
-	iowrite32(IMR_NORMAL_MASK, adapter->hw.hw_addr + REG_IMR);
-	ioread32(adapter->hw.hw_addr + REG_IMR);
+	atlx_imr_set(adapter, IMR_NORMAL_MASK);
 }
 
 /*
@@ -171,8 +177,7 @@ static void atlx_irq_enable(struct atlx_adapter *adapter)
  */
 static void atlx_irq_disable(struct atlx_adapter *adapter)
 {
-	iowrite32(0, adapter->hw.hw_addr + REG_IMR);
-	ioread32(adapter->hw.hw_addr + REG_IMR);
+	atlx_imr_set(adapter, 0);
 	synchronize_irq(adapter->pdev->irq);
 }
 
-- 
1.7.1

^ permalink raw reply related

* [PATCH 6/9] atl1: add value to check ability of reenabling IRQs
From: Tony Zelenoff @ 2012-04-13 16:09 UTC (permalink / raw)
  To: davem; +Cc: antonz, netdev, jcliburn, csnook, khorenko
In-Reply-To: <1334333394-38404-1-git-send-email-antonz@parallels.com>

Unfortunately it is not clear from code is usage of
IMR register possible or not. So, to prevent possible
side-effects of reading this register i prefer store
interrupts enable flag separately.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.h |    6 ++++++
 drivers/net/ethernet/atheros/atlx/atlx.c |    2 ++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.h b/drivers/net/ethernet/atheros/atlx/atl1.h
index b55084c..5209875 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.h
+++ b/drivers/net/ethernet/atheros/atlx/atl1.h
@@ -782,6 +782,12 @@ struct atl1_adapter {
 	u16 ict;		/* interrupt clear timer (2us resolution */
 	struct mii_if_info mii;	/* MII interface info */
 
+	/*
+	 * Use this value to check is napi handler allowed to
+	 * enable ints or not
+	 */
+	bool int_enabled;
+
 	u32 bd_number;		/* board number */
 	bool pci_using_64;
 	struct atl1_hw hw;
diff --git a/drivers/net/ethernet/atheros/atlx/atlx.c b/drivers/net/ethernet/atheros/atlx/atlx.c
index 6c4a706..b4f3aa4 100644
--- a/drivers/net/ethernet/atheros/atlx/atlx.c
+++ b/drivers/net/ethernet/atheros/atlx/atlx.c
@@ -169,6 +169,7 @@ static inline void atlx_imr_set(struct atlx_adapter *adapter,
 static void atlx_irq_enable(struct atlx_adapter *adapter)
 {
 	atlx_imr_set(adapter, IMR_NORMAL_MASK);
+	adapter->int_enabled = true;
 }
 
 /*
@@ -177,6 +178,7 @@ static void atlx_irq_enable(struct atlx_adapter *adapter)
  */
 static void atlx_irq_disable(struct atlx_adapter *adapter)
 {
+	adapter->int_enabled = false;
 	atlx_imr_set(adapter, 0);
 	synchronize_irq(adapter->pdev->irq);
 }
-- 
1.7.1

^ permalink raw reply related

* [PATCH 7/9] atl1: enable errors and link ints when rx/tx scheduled
From: Tony Zelenoff @ 2012-04-13 16:09 UTC (permalink / raw)
  To: davem; +Cc: antonz, netdev, jcliburn, csnook, khorenko
In-Reply-To: <1334333394-38404-1-git-send-email-antonz@parallels.com>

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c |   32 ++++++++++++++++++++---------
 drivers/net/ethernet/atheros/atlx/atl1.h |   10 ++++++--
 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index 4fcdf57..5781597 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -2460,20 +2460,33 @@ static int atl1_rings_clean(struct napi_struct *napi, int budget)
 
 	napi_complete(napi);
 	/* re-enable Interrupt */
-	iowrite32(ISR_DIS_SMB | ISR_DIS_DMA, adapter->hw.hw_addr + REG_ISR);
+	if (likely(adapter->int_enabled))
+		atlx_imr_set(adapter, IMR_NORMAL_MASK);
 	return work_done;
 }
 
 static inline int atl1_sched_rings_clean(struct atl1_adapter* adapter)
 {
-	if (likely(napi_schedule_prep(&adapter->napi))) {
-		__napi_schedule(&adapter->napi);
+	if (!napi_schedule_prep(&adapter->napi))
+		/* It is possible in case even the RX/TX ints are disabled via IMR
+		 * register the ISR bits are set anyway (but do not produce IRQ).
+		 * To handle such situation the napi functions used to check is
+		 * something scheduled or not.
+		 */
+		return 0;
+
+	__napi_schedule(&adapter->napi);
+
+	/*
+	 * Disable RX/TX ints via IMR register if it is
+	 * allowed. NAPI handler must reenable them in same
+	 * way.
+	 */
+	if (!adapter->int_enabled)
 		return 1;
-	}
 
-	dev_printk(KERN_ERR, &adapter->pdev->dev,
-		   "rx: INTs must be disabled!");
-	return 0;
+	atlx_imr_set(adapter, IMR_NORXTX_MASK);
+	return 1;
 }
 
 /*
@@ -2538,8 +2551,7 @@ static irqreturn_t atl1_intr(int irq, void *data)
 		/* transmit or receive event */
 		if (status & (ISR_CMB_TX | ISR_CMB_RX) &&
 		    atl1_sched_rings_clean(adapter))
-			/* Go away with INTs disabled */
-			return IRQ_HANDLED;
+			break;
 
 		/* rx exception */
 		if (unlikely(status & (ISR_RXF_OV | ISR_RFD_UNRUN |
@@ -2551,7 +2563,7 @@ static irqreturn_t atl1_intr(int irq, void *data)
 					"rx exception, ISR = 0x%x\n",
 					status);
 			if (atl1_sched_rings_clean(adapter))
-				return IRQ_HANDLED;
+				break;
 		}
 
 		if (--max_ints < 0)
diff --git a/drivers/net/ethernet/atheros/atlx/atl1.h b/drivers/net/ethernet/atheros/atlx/atl1.h
index 5209875..3bf79a5 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.h
+++ b/drivers/net/ethernet/atheros/atlx/atl1.h
@@ -275,13 +275,17 @@ static u32 atl1_check_link(struct atl1_adapter *adapter);
 #define ISR_DIS_SMB				0x20000000
 #define ISR_DIS_DMA				0x40000000
 
-/* Normal Interrupt mask  */
-#define IMR_NORMAL_MASK	(\
+/* Normal Interrupt mask without RX/TX enabled */
+#define IMR_NORXTX_MASK	(\
 	ISR_SMB		|\
 	ISR_GPHY	|\
 	ISR_PHY_LINKDOWN|\
 	ISR_DMAR_TO_RST	|\
-	ISR_DMAW_TO_RST	|\
+	ISR_DMAW_TO_RST)
+
+/* Normal Interrupt mask  */
+#define IMR_NORMAL_MASK	(\
+	IMR_NORXTX_MASK	|\
 	ISR_CMB_TX	|\
 	ISR_CMB_RX)
 
-- 
1.7.1

^ permalink raw reply related

* [PATCH 8/9] atl1: do not process interrupts in cycle in handler
From: Tony Zelenoff @ 2012-04-13 16:09 UTC (permalink / raw)
  To: davem; +Cc: antonz, netdev, jcliburn, csnook, khorenko
In-Reply-To: <1334333394-38404-1-git-send-email-antonz@parallels.com>

As the rx/tx handled inside napi handler, the cycle is
not needed now, because only the rx/tx need such kind of
processing.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c |  101 ++++++++++++++----------------
 1 files changed, 46 insertions(+), 55 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index 5781597..3c57a30 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -2499,77 +2499,68 @@ static irqreturn_t atl1_intr(int irq, void *data)
 {
 	struct atl1_adapter *adapter = netdev_priv(data);
 	u32 status;
-	int max_ints = 10;
 
 	status = adapter->cmb.cmb->int_stats;
 	if (!status)
 		return IRQ_NONE;
 
-	do {
-		/* clear CMB interrupt status at once */
-		adapter->cmb.cmb->int_stats = 0;
-
-		if (status & ISR_GPHY)	/* clear phy status */
-			atlx_clear_phy_int(adapter);
+	/* clear CMB interrupt status at once */
+	adapter->cmb.cmb->int_stats = 0;
 
-		/* clear ISR status, and Enable CMB DMA/Disable Interrupt */
-		iowrite32(status | ISR_DIS_INT, adapter->hw.hw_addr + REG_ISR);
+	if (status & ISR_GPHY)	/* clear phy status */
+		atlx_clear_phy_int(adapter);
 
-		/* check if SMB intr */
-		if (status & ISR_SMB)
-			atl1_inc_smb(adapter);
+	/* clear ISR status, and Enable CMB DMA/Disable Interrupt */
+	iowrite32(status | ISR_DIS_INT, adapter->hw.hw_addr + REG_ISR);
 
-		/* check if PCIE PHY Link down */
-		if (status & ISR_PHY_LINKDOWN) {
-			if (netif_msg_intr(adapter))
-				dev_printk(KERN_DEBUG, &adapter->pdev->dev,
-					"pcie phy link down %x\n", status);
-			if (netif_running(adapter->netdev)) {	/* reset MAC */
-				atlx_irq_disable(adapter);
-				schedule_work(&adapter->reset_dev_task);
-				return IRQ_HANDLED;
-			}
-		}
+	/* check if SMB intr */
+	if (status & ISR_SMB)
+		atl1_inc_smb(adapter);
 
-		/* check if DMA read/write error ? */
-		if (status & (ISR_DMAR_TO_RST | ISR_DMAW_TO_RST)) {
-			if (netif_msg_intr(adapter))
-				dev_printk(KERN_DEBUG, &adapter->pdev->dev,
-					"pcie DMA r/w error (status = 0x%x)\n",
-					status);
+	/* check if PCIE PHY Link down */
+	if (status & ISR_PHY_LINKDOWN) {
+		if (netif_msg_intr(adapter))
+			dev_printk(KERN_DEBUG, &adapter->pdev->dev,
+				"pcie phy link down %x\n", status);
+		if (netif_running(adapter->netdev)) {	/* reset MAC */
 			atlx_irq_disable(adapter);
 			schedule_work(&adapter->reset_dev_task);
 			return IRQ_HANDLED;
 		}
+	}
 
-		/* link event */
-		if (status & ISR_GPHY) {
-			adapter->soft_stats.tx_carrier_errors++;
-			atl1_check_for_link(adapter);
-		}
-
-		/* transmit or receive event */
-		if (status & (ISR_CMB_TX | ISR_CMB_RX) &&
-		    atl1_sched_rings_clean(adapter))
-			break;
-
-		/* rx exception */
-		if (unlikely(status & (ISR_RXF_OV | ISR_RFD_UNRUN |
-			ISR_RRD_OV | ISR_HOST_RFD_UNRUN |
-			ISR_HOST_RRD_OV))) {
-			if (netif_msg_intr(adapter))
-				dev_printk(KERN_DEBUG,
-					&adapter->pdev->dev,
-					"rx exception, ISR = 0x%x\n",
-					status);
-			if (atl1_sched_rings_clean(adapter))
-				break;
-		}
-
-		if (--max_ints < 0)
-			break;
-
-	} while ((status = adapter->cmb.cmb->int_stats));
+	/* check if DMA read/write error ? */
+	if (status & (ISR_DMAR_TO_RST | ISR_DMAW_TO_RST)) {
+		if (netif_msg_intr(adapter))
+			dev_printk(KERN_DEBUG, &adapter->pdev->dev,
+				"pcie DMA r/w error (status = 0x%x)\n",
+				status);
+		atlx_irq_disable(adapter);
+		schedule_work(&adapter->reset_dev_task);
+		return IRQ_HANDLED;
+	}
+
+	/* link event */
+	if (status & ISR_GPHY) {
+		adapter->soft_stats.tx_carrier_errors++;
+		atl1_check_for_link(adapter);
+	}
+
+	/* transmit or receive event */
+	if (status & (ISR_CMB_TX | ISR_CMB_RX))
+	    atl1_sched_rings_clean(adapter);
+
+	/* rx exception */
+	if (unlikely(status & (ISR_RXF_OV | ISR_RFD_UNRUN |
+		ISR_RRD_OV | ISR_HOST_RFD_UNRUN |
+		ISR_HOST_RRD_OV))) {
+		if (netif_msg_intr(adapter))
+			dev_printk(KERN_DEBUG,
+				&adapter->pdev->dev,
+				"rx exception, ISR = 0x%x\n",
+				status);
+		atl1_sched_rings_clean(adapter);
+	}
 
 	/* re-enable Interrupt */
 	iowrite32(ISR_DIS_SMB | ISR_DIS_DMA, adapter->hw.hw_addr + REG_ISR);
-- 
1.7.1

^ permalink raw reply related

* [PATCH 9/9] atl1: do not drop rx/tx interrupts before they are scheduled
From: Tony Zelenoff @ 2012-04-13 16:09 UTC (permalink / raw)
  To: davem; +Cc: antonz, netdev, jcliburn, csnook, khorenko
In-Reply-To: <1334333394-38404-1-git-send-email-antonz@parallels.com>

To prevent interrupts lost they should be dropped only if
they are scheduled via napi interfaces. In other case, there is
exists situation when napi handler process TX interrupt, stay in
RX processing and in that moment any other interrupt received.
Then before this patch TX bit in ISR will be cleaned, napi
schedule will not occur in case of currently processing event and
TX interrupt definitely will be lost.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index 3c57a30..a0b87b1 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -2504,8 +2504,12 @@ static irqreturn_t atl1_intr(int irq, void *data)
 	if (!status)
 		return IRQ_NONE;
 
-	/* clear CMB interrupt status at once */
-	adapter->cmb.cmb->int_stats = 0;
+	/* clear CMB interrupt status at once,
+	 * but leave rx/tx interrupt status in case it should be dropped
+	 * only if rx/tx processing queued. In other case interrupt
+	 * can be lost.
+	 */
+	adapter->cmb.cmb->int_stats = status & (ISR_CMB_TX | ISR_CMB_RX);
 
 	if (status & ISR_GPHY)	/* clear phy status */
 		atlx_clear_phy_int(adapter);
@@ -2547,8 +2551,10 @@ static irqreturn_t atl1_intr(int irq, void *data)
 	}
 
 	/* transmit or receive event */
-	if (status & (ISR_CMB_TX | ISR_CMB_RX))
-	    atl1_sched_rings_clean(adapter);
+	if (status & (ISR_CMB_TX | ISR_CMB_RX) &&
+	    atl1_sched_rings_clean(adapter))
+		adapter->cmb.cmb->int_stats = adapter->cmb.cmb->int_stats &
+					      ~(ISR_CMB_TX | ISR_CMB_RX);
 
 	/* rx exception */
 	if (unlikely(status & (ISR_RXF_OV | ISR_RFD_UNRUN |
-- 
1.7.1

^ permalink raw reply related

* Re: suspicious RCU usage warnings in 3.3.0
From: Paul E. McKenney @ 2012-04-13 16:30 UTC (permalink / raw)
  To: David Miller; +Cc: mroos, shemminger, linux-kernel, netdev
In-Reply-To: <20120413.105556.73360716787161031.davem@davemloft.net>

On Fri, Apr 13, 2012 at 10:55:56AM -0400, David Miller wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Date: Fri, 13 Apr 2012 06:35:13 -0700
> 
> DANGER WILL ROBINSON, McKenney is hacking sparc64 assembler!
> :-))))

;-) ;-) ;-)

> > My main concern about my patch is my removal of this line:
> > 
> > 	bne,pn			%icc, __handle_softirq
> > 
> > It is quite possible that this should instead change to look as follows:
> > 
> > 	bne,pn			%icc, __handle_preemption
> > 
> 
> I would say it should not, because the __handle_softirq code path,
> if taken, unconditionally branches to the very next instruction
> after this branch and it's delay slot.

Beginners luck beats skill.  Now I just have to be very careful to
avoid ever submitting a second sparc64 assembler patch.  ;-)

> > Anyway, patch with updated changelog below.
> 
> Great work, I'll apply this after I test it out a bit myself.

Cool!!!

							Thanx, Paul

> Thanks a lot Paul!
> 

^ permalink raw reply

* Re: [PATCH v6] ipv6: fix problem with expired dst cache
From: David Miller @ 2012-04-13 16:58 UTC (permalink / raw)
  To: gaofeng; +Cc: netdev
In-Reply-To: <1333707190-14075-1-git-send-email-gaofeng@cn.fujitsu.com>

From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Fri, 6 Apr 2012 18:13:10 +0800

> If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
> this dst cache will not check expire because it has no RTF_EXPIRES flag.
> So this dst cache will always be used until the dst gc run.
> 
> Change the struct dst_entry,add a union contains new pointer from and expires.
> When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
> we can use this field to point to where the dst cache copy from.
> The dst.from is only used in IPV6.
> 
> rt6_check_expired check if rt6_info.dst.from is expired.
> 
> ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
> and RTF_DEFAULT.then hold the ort.
> 
> ip6_dst_destroy release the ort.
> 
> Add some functions to operate the RTF_EXPIRES flag and expires(from) together.
> and change the code to use these new adding functions.
> 
> Changes from v5:
> modify ip6_route_add and ndisc_router_discovery to use new adding functions.
> 
> Only set dst.from when the ort has flag RTF_ADDRCONF
> and RTF_DEFAULT.then hold the ort.
> 
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>

Applied, thanks for doing this work.

^ permalink raw reply

* Re: sky2: in free_buffers() do proper checks before call of rx_clean()
From: David Miller @ 2012-04-13 16:59 UTC (permalink / raw)
  To: LinoSanfilippo; +Cc: shemminger, mlindner, netdev, linux-kernel
In-Reply-To: <20120403001327.GB10227@Neptun>

From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Date: Tue, 3 Apr 2012 02:13:27 +0200

> In sky2_open() free_buffers() is called in case that alloc_buffers() failed.
> Here we do the check for sky2->rx_le too late, since we already called
> rx_clean() which accessed rx_le. Furthermore we lack a check for sky2->rx_ring
> which is also accessed by rx_clean().
> 
> With this patch both sky2->rx_le and sky2->rx_ring are checked before
> alloc_buffers() is called.
> 
> Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>

Stephen, I'm still waiting on your review of this patch.

Thanks.

^ permalink raw reply

* Re: [PATCH 12/37] atl1c: restore max-read-request-size in Device Conrol Register
From: Ben Hutchings @ 2012-04-13 17:02 UTC (permalink / raw)
  To: xiong; +Cc: davem, netdev, linux-kernel, qca-linux-team, nic-devel, linux-pci
In-Reply-To: <1334276102-15866-13-git-send-email-xiong@qca.qualcomm.com>

On Fri, 2012-04-13 at 08:14 +0800, xiong wrote:
> in some platforms, we found the max-read-request-size in Device Control
> Register is set to 0 by (BIOS?) during bootup, this will cause the
> performance(throughput) very bad.
> Restore it to a min-value.
> register definition of REG_DEVICE_CTRL is refined as well.
[...]

I'm guessing this memory-mapped register is an alias for the PCIe device
control register.  I think you should be using pcie_set_readrq() to
ensure that the final setting conforms to the device's Max Payload Size
and system policy.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH] pppoatm: Fix excessive queue bloat
From: David Miller @ 2012-04-13 17:04 UTC (permalink / raw)
  To: chas; +Cc: dwmw2, netdev, paulus, eric.dumazet
In-Reply-To: <20120410102612.368b1a4f@thirdoffive.cmf.nrl.navy.mil>

From: chas williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Date: Tue, 10 Apr 2012 10:26:12 -0400

> On Sun, 08 Apr 2012 21:53:57 +0200
> David Woodhouse <dwmw2@infradead.org> wrote:
> 
>> Seriously, this gets *much* easier if we just ditch the checks against
>> sk_sndbuf. We just wake up whenever decrementing ->inflight from zero.
>> Can I?
> 
> i dont know.

Please do not take this discussion private, and off of the netdev list.

Now nobody else on the list can see your response and and reply to it.

^ permalink raw reply

* Re: ppp: Fix race condition with queue start/stop
From: David Miller @ 2012-04-13 17:07 UTC (permalink / raw)
  To: dwmw2; +Cc: paulus, netdev
In-Reply-To: <1333915304.31812.63.camel@shinybook.infradead.org>

From: David Woodhouse <dwmw2@infradead.org>
Date: Sun, 08 Apr 2012 21:01:44 +0100

> Commit e675f0cc9a872fd152edc0c77acfed19bf28b81e ("ppp: Don't stop and
> restart queue on every TX packet") introduced a race condition which
> could leave the net queue stopped even when the channel is no longer
> busy. By calling netif_stop_queue() from ppp_start_xmit(), based on the
> return value from ppp_xmit_process() but *after* all the locks have been
> dropped, we could potentially do so *after* the channel has actually
> finished transmitting and attempted to re-wake the queue.
> 
> Fix this by moving the netif_stop_queue() into ppp_xmit_process() under
> the xmit lock. I hadn't done this previously, because it gets called
> from other places than ppp_start_xmit(). But I now think it's the better
> option. The net queue *should* be stopped if the channel becomes
> congested due to writes from pppd, anyway.
> 
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>

Applied, thanks David.

^ permalink raw reply

* Re: [PATCH] pppoatm: Fix excessive queue bloat
From: David Miller @ 2012-04-13 17:05 UTC (permalink / raw)
  To: dwmw2; +Cc: netdev, paulus, chas, eric.dumazet
In-Reply-To: <1333914837.31812.57.camel@shinybook.infradead.org>

From: David Woodhouse <dwmw2@infradead.org>
Date: Sun, 08 Apr 2012 21:53:57 +0200

> We discovered that PPPoATM has an excessively deep transmit queue. A
> queue the size of the default socket send buffer (wmem_default) is
> maintained between the PPP generic core and the ATM device.
> 
> Fix it to queue a maximum of *two* packets. The one the ATM device is
> currently working on, and one more for the ATM driver to process
> immediately in its TX done interrupt handler. The PPP core is designed
> to feed packets to the channel with minimal latency, so that really
> ought to be enough to keep the ATM device busy.
> 
> While we're at it, fix the fact that we were triggering the wakeup
> tasklet on *every* pppoatm_pop() call. The comment saying "this is
> inefficient, but doing it right is too hard" turns out to be overly
> pessimistic... I think :)
> 
> On machines like the Traverse Geos, with a slow Geode CPU and two
> high-speed ADSL2+ interfaces, there were reports of extremely high CPU
> usage which could partly be attributed to the extra wakeups.
> 
> (The wakeup handling could actually be made a whole lot easier if we
>  stop checking sk->sk_sndbuf altogether. Given that we now only queue
>  *two* packets ever, one wonders what the point is. As it is, you could
>  already deadlock the thing by setting the sk_sndbuf to a value lower
>  than the MTU of the device, and it'd just block for ever.)
> 
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>

Applied to net-next, thanks David.

I always considered the use of socket buffers to handle packets
moving around the ATM layers as a fundamental design error.

Although it's major surgery, fixing that up would be a much
appreciated contribution for someone suitably motivated :-)

^ permalink raw reply

* Re: [PATCH] skbuff: struct ubuf_info callback type safety
From: David Miller @ 2012-04-13 17:09 UTC (permalink / raw)
  To: mst
  Cc: ian.campbell, eric.dumazet, netdev, linux-kernel, virtualization,
	kvm, shemminger, xma
In-Reply-To: <20120409102402.GA5255@redhat.com>

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Mon, 9 Apr 2012 13:24:02 +0300

> The skb struct ubuf_info callback gets passed struct ubuf_info
> itself, not the arg value as the field name and the function signature
> seem to imply. Rename the arg field to ctx to match usage,
> add documentation and change the callback argument type
> to make usage clear and to have compiler check correctness.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Applied, thanks for fixing this Michael.

^ permalink raw reply


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