[PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

Working around problem causing high CPU load and hanging system when
there is alot of network trafic.

It is kind of an ugly way to work around it, but it allows the Soekris
net5501 to have trafic between two of it's NICs without hanging so much
that the watchdog kicks in and does a hard reboot of the system.

There is more info on the problem here:
http://http://lists.soekris.com/pipermail/soekris-tech/2010-October/016889.html

Tested with positive results on two Soekris net5501-70 boxes.

Signed-off-by: Bjarke Istrup Pedersen <gurligebis@gentoo.org>
---
 drivers/net/ethernet/via/via-rhine.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
index f34dd99..8c77dcd 100644
--- a/drivers/net/ethernet/via/via-rhine.c
+++ b/drivers/net/ethernet/via/via-rhine.c
@@ -1567,6 +1567,9 @@ static irqreturn_t rhine_interrupt(int irq, void *dev_instance)
 	int boguscnt = max_interrupt_work;
 	int handled = 0;
 
+	if (!spin_trylock(&rp->lock))
+		return IRQ_RETVAL(handled);
+
 	while ((intr_status = get_intr_status(dev))) {
 		handled = 1;
 
@@ -1616,6 +1619,8 @@ static irqreturn_t rhine_interrupt(int irq, void *dev_instance)
 		}
 	}
 
+	spin_unlock(&rp->lock);
+
 	if (debug > 3)
 		netdev_dbg(dev, "exiting interrupt, status=%08x\n",
 			   ioread16(ioaddr + IntrStatus));
-- 
1.7.8

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Ben Hutchings]

On Wed, 2011-12-28 at 12:28 +0000, Bjarke Istrup Pedersen wrote:
> Working around problem causing high CPU load and hanging system when
> there is alot of network trafic.
> 
> It is kind of an ugly way to work around it, but it allows the Soekris
> net5501 to have trafic between two of it's NICs without hanging so much
> that the watchdog kicks in and does a hard reboot of the system.
> 
> There is more info on the problem here:
> http://http://lists.soekris.com/pipermail/soekris-tech/2010-October/016889.html
> 
> Tested with positive results on two Soekris net5501-70 boxes.

This is completely wrong.  In a UP configuration the extra spinlock
calls have no effect (except perhaps a small delay).  In an SMP
configuration they will cause rhine_tx() to deadlock when it also tries
to acquire the spinlock.

Ben.

[...]
> diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
> index f34dd99..8c77dcd 100644
> --- a/drivers/net/ethernet/via/via-rhine.c
> +++ b/drivers/net/ethernet/via/via-rhine.c
> @@ -1567,6 +1567,9 @@ static irqreturn_t rhine_interrupt(int irq, void *dev_instance)
>         int boguscnt = max_interrupt_work;
>         int handled = 0;
>  
> +       if (!spin_trylock(&rp->lock))
> +               return IRQ_RETVAL(handled);
> +
>         while ((intr_status = get_intr_status(dev))) {
>                 handled = 1;
>  
> @@ -1616,6 +1619,8 @@ static irqreturn_t rhine_interrupt(int irq, void *dev_instance)
>                 }
>         }
>  
> +       spin_unlock(&rp->lock);
> +
>         if (debug > 3)
>                 netdev_dbg(dev, "exiting interrupt, status=%08x\n",
>                            ioread16(ioaddr + IntrStatus));

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

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/28 Ben Hutchings <bhutchings@solarflare.com>:
> On Wed, 2011-12-28 at 12:28 +0000, Bjarke Istrup Pedersen wrote:
>> Working around problem causing high CPU load and hanging system when
>> there is alot of network trafic.
>>
>> It is kind of an ugly way to work around it, but it allows the Soekris
>> net5501 to have trafic between two of it's NICs without hanging so much
>> that the watchdog kicks in and does a hard reboot of the system.
>>
>> There is more info on the problem here:
>> http://http://lists.soekris.com/pipermail/soekris-tech/2010-October/016889.html
>>
>> Tested with positive results on two Soekris net5501-70 boxes.
>
> This is completely wrong.  In a UP configuration the extra spinlock
> calls have no effect (except perhaps a small delay).  In an SMP
> configuration they will cause rhine_tx() to deadlock when it also tries
> to acquire the spinlock.
>
> Ben.

Okay, the Soekris net5501-70 boxes are single-core, and I haven't got
any SMP boxes with that nic.
Is there a better solution for the problem then, to avoid it hanging
the box on a non-smp machine with a slow (500mhz) cpu?

/Bjarke

> [...]
>> diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
>> index f34dd99..8c77dcd 100644
>> --- a/drivers/net/ethernet/via/via-rhine.c
>> +++ b/drivers/net/ethernet/via/via-rhine.c
>> @@ -1567,6 +1567,9 @@ static irqreturn_t rhine_interrupt(int irq, void *dev_instance)
>>         int boguscnt = max_interrupt_work;
>>         int handled = 0;
>>
>> +       if (!spin_trylock(&rp->lock))
>> +               return IRQ_RETVAL(handled);
>> +
>>         while ((intr_status = get_intr_status(dev))) {
>>                 handled = 1;
>>
>> @@ -1616,6 +1619,8 @@ static irqreturn_t rhine_interrupt(int irq, void *dev_instance)
>>                 }
>>         }
>>
>> +       spin_unlock(&rp->lock);
>> +
>>         if (debug > 3)
>>                 netdev_dbg(dev, "exiting interrupt, status=%08x\n",
>>                            ioread16(ioaddr + IntrStatus));
>
> --
> 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.
>

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Ben Hutchings]

On Wed, 2011-12-28 at 16:14 +0100, Bjarke Istrup Pedersen wrote:
> 2011/12/28 Ben Hutchings <bhutchings@solarflare.com>:
> > On Wed, 2011-12-28 at 12:28 +0000, Bjarke Istrup Pedersen wrote:
> >> Working around problem causing high CPU load and hanging system when
> >> there is alot of network trafic.
> >>
> >> It is kind of an ugly way to work around it, but it allows the Soekris
> >> net5501 to have trafic between two of it's NICs without hanging so much
> >> that the watchdog kicks in and does a hard reboot of the system.
> >>
> >> There is more info on the problem here:
> >> http://http://lists.soekris.com/pipermail/soekris-tech/2010-October/016889.html
> >>
> >> Tested with positive results on two Soekris net5501-70 boxes.
> >
> > This is completely wrong.  In a UP configuration the extra spinlock
> > calls have no effect (except perhaps a small delay).  In an SMP
> > configuration they will cause rhine_tx() to deadlock when it also tries
> > to acquire the spinlock.
> >
> > Ben.
> 
> Okay, the Soekris net5501-70 boxes are single-core, and I haven't got
> any SMP boxes with that nic.
> Is there a better solution for the problem then, to avoid it hanging
> the box on a non-smp machine with a slow (500mhz) cpu?

If the system actually hangs then I assume there is some bug in the
driver.  I would guess the actual problem is that the interrupt and NAPI
handlers are running constantly so that user processes never run (which
I think counts as soft lockup).

If the hardware supports it, interrupt moderation may help a little by
slightly reducing the per-packet processing cost, but it isn't a full
solution.  Or you can use a real-time kernel, which schedules interrupt
and NAPI handlers as tasks, and adjust priorities so that user processes
can still run.  But that brings its own problems (including generally
lower throughput).

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.

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/28 Ben Hutchings <bhutchings@solarflare.com>:
> On Wed, 2011-12-28 at 16:14 +0100, Bjarke Istrup Pedersen wrote:
>> 2011/12/28 Ben Hutchings <bhutchings@solarflare.com>:
>> > On Wed, 2011-12-28 at 12:28 +0000, Bjarke Istrup Pedersen wrote:
>> >> Working around problem causing high CPU load and hanging system when
>> >> there is alot of network trafic.
>> >>
>> >> It is kind of an ugly way to work around it, but it allows the Soekris
>> >> net5501 to have trafic between two of it's NICs without hanging so much
>> >> that the watchdog kicks in and does a hard reboot of the system.
>> >>
>> >> There is more info on the problem here:
>> >> http://http://lists.soekris.com/pipermail/soekris-tech/2010-October/016889.html
>> >>
>> >> Tested with positive results on two Soekris net5501-70 boxes.
>> >
>> > This is completely wrong.  In a UP configuration the extra spinlock
>> > calls have no effect (except perhaps a small delay).  In an SMP
>> > configuration they will cause rhine_tx() to deadlock when it also tries
>> > to acquire the spinlock.
>> >
>> > Ben.
>>
>> Okay, the Soekris net5501-70 boxes are single-core, and I haven't got
>> any SMP boxes with that nic.
>> Is there a better solution for the problem then, to avoid it hanging
>> the box on a non-smp machine with a slow (500mhz) cpu?
>
> If the system actually hangs then I assume there is some bug in the
> driver.  I would guess the actual problem is that the interrupt and NAPI
> handlers are running constantly so that user processes never run (which
> I think counts as soft lockup).
>
> If the hardware supports it, interrupt moderation may help a little by
> slightly reducing the per-packet processing cost, but it isn't a full
> solution.  Or you can use a real-time kernel, which schedules interrupt
> and NAPI handlers as tasks, and adjust priorities so that user processes
> can still run.  But that brings its own problems (including generally
> lower throughput).
>
> Ben.

That would be an option, but I don't think the hardware supports it,
and it doesn't fix the problems in the driver, just hides them.
From what I can read in the thread I linked to in the patch, the
problem only exists in the Linux driver - *BSD isn't affected by this,
on the same hardware.

Since the hack here fixes the problem on non-smp machines, it seems
like there are some race conditions in the interrupt code in the
driver, like you mention.
I'm not well enough into this driver to be able to pinpoint what's
causing it, but if somebody else is, and got some ideas, I'll be more
than willing to test :)

/Bjarke

> --
> 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.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Stephen Hemminger]

Looks like the hardware isn't really disabling interrupts correctly to support
NAPI. NAPI is supposed to be friendly and under load the work should move to
ksoftirqd. I suspect the IRQ management in this driver is borked.
There is some stupid spin loops in the IRQ handler that happen when looking for
Tx IRQ. I would run with debug set  and see if this is triggering.

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 28 Dec 2011 10:17:10 -0800

> Looks like the hardware isn't really disabling interrupts correctly to support
> NAPI. NAPI is supposed to be friendly and under load the work should move to
> ksoftirqd. I suspect the IRQ management in this driver is borked.
> There is some stupid spin loops in the IRQ handler that happen when looking for
> Tx IRQ. I would run with debug set  and see if this is triggering.

This could be simply a side effect of the fact that via-rhine only does
RX work in it's NAPI handler.

If all the work (or at least both TX and RX) were moved into the NAPI
handler, it'd probably be easier to properly shut off all IRQs during
NAPI processing, and thus avoid the high CPU load in the IRQ handler
being seen here.

This "shut off only some interrupts" scheme is just asking for trouble.

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/28 David Miller <davem@davemloft.net>:
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Wed, 28 Dec 2011 10:17:10 -0800
>
>> Looks like the hardware isn't really disabling interrupts correctly to support
>> NAPI. NAPI is supposed to be friendly and under load the work should move to
>> ksoftirqd. I suspect the IRQ management in this driver is borked.
>> There is some stupid spin loops in the IRQ handler that happen when looking for
>> Tx IRQ. I would run with debug set  and see if this is triggering.
>
> This could be simply a side effect of the fact that via-rhine only does
> RX work in it's NAPI handler.
>
> If all the work (or at least both TX and RX) were moved into the NAPI
> handler, it'd probably be easier to properly shut off all IRQs during
> NAPI processing, and thus avoid the high CPU load in the IRQ handler
> being seen here.
>
> This "shut off only some interrupts" scheme is just asking for trouble.

I agree, it's an ugly hack :)

Is there some way I can help with this, without having to touch the
code? (C isn't my strong language, especially not kernel mode code) :)

/Bjarke

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Francois Romieu]

Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> 2011/12/28 David Miller <davem@davemloft.net>:
[...]
> > This "shut off only some interrupts" scheme is just asking for trouble.
> 
> I agree, it's an ugly hack :)
> 
> Is there some way I can help with this, without having to touch the
> code? (C isn't my strong language, especially not kernel mode code) :)

Feel free to try the extra "new year's eve week end is getting closer"
patch below (against net-next).

You have been warned.

diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
index 5c4983b..6350a2c 100644
--- a/drivers/net/ethernet/via/via-rhine.c
+++ b/drivers/net/ethernet/via/via-rhine.c
@@ -42,7 +42,8 @@
 
 #define DEBUG
 static int debug = 1;	/* 1 normal messages, 0 quiet .. 7 verbose. */
-static int max_interrupt_work = 20;
+#define RHINE_MSG_DEFAULT \
+        (NETIF_MSG_INTR)
 
 /* Set the copy breakpoint for the copy-only-tiny-frames scheme.
    Setting to > 1518 effectively disables this feature. */
@@ -128,11 +129,9 @@ MODULE_AUTHOR("Donald Becker <becker@scyld.com>");
 MODULE_DESCRIPTION("VIA Rhine PCI Fast Ethernet driver");
 MODULE_LICENSE("GPL");
 
-module_param(max_interrupt_work, int, 0);
 module_param(debug, int, 0);
 module_param(rx_copybreak, int, 0);
 module_param(avoid_D3, bool, 0);
-MODULE_PARM_DESC(max_interrupt_work, "VIA Rhine maximum events handled per interrupt");
 MODULE_PARM_DESC(debug, "VIA Rhine debug level (0-7)");
 MODULE_PARM_DESC(rx_copybreak, "VIA Rhine copy breakpoint for copy-only-tiny-frames");
 MODULE_PARM_DESC(avoid_D3, "Avoid power state D3 (work-around for broken BIOSes)");
@@ -351,16 +350,25 @@ static const int mmio_verify_registers[] = {
 
 /* Bits in the interrupt status/mask registers. */
 enum intr_status_bits {
-	IntrRxDone=0x0001, IntrRxErr=0x0004, IntrRxEmpty=0x0020,
-	IntrTxDone=0x0002, IntrTxError=0x0008, IntrTxUnderrun=0x0210,
-	IntrPCIErr=0x0040,
-	IntrStatsMax=0x0080, IntrRxEarly=0x0100,
-	IntrRxOverflow=0x0400, IntrRxDropped=0x0800, IntrRxNoBuf=0x1000,
-	IntrTxAborted=0x2000, IntrLinkChange=0x4000,
-	IntrRxWakeUp=0x8000,
-	IntrNormalSummary=0x0003, IntrAbnormalSummary=0xC260,
-	IntrTxDescRace=0x080000,	/* mapped from IntrStatus2 */
-	IntrTxErrSummary=0x082218,
+	IntrRxDone	= 0x0001,
+	IntrTxDone	= 0x0002,
+	IntrRxErr	= 0x0004,
+	IntrTxError	= 0x0008,
+	IntrRxEmpty	= 0x0020,
+	IntrPCIErr	= 0x0040,
+	IntrStatsMax	= 0x0080,
+	IntrRxEarly	= 0x0100,
+	IntrTxUnderrun	= 0x0210,
+	IntrRxOverflow	= 0x0400,
+	IntrRxDropped	= 0x0800,
+	IntrRxNoBuf	= 0x1000,
+	IntrTxAborted	= 0x2000,
+	IntrLinkChange	= 0x4000,
+	IntrRxWakeUp	= 0x8000,
+	IntrTxDescRace		= 0x080000,	/* mapped from IntrStatus2 */
+	IntrNormalSummary	= IntrRxDone | IntrTxDone,
+	IntrTxErrSummary	= IntrTxDescRace | IntrTxAborted | IntrTxError |
+				  IntrTxUnderrun,
 };
 
 /* Bits in WOLcrSet/WOLcrClr and PwrcsrSet/PwrcsrClr */
@@ -439,8 +447,12 @@ struct rhine_private {
 	struct net_device *dev;
 	struct napi_struct napi;
 	spinlock_t lock;
+	struct mutex task_lock;
+	struct work_struct slow_event_task;
 	struct work_struct reset_task;
 
+	u32 msg_enable;
+
 	/* Frequently used values: keep some adjacent for cache effect. */
 	u32 quirks;
 	struct rx_desc *rx_head_desc;
@@ -482,7 +494,6 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
 static irqreturn_t rhine_interrupt(int irq, void *dev_instance);
 static void rhine_tx(struct net_device *dev);
 static int rhine_rx(struct net_device *dev, int limit);
-static void rhine_error(struct net_device *dev, int intr_status);
 static void rhine_set_rx_mode(struct net_device *dev);
 static struct net_device_stats *rhine_get_stats(struct net_device *dev);
 static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
@@ -497,6 +508,8 @@ static void rhine_set_cam_mask(void __iomem *ioaddr, u32 mask);
 static void rhine_set_vlan_cam_mask(void __iomem *ioaddr, u32 mask);
 static void rhine_init_cam_filter(struct net_device *dev);
 static void rhine_update_vcam(struct net_device *dev);
+static void rhine_slow_event_task(struct work_struct *work);
+static void rhine_restart_tx(struct net_device *dev);
 
 #define RHINE_WAIT_FOR(condition)				\
 do {								\
@@ -508,7 +521,7 @@ do {								\
 			1024 - i, __func__, __LINE__);		\
 } while (0)
 
-static inline u32 get_intr_status(struct net_device *dev)
+static u32 get_intr_status(struct net_device *dev)
 {
 	struct rhine_private *rp = netdev_priv(dev);
 	void __iomem *ioaddr = rp->base;
@@ -521,6 +534,16 @@ static inline u32 get_intr_status(struct net_device *dev)
 	return intr_status;
 }
 
+static void rhine_ack_event(struct rhine_private *rp, u32 mask)
+{
+	void __iomem *ioaddr = rp->base;
+
+	if (rp->quirks & rqStatusWBRace)
+		iowrite8(mask >> 16, ioaddr + IntrStatus2);
+	iowrite16(mask, ioaddr + IntrStatus);
+	mmiowb();
+}
+
 /*
  * Get power related registers into sane state.
  * Notify user about past WOL event.
@@ -657,23 +680,62 @@ static void rhine_poll(struct net_device *dev)
 }
 #endif
 
+#define RHINE_EVENT_NAPI_RX	(IntrRxDone | \
+				 IntrRxErr | \
+				 IntrRxEmpty | \
+				 IntrRxOverflow	| \
+				 IntrRxDropped | \
+				 IntrRxNoBuf | \
+				 IntrRxWakeUp)
+
+#define RHINE_EVENT_NAPI_TX_ERR	(IntrTxError | \
+				 IntrTxAborted | \
+				 IntrTxUnderrun | \
+				 IntrTxDescRace)
+#define RHINE_EVENT_NAPI_TX	(IntrTxDone | RHINE_EVENT_NAPI_TX_ERR)
+
+#define RHINE_EVENT_NAPI	(RHINE_EVENT_NAPI_RX | RHINE_EVENT_NAPI_TX)
+#define RHINE_EVENT_SLOW	(IntrPCIErr | IntrStatsMax | IntrLinkChange)
+#define RHINE_EVENT		(RHINE_EVENT_NAPI | RHINE_EVENT_SLOW)
+
 static int rhine_napipoll(struct napi_struct *napi, int budget)
 {
 	struct rhine_private *rp = container_of(napi, struct rhine_private, napi);
 	struct net_device *dev = rp->dev;
 	void __iomem *ioaddr = rp->base;
-	int work_done;
+	u16 enable_mask = RHINE_EVENT & 0xffff;
+	int work_done = 0;
+	u32 status;
+
+	status = get_intr_status(dev);
+	rhine_ack_event(rp, status & ~RHINE_EVENT_SLOW);
+
+	if (status & RHINE_EVENT_NAPI_RX)
+		work_done += rhine_rx(dev, budget);
+
+	if (status & RHINE_EVENT_NAPI_TX) {
+		if (status & RHINE_EVENT_NAPI_TX_ERR) {
+			/* Avoid scavenging before Tx engine turned off */
+			RHINE_WAIT_FOR(!(ioread8(ioaddr + ChipCmd) & CmdTxOn));
+			if (ioread8(ioaddr + ChipCmd) & CmdTxOn)
+				netif_warn(rp, tx_err, dev, "Tx still on\n");
+		}
+
+		rhine_tx(dev);
+
+		if (status & RHINE_EVENT_NAPI_TX_ERR)
+			rhine_restart_tx(dev);
+	}
 
-	work_done = rhine_rx(dev, budget);
+	if (status & RHINE_EVENT_SLOW) {
+		enable_mask &= ~RHINE_EVENT_SLOW;
+		schedule_work(&rp->slow_event_task);
+	}
 
 	if (work_done < budget) {
 		napi_complete(napi);
-
-		iowrite16(IntrRxDone | IntrRxErr | IntrRxEmpty| IntrRxOverflow |
-			  IntrRxDropped | IntrRxNoBuf | IntrTxAborted |
-			  IntrTxDone | IntrTxError | IntrTxUnderrun |
-			  IntrPCIErr | IntrStatsMax | IntrLinkChange,
-			  ioaddr + IntrEnable);
+		iowrite16(enable_mask, ioaddr + IntrEnable);
+		mmiowb();
 	}
 	return work_done;
 }
@@ -797,6 +859,7 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
 	rp->quirks = quirks;
 	rp->pioaddr = pioaddr;
 	rp->pdev = pdev;
+	rp->msg_enable = netif_msg_init(-1, RHINE_MSG_DEFAULT);
 
 	rc = pci_request_regions(pdev, DRV_NAME);
 	if (rc)
@@ -856,7 +919,9 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
 	dev->irq = pdev->irq;
 
 	spin_lock_init(&rp->lock);
+	mutex_init(&rp->task_lock);
 	INIT_WORK(&rp->reset_task, rhine_reset_task);
+	INIT_WORK(&rp->slow_event_task, rhine_slow_event_task);
 
 	rp->mii_if.dev = dev;
 	rp->mii_if.mdio_read = mdio_read;
@@ -1310,12 +1375,7 @@ static void init_registers(struct net_device *dev)
 
 	napi_enable(&rp->napi);
 
-	/* Enable interrupts by setting the interrupt mask. */
-	iowrite16(IntrRxDone | IntrRxErr | IntrRxEmpty| IntrRxOverflow |
-	       IntrRxDropped | IntrRxNoBuf | IntrTxAborted |
-	       IntrTxDone | IntrTxError | IntrTxUnderrun |
-	       IntrPCIErr | IntrStatsMax | IntrLinkChange,
-	       ioaddr + IntrEnable);
+	iowrite16(RHINE_EVENT & 0xffff, ioaddr + IntrEnable);
 
 	iowrite16(CmdStart | CmdTxOn | CmdRxOn | (Cmd1NoTxPoll << 8),
 	       ioaddr + ChipCmd);
@@ -1434,8 +1494,7 @@ static void rhine_reset_task(struct work_struct *work)
 						reset_task);
 	struct net_device *dev = rp->dev;
 
-	/* protect against concurrent rx interrupts */
-	disable_irq(rp->pdev->irq);
+	mutex_lock(&rp->task_lock);
 
 	napi_disable(&rp->napi);
 
@@ -1452,7 +1511,8 @@ static void rhine_reset_task(struct work_struct *work)
 	init_registers(dev);
 
 	spin_unlock_bh(&rp->lock);
-	enable_irq(rp->pdev->irq);
+
+	mutex_unlock(&rp->task_lock);
 
 	dev->trans_start = jiffies; /* prevent tx timeout */
 	dev->stats.tx_errors++;
@@ -1565,63 +1625,26 @@ static irqreturn_t rhine_interrupt(int irq, void *dev_instance)
 {
 	struct net_device *dev = dev_instance;
 	struct rhine_private *rp = netdev_priv(dev);
-	void __iomem *ioaddr = rp->base;
-	u32 intr_status;
-	int boguscnt = max_interrupt_work;
+	u32 status;
 	int handled = 0;
 
-	while ((intr_status = get_intr_status(dev))) {
-		handled = 1;
+	status = get_intr_status(dev);
 
-		/* Acknowledge all of the current interrupt sources ASAP. */
-		if (intr_status & IntrTxDescRace)
-			iowrite8(0x08, ioaddr + IntrStatus2);
-		iowrite16(intr_status & 0xffff, ioaddr + IntrStatus);
-		IOSYNC;
-
-		if (debug > 4)
-			netdev_dbg(dev, "Interrupt, status %08x\n",
-				   intr_status);
-
-		if (intr_status & (IntrRxDone | IntrRxErr | IntrRxDropped |
-				   IntrRxWakeUp | IntrRxEmpty | IntrRxNoBuf)) {
-			iowrite16(IntrTxAborted |
-				  IntrTxDone | IntrTxError | IntrTxUnderrun |
-				  IntrPCIErr | IntrStatsMax | IntrLinkChange,
-				  ioaddr + IntrEnable);
-
-			napi_schedule(&rp->napi);
-		}
+	netif_dbg(rp, intr, dev, "Interrupt, status %08x\n", status);
 
-		if (intr_status & (IntrTxErrSummary | IntrTxDone)) {
-			if (intr_status & IntrTxErrSummary) {
-				/* Avoid scavenging before Tx engine turned off */
-				RHINE_WAIT_FOR(!(ioread8(ioaddr+ChipCmd) & CmdTxOn));
-				if (debug > 2 &&
-				    ioread8(ioaddr+ChipCmd) & CmdTxOn)
-					netdev_warn(dev,
-						    "%s: Tx engine still on\n",
-						    __func__);
-			}
-			rhine_tx(dev);
-		}
+	if (status & RHINE_EVENT) {
+		handled = 1;
 
-		/* Abnormal error summary/uncommon events handlers. */
-		if (intr_status & (IntrPCIErr | IntrLinkChange |
-				   IntrStatsMax | IntrTxError | IntrTxAborted |
-				   IntrTxUnderrun | IntrTxDescRace))
-			rhine_error(dev, intr_status);
+		iowrite16(0x0000, rp->base + IntrEnable);
+		mmiowb();
+		napi_schedule(&rp->napi);
+	}
 
-		if (--boguscnt < 0) {
-			netdev_warn(dev, "Too much work at interrupt, status=%#08x\n",
-				    intr_status);
-			break;
-		}
+	if (status & ~(IntrLinkChange | IntrStatsMax | RHINE_EVENT_NAPI)) {
+		netif_err(rp, intr, dev, "Something Wicked happened! %08x\n",
+			  status);
 	}
 
-	if (debug > 3)
-		netdev_dbg(dev, "exiting interrupt, status=%08x\n",
-			   ioread16(ioaddr + IntrStatus));
 	return IRQ_RETVAL(handled);
 }
 
@@ -1632,8 +1655,6 @@ static void rhine_tx(struct net_device *dev)
 	struct rhine_private *rp = netdev_priv(dev);
 	int txstatus = 0, entry = rp->dirty_tx % TX_RING_SIZE;
 
-	spin_lock(&rp->lock);
-
 	/* find and cleanup dirty tx descriptors */
 	while (rp->dirty_tx != rp->cur_tx) {
 		txstatus = le32_to_cpu(rp->tx_ring[entry].tx_status);
@@ -1687,8 +1708,6 @@ static void rhine_tx(struct net_device *dev)
 	}
 	if ((rp->cur_tx - rp->dirty_tx) < TX_QUEUE_LEN - 4)
 		netif_wake_queue(dev);
-
-	spin_unlock(&rp->lock);
 }
 
 /**
@@ -1852,7 +1871,8 @@ static inline void clear_tally_counters(void __iomem *ioaddr)
 	ioread16(ioaddr + RxMissed);
 }
 
-static void rhine_restart_tx(struct net_device *dev) {
+static void rhine_restart_tx(struct net_device *dev)
+{
 	struct rhine_private *rp = netdev_priv(dev);
 	void __iomem *ioaddr = rp->base;
 	int entry = rp->dirty_tx % TX_RING_SIZE;
@@ -1880,8 +1900,7 @@ static void rhine_restart_tx(struct net_device *dev) {
 		iowrite8(ioread8(ioaddr + ChipCmd1) | Cmd1TxDemand,
 		       ioaddr + ChipCmd1);
 		IOSYNC;
-	}
-	else {
+	} else {
 		/* This should never happen */
 		if (debug > 1)
 			netdev_warn(dev, "%s() Another error occurred %08x\n",
@@ -1890,15 +1909,25 @@ static void rhine_restart_tx(struct net_device *dev) {
 
 }
 
-static void rhine_error(struct net_device *dev, int intr_status)
+static void rhine_slow_event_task(struct work_struct *work)
 {
-	struct rhine_private *rp = netdev_priv(dev);
+	struct rhine_private *rp =
+		container_of(work, struct rhine_private, slow_event_task);
+	struct net_device *dev = rp->dev;
 	void __iomem *ioaddr = rp->base;
+	u32 intr_status;
 
-	spin_lock(&rp->lock);
+	if (!netif_running(dev))
+		return;
+
+	mutex_lock(&rp->task_lock);
+
+	intr_status = get_intr_status(dev);
+	rhine_ack_event(rp, intr_status & RHINE_EVENT_SLOW);
 
 	if (intr_status & IntrLinkChange)
 		rhine_check_media(dev, 0);
+
 	if (intr_status & IntrStatsMax) {
 		dev->stats.rx_crc_errors += ioread16(ioaddr + RxCRCErrs);
 		dev->stats.rx_missed_errors += ioread16(ioaddr + RxMissed);
@@ -1930,19 +1959,15 @@ static void rhine_error(struct net_device *dev, int intr_status)
 			netdev_info(dev, "Unspecified error. Tx threshold now %02x\n",
 				    rp->tx_thresh);
 	}
-	if (intr_status & (IntrTxAborted | IntrTxUnderrun | IntrTxDescRace |
-			   IntrTxError))
-		rhine_restart_tx(dev);
 
-	if (intr_status & ~(IntrLinkChange | IntrStatsMax | IntrTxUnderrun |
-			    IntrTxError | IntrTxAborted | IntrNormalSummary |
-			    IntrTxDescRace)) {
-		if (debug > 1)
-			netdev_err(dev, "Something Wicked happened! %08x\n",
-				   intr_status);
-	}
+	napi_disable(&rp->napi);
+	iowrite16(0x0000, ioaddr + IntrEnable);
+	mmiowb();
+	/* Slow and safe. Consider __napi_schedule as a replacement ? */
+	napi_enable(&rp->napi);
+	napi_schedule(&rp->napi);
 
-	spin_unlock(&rp->lock);
+	mutex_unlock(&rp->task_lock);
 }
 
 static struct net_device_stats *rhine_get_stats(struct net_device *dev)
@@ -2133,6 +2158,7 @@ static int rhine_close(struct net_device *dev)
 	void __iomem *ioaddr = rp->base;
 
 	napi_disable(&rp->napi);
+	cancel_work_sync(&rp->slow_event_task);
 	cancel_work_sync(&rp->reset_task);
 	netif_stop_queue(dev);
 

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Francois Romieu]

Francois Romieu <romieu@fr.zoreil.com> :
[...]
> Feel free to try the extra "new year's eve week end is getting closer"
> patch below (against net-next).
> 
> You have been warned.

The one below could be a bit better.

Out for dinner.

diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
index 5c4983b..66e5c08 100644
--- a/drivers/net/ethernet/via/via-rhine.c
+++ b/drivers/net/ethernet/via/via-rhine.c
@@ -42,7 +42,8 @@
 
 #define DEBUG
 static int debug = 1;	/* 1 normal messages, 0 quiet .. 7 verbose. */
-static int max_interrupt_work = 20;
+#define RHINE_MSG_DEFAULT \
+        (NETIF_MSG_INTR)
 
 /* Set the copy breakpoint for the copy-only-tiny-frames scheme.
    Setting to > 1518 effectively disables this feature. */
@@ -128,11 +129,9 @@ MODULE_AUTHOR("Donald Becker <becker@scyld.com>");
 MODULE_DESCRIPTION("VIA Rhine PCI Fast Ethernet driver");
 MODULE_LICENSE("GPL");
 
-module_param(max_interrupt_work, int, 0);
 module_param(debug, int, 0);
 module_param(rx_copybreak, int, 0);
 module_param(avoid_D3, bool, 0);
-MODULE_PARM_DESC(max_interrupt_work, "VIA Rhine maximum events handled per interrupt");
 MODULE_PARM_DESC(debug, "VIA Rhine debug level (0-7)");
 MODULE_PARM_DESC(rx_copybreak, "VIA Rhine copy breakpoint for copy-only-tiny-frames");
 MODULE_PARM_DESC(avoid_D3, "Avoid power state D3 (work-around for broken BIOSes)");
@@ -351,16 +350,25 @@ static const int mmio_verify_registers[] = {
 
 /* Bits in the interrupt status/mask registers. */
 enum intr_status_bits {
-	IntrRxDone=0x0001, IntrRxErr=0x0004, IntrRxEmpty=0x0020,
-	IntrTxDone=0x0002, IntrTxError=0x0008, IntrTxUnderrun=0x0210,
-	IntrPCIErr=0x0040,
-	IntrStatsMax=0x0080, IntrRxEarly=0x0100,
-	IntrRxOverflow=0x0400, IntrRxDropped=0x0800, IntrRxNoBuf=0x1000,
-	IntrTxAborted=0x2000, IntrLinkChange=0x4000,
-	IntrRxWakeUp=0x8000,
-	IntrNormalSummary=0x0003, IntrAbnormalSummary=0xC260,
-	IntrTxDescRace=0x080000,	/* mapped from IntrStatus2 */
-	IntrTxErrSummary=0x082218,
+	IntrRxDone	= 0x0001,
+	IntrTxDone	= 0x0002,
+	IntrRxErr	= 0x0004,
+	IntrTxError	= 0x0008,
+	IntrRxEmpty	= 0x0020,
+	IntrPCIErr	= 0x0040,
+	IntrStatsMax	= 0x0080,
+	IntrRxEarly	= 0x0100,
+	IntrTxUnderrun	= 0x0210,
+	IntrRxOverflow	= 0x0400,
+	IntrRxDropped	= 0x0800,
+	IntrRxNoBuf	= 0x1000,
+	IntrTxAborted	= 0x2000,
+	IntrLinkChange	= 0x4000,
+	IntrRxWakeUp	= 0x8000,
+	IntrTxDescRace		= 0x080000,	/* mapped from IntrStatus2 */
+	IntrNormalSummary	= IntrRxDone | IntrTxDone,
+	IntrTxErrSummary	= IntrTxDescRace | IntrTxAborted | IntrTxError |
+				  IntrTxUnderrun,
 };
 
 /* Bits in WOLcrSet/WOLcrClr and PwrcsrSet/PwrcsrClr */
@@ -439,8 +447,12 @@ struct rhine_private {
 	struct net_device *dev;
 	struct napi_struct napi;
 	spinlock_t lock;
+	struct mutex task_lock;
+	struct work_struct slow_event_task;
 	struct work_struct reset_task;
 
+	u32 msg_enable;
+
 	/* Frequently used values: keep some adjacent for cache effect. */
 	u32 quirks;
 	struct rx_desc *rx_head_desc;
@@ -482,7 +494,6 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
 static irqreturn_t rhine_interrupt(int irq, void *dev_instance);
 static void rhine_tx(struct net_device *dev);
 static int rhine_rx(struct net_device *dev, int limit);
-static void rhine_error(struct net_device *dev, int intr_status);
 static void rhine_set_rx_mode(struct net_device *dev);
 static struct net_device_stats *rhine_get_stats(struct net_device *dev);
 static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
@@ -497,6 +508,8 @@ static void rhine_set_cam_mask(void __iomem *ioaddr, u32 mask);
 static void rhine_set_vlan_cam_mask(void __iomem *ioaddr, u32 mask);
 static void rhine_init_cam_filter(struct net_device *dev);
 static void rhine_update_vcam(struct net_device *dev);
+static void rhine_slow_event_task(struct work_struct *work);
+static void rhine_restart_tx(struct net_device *dev);
 
 #define RHINE_WAIT_FOR(condition)				\
 do {								\
@@ -508,7 +521,7 @@ do {								\
 			1024 - i, __func__, __LINE__);		\
 } while (0)
 
-static inline u32 get_intr_status(struct net_device *dev)
+static u32 get_intr_status(struct net_device *dev)
 {
 	struct rhine_private *rp = netdev_priv(dev);
 	void __iomem *ioaddr = rp->base;
@@ -521,6 +534,16 @@ static inline u32 get_intr_status(struct net_device *dev)
 	return intr_status;
 }
 
+static void rhine_ack_event(struct rhine_private *rp, u32 mask)
+{
+	void __iomem *ioaddr = rp->base;
+
+	if (rp->quirks & rqStatusWBRace)
+		iowrite8(mask >> 16, ioaddr + IntrStatus2);
+	iowrite16(mask, ioaddr + IntrStatus);
+	mmiowb();
+}
+
 /*
  * Get power related registers into sane state.
  * Notify user about past WOL event.
@@ -657,23 +680,127 @@ static void rhine_poll(struct net_device *dev)
 }
 #endif
 
+static void rhine_kick_tx_threshold(struct rhine_private *rp)
+{
+	if (rp->tx_thresh < 0xe0) {
+		void __iomem *ioaddr = rp->base;
+
+		rp->tx_thresh += 0x20;
+		BYTE_REG_BITS_SET(rp->tx_thresh, 0x80, ioaddr + TxConfig);
+	}
+}
+
+static void rhine_tx_err(struct rhine_private *rp, u32 status)
+{
+	struct net_device *dev = rp->dev;
+
+	if (status & IntrTxAborted) {
+		netif_info(rp, tx_err, dev,
+			   "Abort %08x, frame dropped\n", status);
+	}
+
+	if (status & IntrTxUnderrun) {
+		rhine_kick_tx_threshold(rp);
+		netif_info(rp, tx_err ,dev, "Transmitter underrun, "
+			   "Tx threshold now %02x\n", rp->tx_thresh);
+	}
+
+	if (status & IntrTxDescRace)
+		netif_info(rp, tx_err, dev, "Tx descriptor write-back race\n");
+
+	if ((status & IntrTxError) &&
+	    (status & (IntrTxAborted | IntrTxUnderrun | IntrTxDescRace)) == 0) {
+		rhine_kick_tx_threshold(rp);
+		netif_info(rp, tx_err, dev, "Unspecified error. "
+			   "Tx threshold now %02x\n", rp->tx_thresh);
+	}
+
+	rhine_restart_tx(dev);
+}
+
+static void rhine_update_rx_crc_and_missed_errord(struct rhine_private *rp)
+{
+	void __iomem *ioaddr = rp->base;
+	struct net_device_stats *stats = &rp->dev->stats;
+
+	stats->rx_crc_errors    += ioread16(ioaddr + RxCRCErrs);
+	stats->rx_missed_errors += ioread16(ioaddr + RxMissed);
+
+	/*
+	 * Clears the "tally counters" for CRC errors and missed frames(?).
+	 * It has been reported that some chips need a write of 0 to clear
+	 * these, for others the counters are set to 1 when written to and
+	 * instead cleared when read. So we clear them both ways ...
+	 */
+	iowrite32(0, ioaddr + RxMissed);
+	ioread16(ioaddr + RxCRCErrs);
+	ioread16(ioaddr + RxMissed);
+}
+
+#define RHINE_EVENT_NAPI_RX	(IntrRxDone | \
+				 IntrRxErr | \
+				 IntrRxEmpty | \
+				 IntrRxOverflow	| \
+				 IntrRxDropped | \
+				 IntrRxNoBuf | \
+				 IntrRxWakeUp)
+
+#define RHINE_EVENT_NAPI_TX_ERR	(IntrTxError | \
+				 IntrTxAborted | \
+				 IntrTxUnderrun | \
+				 IntrTxDescRace)
+#define RHINE_EVENT_NAPI_TX	(IntrTxDone | RHINE_EVENT_NAPI_TX_ERR)
+
+#define RHINE_EVENT_NAPI	(RHINE_EVENT_NAPI_RX | \
+				 RHINE_EVENT_NAPI_TX | \
+				 IntrStatsMax)
+#define RHINE_EVENT_SLOW	(IntrPCIErr | IntrLinkChange)
+#define RHINE_EVENT		(RHINE_EVENT_NAPI | RHINE_EVENT_SLOW)
+
 static int rhine_napipoll(struct napi_struct *napi, int budget)
 {
 	struct rhine_private *rp = container_of(napi, struct rhine_private, napi);
 	struct net_device *dev = rp->dev;
 	void __iomem *ioaddr = rp->base;
-	int work_done;
+	u16 enable_mask = RHINE_EVENT & 0xffff;
+	int work_done = 0;
+	u32 status;
+
+	status = get_intr_status(dev);
+	rhine_ack_event(rp, status & ~RHINE_EVENT_SLOW);
+
+	if (status & RHINE_EVENT_NAPI_RX)
+		work_done += rhine_rx(dev, budget);
+
+	if (status & RHINE_EVENT_NAPI_TX) {
+		if (status & RHINE_EVENT_NAPI_TX_ERR) {
+			/* Avoid scavenging before Tx engine turned off */
+			RHINE_WAIT_FOR(!(ioread8(ioaddr + ChipCmd) & CmdTxOn));
+			if (ioread8(ioaddr + ChipCmd) & CmdTxOn)
+				netif_warn(rp, tx_err, dev, "Tx still on\n");
+		}
 
-	work_done = rhine_rx(dev, budget);
+		rhine_tx(dev);
+
+		if (status & RHINE_EVENT_NAPI_TX_ERR)
+			rhine_tx_err(rp, status);
+	}
+
+	if (status & IntrStatsMax) {
+		spin_lock(&rp->lock);
+		rhine_update_rx_crc_and_missed_errord(rp);
+		spin_unlock(&rp->lock);
+	}
+
+	if (status & RHINE_EVENT_SLOW) {
+		enable_mask &= ~RHINE_EVENT_SLOW;
+		schedule_work(&rp->slow_event_task);
+	}
 
 	if (work_done < budget) {
 		napi_complete(napi);
-
-		iowrite16(IntrRxDone | IntrRxErr | IntrRxEmpty| IntrRxOverflow |
-			  IntrRxDropped | IntrRxNoBuf | IntrTxAborted |
-			  IntrTxDone | IntrTxError | IntrTxUnderrun |
-			  IntrPCIErr | IntrStatsMax | IntrLinkChange,
-			  ioaddr + IntrEnable);
+		iowrite16(enable_mask, ioaddr + IntrEnable);
+		mmiowb();
 	}
 	return work_done;
 }
@@ -797,6 +924,7 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
 	rp->quirks = quirks;
 	rp->pioaddr = pioaddr;
 	rp->pdev = pdev;
+	rp->msg_enable = netif_msg_init(-1, RHINE_MSG_DEFAULT);
 
 	rc = pci_request_regions(pdev, DRV_NAME);
 	if (rc)
@@ -856,7 +984,9 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
 	dev->irq = pdev->irq;
 
 	spin_lock_init(&rp->lock);
+	mutex_init(&rp->task_lock);
 	INIT_WORK(&rp->reset_task, rhine_reset_task);
+	INIT_WORK(&rp->slow_event_task, rhine_slow_event_task);
 
 	rp->mii_if.dev = dev;
 	rp->mii_if.mdio_read = mdio_read;
@@ -1310,12 +1440,7 @@ static void init_registers(struct net_device *dev)
 
 	napi_enable(&rp->napi);
 
-	/* Enable interrupts by setting the interrupt mask. */
-	iowrite16(IntrRxDone | IntrRxErr | IntrRxEmpty| IntrRxOverflow |
-	       IntrRxDropped | IntrRxNoBuf | IntrTxAborted |
-	       IntrTxDone | IntrTxError | IntrTxUnderrun |
-	       IntrPCIErr | IntrStatsMax | IntrLinkChange,
-	       ioaddr + IntrEnable);
+	iowrite16(RHINE_EVENT & 0xffff, ioaddr + IntrEnable);
 
 	iowrite16(CmdStart | CmdTxOn | CmdRxOn | (Cmd1NoTxPoll << 8),
 	       ioaddr + ChipCmd);
@@ -1434,13 +1559,10 @@ static void rhine_reset_task(struct work_struct *work)
 						reset_task);
 	struct net_device *dev = rp->dev;
 
-	/* protect against concurrent rx interrupts */
-	disable_irq(rp->pdev->irq);
+	mutex_lock(&rp->task_lock);
 
 	napi_disable(&rp->napi);
 
-	spin_lock_bh(&rp->lock);
-
 	/* clear all descriptors */
 	free_tbufs(dev);
 	free_rbufs(dev);
@@ -1451,12 +1573,11 @@ static void rhine_reset_task(struct work_struct *work)
 	rhine_chip_reset(dev);
 	init_registers(dev);
 
-	spin_unlock_bh(&rp->lock);
-	enable_irq(rp->pdev->irq);
-
 	dev->trans_start = jiffies; /* prevent tx timeout */
 	dev->stats.tx_errors++;
 	netif_wake_queue(dev);
+
+	mutex_unlock(&rp->task_lock);
 }
 
 static void rhine_tx_timeout(struct net_device *dev)
@@ -1477,7 +1598,6 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
 	struct rhine_private *rp = netdev_priv(dev);
 	void __iomem *ioaddr = rp->base;
 	unsigned entry;
-	unsigned long flags;
 
 	/* Caution: the write order is important here, set the field
 	   with the "ownership" bits last. */
@@ -1529,7 +1649,6 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
 		rp->tx_ring[entry].tx_status = 0;
 
 	/* lock eth irq */
-	spin_lock_irqsave(&rp->lock, flags);
 	wmb();
 	rp->tx_ring[entry].tx_status |= cpu_to_le32(DescOwn);
 	wmb();
@@ -1543,15 +1662,12 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
 		BYTE_REG_BITS_ON(1 << 7, ioaddr + TQWake);
 
 	/* Wake the potentially-idle transmit channel */
-	iowrite8(ioread8(ioaddr + ChipCmd1) | Cmd1TxDemand,
-	       ioaddr + ChipCmd1);
+	iowrite8(ioread8(ioaddr + ChipCmd1) | Cmd1TxDemand, ioaddr + ChipCmd1);
 	IOSYNC;
 
 	if (rp->cur_tx == rp->dirty_tx + TX_QUEUE_LEN)
 		netif_stop_queue(dev);
 
-	spin_unlock_irqrestore(&rp->lock, flags);
-
 	if (debug > 4) {
 		netdev_dbg(dev, "Transmit frame #%d queued in slot %d\n",
 			   rp->cur_tx-1, entry);
@@ -1565,63 +1681,26 @@ static irqreturn_t rhine_interrupt(int irq, void *dev_instance)
 {
 	struct net_device *dev = dev_instance;
 	struct rhine_private *rp = netdev_priv(dev);
-	void __iomem *ioaddr = rp->base;
-	u32 intr_status;
-	int boguscnt = max_interrupt_work;
+	u32 status;
 	int handled = 0;
 
-	while ((intr_status = get_intr_status(dev))) {
-		handled = 1;
-
-		/* Acknowledge all of the current interrupt sources ASAP. */
-		if (intr_status & IntrTxDescRace)
-			iowrite8(0x08, ioaddr + IntrStatus2);
-		iowrite16(intr_status & 0xffff, ioaddr + IntrStatus);
-		IOSYNC;
-
-		if (debug > 4)
-			netdev_dbg(dev, "Interrupt, status %08x\n",
-				   intr_status);
-
-		if (intr_status & (IntrRxDone | IntrRxErr | IntrRxDropped |
-				   IntrRxWakeUp | IntrRxEmpty | IntrRxNoBuf)) {
-			iowrite16(IntrTxAborted |
-				  IntrTxDone | IntrTxError | IntrTxUnderrun |
-				  IntrPCIErr | IntrStatsMax | IntrLinkChange,
-				  ioaddr + IntrEnable);
+	status = get_intr_status(dev);
 
-			napi_schedule(&rp->napi);
-		}
+	netif_dbg(rp, intr, dev, "Interrupt, status %08x\n", status);
 
-		if (intr_status & (IntrTxErrSummary | IntrTxDone)) {
-			if (intr_status & IntrTxErrSummary) {
-				/* Avoid scavenging before Tx engine turned off */
-				RHINE_WAIT_FOR(!(ioread8(ioaddr+ChipCmd) & CmdTxOn));
-				if (debug > 2 &&
-				    ioread8(ioaddr+ChipCmd) & CmdTxOn)
-					netdev_warn(dev,
-						    "%s: Tx engine still on\n",
-						    __func__);
-			}
-			rhine_tx(dev);
-		}
+	if (status & RHINE_EVENT) {
+		handled = 1;
 
-		/* Abnormal error summary/uncommon events handlers. */
-		if (intr_status & (IntrPCIErr | IntrLinkChange |
-				   IntrStatsMax | IntrTxError | IntrTxAborted |
-				   IntrTxUnderrun | IntrTxDescRace))
-			rhine_error(dev, intr_status);
+		iowrite16(0x0000, rp->base + IntrEnable);
+		mmiowb();
+		napi_schedule(&rp->napi);
+	}
 
-		if (--boguscnt < 0) {
-			netdev_warn(dev, "Too much work at interrupt, status=%#08x\n",
-				    intr_status);
-			break;
-		}
+	if (status & ~(IntrLinkChange | IntrStatsMax | RHINE_EVENT_NAPI)) {
+		netif_err(rp, intr, dev, "Something Wicked happened! %08x\n",
+			  status);
 	}
 
-	if (debug > 3)
-		netdev_dbg(dev, "exiting interrupt, status=%08x\n",
-			   ioread16(ioaddr + IntrStatus));
 	return IRQ_RETVAL(handled);
 }
 
@@ -1632,8 +1711,6 @@ static void rhine_tx(struct net_device *dev)
 	struct rhine_private *rp = netdev_priv(dev);
 	int txstatus = 0, entry = rp->dirty_tx % TX_RING_SIZE;
 
-	spin_lock(&rp->lock);
-
 	/* find and cleanup dirty tx descriptors */
 	while (rp->dirty_tx != rp->cur_tx) {
 		txstatus = le32_to_cpu(rp->tx_ring[entry].tx_status);
@@ -1687,8 +1764,6 @@ static void rhine_tx(struct net_device *dev)
 	}
 	if ((rp->cur_tx - rp->dirty_tx) < TX_QUEUE_LEN - 4)
 		netif_wake_queue(dev);
-
-	spin_unlock(&rp->lock);
 }
 
 /**
@@ -1839,20 +1914,8 @@ static int rhine_rx(struct net_device *dev, int limit)
 	return count;
 }
 
-/*
- * Clears the "tally counters" for CRC errors and missed frames(?).
- * It has been reported that some chips need a write of 0 to clear
- * these, for others the counters are set to 1 when written to and
- * instead cleared when read. So we clear them both ways ...
- */
-static inline void clear_tally_counters(void __iomem *ioaddr)
+static void rhine_restart_tx(struct net_device *dev)
 {
-	iowrite32(0, ioaddr + RxMissed);
-	ioread16(ioaddr + RxCRCErrs);
-	ioread16(ioaddr + RxMissed);
-}
-
-static void rhine_restart_tx(struct net_device *dev) {
 	struct rhine_private *rp = netdev_priv(dev);
 	void __iomem *ioaddr = rp->base;
 	int entry = rp->dirty_tx % TX_RING_SIZE;
@@ -1880,8 +1943,7 @@ static void rhine_restart_tx(struct net_device *dev) {
 		iowrite8(ioread8(ioaddr + ChipCmd1) | Cmd1TxDemand,
 		       ioaddr + ChipCmd1);
 		IOSYNC;
-	}
-	else {
+	} else {
 		/* This should never happen */
 		if (debug > 1)
 			netdev_warn(dev, "%s() Another error occurred %08x\n",
@@ -1890,72 +1952,46 @@ static void rhine_restart_tx(struct net_device *dev) {
 
 }
 
-static void rhine_error(struct net_device *dev, int intr_status)
+static void rhine_slow_event_task(struct work_struct *work)
 {
-	struct rhine_private *rp = netdev_priv(dev);
+	struct rhine_private *rp =
+		container_of(work, struct rhine_private, slow_event_task);
+	struct net_device *dev = rp->dev;
 	void __iomem *ioaddr = rp->base;
+	u32 intr_status;
 
-	spin_lock(&rp->lock);
+	if (!netif_running(dev))
+		return;
+
+	mutex_lock(&rp->task_lock);
+
+	intr_status = get_intr_status(dev);
+	rhine_ack_event(rp, intr_status & RHINE_EVENT_SLOW);
 
 	if (intr_status & IntrLinkChange)
 		rhine_check_media(dev, 0);
-	if (intr_status & IntrStatsMax) {
-		dev->stats.rx_crc_errors += ioread16(ioaddr + RxCRCErrs);
-		dev->stats.rx_missed_errors += ioread16(ioaddr + RxMissed);
-		clear_tally_counters(ioaddr);
-	}
-	if (intr_status & IntrTxAborted) {
-		if (debug > 1)
-			netdev_info(dev, "Abort %08x, frame dropped\n",
-				    intr_status);
-	}
-	if (intr_status & IntrTxUnderrun) {
-		if (rp->tx_thresh < 0xE0)
-			BYTE_REG_BITS_SET((rp->tx_thresh += 0x20), 0x80, ioaddr + TxConfig);
-		if (debug > 1)
-			netdev_info(dev, "Transmitter underrun, Tx threshold now %02x\n",
-				    rp->tx_thresh);
-	}
-	if (intr_status & IntrTxDescRace) {
-		if (debug > 2)
-			netdev_info(dev, "Tx descriptor write-back race\n");
-	}
-	if ((intr_status & IntrTxError) &&
-	    (intr_status & (IntrTxAborted |
-	     IntrTxUnderrun | IntrTxDescRace)) == 0) {
-		if (rp->tx_thresh < 0xE0) {
-			BYTE_REG_BITS_SET((rp->tx_thresh += 0x20), 0x80, ioaddr + TxConfig);
-		}
-		if (debug > 1)
-			netdev_info(dev, "Unspecified error. Tx threshold now %02x\n",
-				    rp->tx_thresh);
-	}
-	if (intr_status & (IntrTxAborted | IntrTxUnderrun | IntrTxDescRace |
-			   IntrTxError))
-		rhine_restart_tx(dev);
 
-	if (intr_status & ~(IntrLinkChange | IntrStatsMax | IntrTxUnderrun |
-			    IntrTxError | IntrTxAborted | IntrNormalSummary |
-			    IntrTxDescRace)) {
-		if (debug > 1)
-			netdev_err(dev, "Something Wicked happened! %08x\n",
-				   intr_status);
-	}
+	/* FIXME: ignore ? */
+	if (intr_status & IntrPCIErr)
+		netif_warn(rp, hw, dev, "PCI error\n");
 
-	spin_unlock(&rp->lock);
+	napi_disable(&rp->napi);
+	iowrite16(0x0000, ioaddr + IntrEnable);
+	mmiowb();
+	/* Slow and safe. Consider __napi_schedule as a replacement ? */
+	napi_enable(&rp->napi);
+	napi_schedule(&rp->napi);
+
+	mutex_unlock(&rp->task_lock);
 }
 
 static struct net_device_stats *rhine_get_stats(struct net_device *dev)
 {
 	struct rhine_private *rp = netdev_priv(dev);
-	void __iomem *ioaddr = rp->base;
-	unsigned long flags;
 
-	spin_lock_irqsave(&rp->lock, flags);
-	dev->stats.rx_crc_errors += ioread16(ioaddr + RxCRCErrs);
-	dev->stats.rx_missed_errors += ioread16(ioaddr + RxMissed);
-	clear_tally_counters(ioaddr);
-	spin_unlock_irqrestore(&rp->lock, flags);
+	spin_lock_bh(&rp->lock);
+	rhine_update_rx_crc_and_missed_errord(rp);
+	spin_unlock_bh(&rp->lock);
 
 	return &dev->stats;
 }
@@ -2022,9 +2058,9 @@ static int netdev_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 	struct rhine_private *rp = netdev_priv(dev);
 	int rc;
 
-	spin_lock_irq(&rp->lock);
+	mutex_lock(&rp->task_lock);
 	rc = mii_ethtool_gset(&rp->mii_if, cmd);
-	spin_unlock_irq(&rp->lock);
+	mutex_unlock(&rp->task_lock);
 
 	return rc;
 }
@@ -2034,10 +2070,10 @@ static int netdev_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 	struct rhine_private *rp = netdev_priv(dev);
 	int rc;
 
-	spin_lock_irq(&rp->lock);
+	mutex_lock(&rp->task_lock);
 	rc = mii_ethtool_sset(&rp->mii_if, cmd);
-	spin_unlock_irq(&rp->lock);
 	rhine_set_carrier(&rp->mii_if);
+	mutex_unlock(&rp->task_lock);
 
 	return rc;
 }
@@ -2073,6 +2109,7 @@ static void rhine_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 	if (!(rp->quirks & rqWOL))
 		return;
 
+	/* FIXME: huh ? */
 	spin_lock_irq(&rp->lock);
 	wol->supported = WAKE_PHY | WAKE_MAGIC |
 			 WAKE_UCAST | WAKE_MCAST | WAKE_BCAST;	/* Untested */
@@ -2092,6 +2129,7 @@ static int rhine_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 	if (wol->wolopts & ~support)
 		return -EINVAL;
 
+	/* FIXME: huh (sic) ? */
 	spin_lock_irq(&rp->lock);
 	rp->wolopts = wol->wolopts;
 	spin_unlock_irq(&rp->lock);
@@ -2119,10 +2157,10 @@ static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	if (!netif_running(dev))
 		return -EINVAL;
 
-	spin_lock_irq(&rp->lock);
+	mutex_lock(&rp->task_lock);
 	rc = generic_mii_ioctl(&rp->mii_if, if_mii(rq), cmd, NULL);
-	spin_unlock_irq(&rp->lock);
 	rhine_set_carrier(&rp->mii_if);
+	mutex_unlock(&rp->task_lock);
 
 	return rc;
 }
@@ -2133,11 +2171,10 @@ static int rhine_close(struct net_device *dev)
 	void __iomem *ioaddr = rp->base;
 
 	napi_disable(&rp->napi);
+	cancel_work_sync(&rp->slow_event_task);
 	cancel_work_sync(&rp->reset_task);
 	netif_stop_queue(dev);
 
-	spin_lock_irq(&rp->lock);
-
 	if (debug > 1)
 		netdev_dbg(dev, "Shutting down ethercard, status was %04x\n",
 			   ioread16(ioaddr + ChipCmd));
@@ -2151,8 +2188,6 @@ static int rhine_close(struct net_device *dev)
 	/* Stop the chip's Tx and Rx processes. */
 	iowrite16(CmdStop, ioaddr + ChipCmd);
 
-	spin_unlock_irq(&rp->lock);
-
 	free_irq(rp->pdev->irq, dev);
 	free_rbufs(dev);
 	free_tbufs(dev);
@@ -2239,10 +2274,12 @@ static int rhine_suspend(struct pci_dev *pdev, pm_message_t state)
 	netif_device_detach(dev);
 	pci_save_state(pdev);
 
+	/* FIXME: ? */
 	spin_lock_irqsave(&rp->lock, flags);
 	rhine_shutdown(pdev);
 	spin_unlock_irqrestore(&rp->lock, flags);
 
+	/* FIXME: nuclear workaround ? Check the commit log. */
 	free_irq(dev->irq, dev);
 	return 0;
 }
@@ -2257,6 +2294,7 @@ static int rhine_resume(struct pci_dev *pdev)
 	if (!netif_running(dev))
 		return 0;
 
+	/* FIXME: see above. */
 	if (request_irq(dev->irq, rhine_interrupt, IRQF_SHARED, dev->name, dev))
 		netdev_err(dev, "request_irq failed\n");
 

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/29 Francois Romieu <romieu@fr.zoreil.com>:
> Francois Romieu <romieu@fr.zoreil.com> :
> [...]
>> Feel free to try the extra "new year's eve week end is getting closer"
>> patch below (against net-next).
>>
>> You have been warned.
>
> The one below could be a bit better.
>
> Out for dinner.

Thanks, I'll try that one in a while, once I'm done bisecting 3.1.4 ->
3.1.5, since all kernels after 3.1.4 won't pass control onto init.

/Bjarke

> diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
> index 5c4983b..66e5c08 100644
> --- a/drivers/net/ethernet/via/via-rhine.c
> +++ b/drivers/net/ethernet/via/via-rhine.c
> @@ -42,7 +42,8 @@
>
>  #define DEBUG
>  static int debug = 1;  /* 1 normal messages, 0 quiet .. 7 verbose. */
> -static int max_interrupt_work = 20;
> +#define RHINE_MSG_DEFAULT \
> +        (NETIF_MSG_INTR)
>
>  /* Set the copy breakpoint for the copy-only-tiny-frames scheme.
>    Setting to > 1518 effectively disables this feature. */
> @@ -128,11 +129,9 @@ MODULE_AUTHOR("Donald Becker <becker@scyld.com>");
>  MODULE_DESCRIPTION("VIA Rhine PCI Fast Ethernet driver");
>  MODULE_LICENSE("GPL");
>
> -module_param(max_interrupt_work, int, 0);
>  module_param(debug, int, 0);
>  module_param(rx_copybreak, int, 0);
>  module_param(avoid_D3, bool, 0);
> -MODULE_PARM_DESC(max_interrupt_work, "VIA Rhine maximum events handled per interrupt");
>  MODULE_PARM_DESC(debug, "VIA Rhine debug level (0-7)");
>  MODULE_PARM_DESC(rx_copybreak, "VIA Rhine copy breakpoint for copy-only-tiny-frames");
>  MODULE_PARM_DESC(avoid_D3, "Avoid power state D3 (work-around for broken BIOSes)");
> @@ -351,16 +350,25 @@ static const int mmio_verify_registers[] = {
>
>  /* Bits in the interrupt status/mask registers. */
>  enum intr_status_bits {
> -       IntrRxDone=0x0001, IntrRxErr=0x0004, IntrRxEmpty=0x0020,
> -       IntrTxDone=0x0002, IntrTxError=0x0008, IntrTxUnderrun=0x0210,
> -       IntrPCIErr=0x0040,
> -       IntrStatsMax=0x0080, IntrRxEarly=0x0100,
> -       IntrRxOverflow=0x0400, IntrRxDropped=0x0800, IntrRxNoBuf=0x1000,
> -       IntrTxAborted=0x2000, IntrLinkChange=0x4000,
> -       IntrRxWakeUp=0x8000,
> -       IntrNormalSummary=0x0003, IntrAbnormalSummary=0xC260,
> -       IntrTxDescRace=0x080000,        /* mapped from IntrStatus2 */
> -       IntrTxErrSummary=0x082218,
> +       IntrRxDone      = 0x0001,
> +       IntrTxDone      = 0x0002,
> +       IntrRxErr       = 0x0004,
> +       IntrTxError     = 0x0008,
> +       IntrRxEmpty     = 0x0020,
> +       IntrPCIErr      = 0x0040,
> +       IntrStatsMax    = 0x0080,
> +       IntrRxEarly     = 0x0100,
> +       IntrTxUnderrun  = 0x0210,
> +       IntrRxOverflow  = 0x0400,
> +       IntrRxDropped   = 0x0800,
> +       IntrRxNoBuf     = 0x1000,
> +       IntrTxAborted   = 0x2000,
> +       IntrLinkChange  = 0x4000,
> +       IntrRxWakeUp    = 0x8000,
> +       IntrTxDescRace          = 0x080000,     /* mapped from IntrStatus2 */
> +       IntrNormalSummary       = IntrRxDone | IntrTxDone,
> +       IntrTxErrSummary        = IntrTxDescRace | IntrTxAborted | IntrTxError |
> +                                 IntrTxUnderrun,
>  };
>
>  /* Bits in WOLcrSet/WOLcrClr and PwrcsrSet/PwrcsrClr */
> @@ -439,8 +447,12 @@ struct rhine_private {
>        struct net_device *dev;
>        struct napi_struct napi;
>        spinlock_t lock;
> +       struct mutex task_lock;
> +       struct work_struct slow_event_task;
>        struct work_struct reset_task;
>
> +       u32 msg_enable;
> +
>        /* Frequently used values: keep some adjacent for cache effect. */
>        u32 quirks;
>        struct rx_desc *rx_head_desc;
> @@ -482,7 +494,6 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
>  static irqreturn_t rhine_interrupt(int irq, void *dev_instance);
>  static void rhine_tx(struct net_device *dev);
>  static int rhine_rx(struct net_device *dev, int limit);
> -static void rhine_error(struct net_device *dev, int intr_status);
>  static void rhine_set_rx_mode(struct net_device *dev);
>  static struct net_device_stats *rhine_get_stats(struct net_device *dev);
>  static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
> @@ -497,6 +508,8 @@ static void rhine_set_cam_mask(void __iomem *ioaddr, u32 mask);
>  static void rhine_set_vlan_cam_mask(void __iomem *ioaddr, u32 mask);
>  static void rhine_init_cam_filter(struct net_device *dev);
>  static void rhine_update_vcam(struct net_device *dev);
> +static void rhine_slow_event_task(struct work_struct *work);
> +static void rhine_restart_tx(struct net_device *dev);
>
>  #define RHINE_WAIT_FOR(condition)                              \
>  do {                                                           \
> @@ -508,7 +521,7 @@ do {                                                                \
>                        1024 - i, __func__, __LINE__);          \
>  } while (0)
>
> -static inline u32 get_intr_status(struct net_device *dev)
> +static u32 get_intr_status(struct net_device *dev)
>  {
>        struct rhine_private *rp = netdev_priv(dev);
>        void __iomem *ioaddr = rp->base;
> @@ -521,6 +534,16 @@ static inline u32 get_intr_status(struct net_device *dev)
>        return intr_status;
>  }
>
> +static void rhine_ack_event(struct rhine_private *rp, u32 mask)
> +{
> +       void __iomem *ioaddr = rp->base;
> +
> +       if (rp->quirks & rqStatusWBRace)
> +               iowrite8(mask >> 16, ioaddr + IntrStatus2);
> +       iowrite16(mask, ioaddr + IntrStatus);
> +       mmiowb();
> +}
> +
>  /*
>  * Get power related registers into sane state.
>  * Notify user about past WOL event.
> @@ -657,23 +680,127 @@ static void rhine_poll(struct net_device *dev)
>  }
>  #endif
>
> +static void rhine_kick_tx_threshold(struct rhine_private *rp)
> +{
> +       if (rp->tx_thresh < 0xe0) {
> +               void __iomem *ioaddr = rp->base;
> +
> +               rp->tx_thresh += 0x20;
> +               BYTE_REG_BITS_SET(rp->tx_thresh, 0x80, ioaddr + TxConfig);
> +       }
> +}
> +
> +static void rhine_tx_err(struct rhine_private *rp, u32 status)
> +{
> +       struct net_device *dev = rp->dev;
> +
> +       if (status & IntrTxAborted) {
> +               netif_info(rp, tx_err, dev,
> +                          "Abort %08x, frame dropped\n", status);
> +       }
> +
> +       if (status & IntrTxUnderrun) {
> +               rhine_kick_tx_threshold(rp);
> +               netif_info(rp, tx_err ,dev, "Transmitter underrun, "
> +                          "Tx threshold now %02x\n", rp->tx_thresh);
> +       }
> +
> +       if (status & IntrTxDescRace)
> +               netif_info(rp, tx_err, dev, "Tx descriptor write-back race\n");
> +
> +       if ((status & IntrTxError) &&
> +           (status & (IntrTxAborted | IntrTxUnderrun | IntrTxDescRace)) == 0) {
> +               rhine_kick_tx_threshold(rp);
> +               netif_info(rp, tx_err, dev, "Unspecified error. "
> +                          "Tx threshold now %02x\n", rp->tx_thresh);
> +       }
> +
> +       rhine_restart_tx(dev);
> +}
> +
> +static void rhine_update_rx_crc_and_missed_errord(struct rhine_private *rp)
> +{
> +       void __iomem *ioaddr = rp->base;
> +       struct net_device_stats *stats = &rp->dev->stats;
> +
> +       stats->rx_crc_errors    += ioread16(ioaddr + RxCRCErrs);
> +       stats->rx_missed_errors += ioread16(ioaddr + RxMissed);
> +
> +       /*
> +        * Clears the "tally counters" for CRC errors and missed frames(?).
> +        * It has been reported that some chips need a write of 0 to clear
> +        * these, for others the counters are set to 1 when written to and
> +        * instead cleared when read. So we clear them both ways ...
> +        */
> +       iowrite32(0, ioaddr + RxMissed);
> +       ioread16(ioaddr + RxCRCErrs);
> +       ioread16(ioaddr + RxMissed);
> +}
> +
> +#define RHINE_EVENT_NAPI_RX    (IntrRxDone | \
> +                                IntrRxErr | \
> +                                IntrRxEmpty | \
> +                                IntrRxOverflow | \
> +                                IntrRxDropped | \
> +                                IntrRxNoBuf | \
> +                                IntrRxWakeUp)
> +
> +#define RHINE_EVENT_NAPI_TX_ERR        (IntrTxError | \
> +                                IntrTxAborted | \
> +                                IntrTxUnderrun | \
> +                                IntrTxDescRace)
> +#define RHINE_EVENT_NAPI_TX    (IntrTxDone | RHINE_EVENT_NAPI_TX_ERR)
> +
> +#define RHINE_EVENT_NAPI       (RHINE_EVENT_NAPI_RX | \
> +                                RHINE_EVENT_NAPI_TX | \
> +                                IntrStatsMax)
> +#define RHINE_EVENT_SLOW       (IntrPCIErr | IntrLinkChange)
> +#define RHINE_EVENT            (RHINE_EVENT_NAPI | RHINE_EVENT_SLOW)
> +
>  static int rhine_napipoll(struct napi_struct *napi, int budget)
>  {
>        struct rhine_private *rp = container_of(napi, struct rhine_private, napi);
>        struct net_device *dev = rp->dev;
>        void __iomem *ioaddr = rp->base;
> -       int work_done;
> +       u16 enable_mask = RHINE_EVENT & 0xffff;
> +       int work_done = 0;
> +       u32 status;
> +
> +       status = get_intr_status(dev);
> +       rhine_ack_event(rp, status & ~RHINE_EVENT_SLOW);
> +
> +       if (status & RHINE_EVENT_NAPI_RX)
> +               work_done += rhine_rx(dev, budget);
> +
> +       if (status & RHINE_EVENT_NAPI_TX) {
> +               if (status & RHINE_EVENT_NAPI_TX_ERR) {
> +                       /* Avoid scavenging before Tx engine turned off */
> +                       RHINE_WAIT_FOR(!(ioread8(ioaddr + ChipCmd) & CmdTxOn));
> +                       if (ioread8(ioaddr + ChipCmd) & CmdTxOn)
> +                               netif_warn(rp, tx_err, dev, "Tx still on\n");
> +               }
>
> -       work_done = rhine_rx(dev, budget);
> +               rhine_tx(dev);
> +
> +               if (status & RHINE_EVENT_NAPI_TX_ERR)
> +                       rhine_tx_err(rp, status);
> +       }
> +
> +       if (status & IntrStatsMax) {
> +               spin_lock(&rp->lock);
> +               rhine_update_rx_crc_and_missed_errord(rp);
> +               spin_unlock(&rp->lock);
> +       }
> +
> +       if (status & RHINE_EVENT_SLOW) {
> +               enable_mask &= ~RHINE_EVENT_SLOW;
> +               schedule_work(&rp->slow_event_task);
> +       }
>
>        if (work_done < budget) {
>                napi_complete(napi);
> -
> -               iowrite16(IntrRxDone | IntrRxErr | IntrRxEmpty| IntrRxOverflow |
> -                         IntrRxDropped | IntrRxNoBuf | IntrTxAborted |
> -                         IntrTxDone | IntrTxError | IntrTxUnderrun |
> -                         IntrPCIErr | IntrStatsMax | IntrLinkChange,
> -                         ioaddr + IntrEnable);
> +               iowrite16(enable_mask, ioaddr + IntrEnable);
> +               mmiowb();
>        }
>        return work_done;
>  }
> @@ -797,6 +924,7 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
>        rp->quirks = quirks;
>        rp->pioaddr = pioaddr;
>        rp->pdev = pdev;
> +       rp->msg_enable = netif_msg_init(-1, RHINE_MSG_DEFAULT);
>
>        rc = pci_request_regions(pdev, DRV_NAME);
>        if (rc)
> @@ -856,7 +984,9 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
>        dev->irq = pdev->irq;
>
>        spin_lock_init(&rp->lock);
> +       mutex_init(&rp->task_lock);
>        INIT_WORK(&rp->reset_task, rhine_reset_task);
> +       INIT_WORK(&rp->slow_event_task, rhine_slow_event_task);
>
>        rp->mii_if.dev = dev;
>        rp->mii_if.mdio_read = mdio_read;
> @@ -1310,12 +1440,7 @@ static void init_registers(struct net_device *dev)
>
>        napi_enable(&rp->napi);
>
> -       /* Enable interrupts by setting the interrupt mask. */
> -       iowrite16(IntrRxDone | IntrRxErr | IntrRxEmpty| IntrRxOverflow |
> -              IntrRxDropped | IntrRxNoBuf | IntrTxAborted |
> -              IntrTxDone | IntrTxError | IntrTxUnderrun |
> -              IntrPCIErr | IntrStatsMax | IntrLinkChange,
> -              ioaddr + IntrEnable);
> +       iowrite16(RHINE_EVENT & 0xffff, ioaddr + IntrEnable);
>
>        iowrite16(CmdStart | CmdTxOn | CmdRxOn | (Cmd1NoTxPoll << 8),
>               ioaddr + ChipCmd);
> @@ -1434,13 +1559,10 @@ static void rhine_reset_task(struct work_struct *work)
>                                                reset_task);
>        struct net_device *dev = rp->dev;
>
> -       /* protect against concurrent rx interrupts */
> -       disable_irq(rp->pdev->irq);
> +       mutex_lock(&rp->task_lock);
>
>        napi_disable(&rp->napi);
>
> -       spin_lock_bh(&rp->lock);
> -
>        /* clear all descriptors */
>        free_tbufs(dev);
>        free_rbufs(dev);
> @@ -1451,12 +1573,11 @@ static void rhine_reset_task(struct work_struct *work)
>        rhine_chip_reset(dev);
>        init_registers(dev);
>
> -       spin_unlock_bh(&rp->lock);
> -       enable_irq(rp->pdev->irq);
> -
>        dev->trans_start = jiffies; /* prevent tx timeout */
>        dev->stats.tx_errors++;
>        netif_wake_queue(dev);
> +
> +       mutex_unlock(&rp->task_lock);
>  }
>
>  static void rhine_tx_timeout(struct net_device *dev)
> @@ -1477,7 +1598,6 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
>        struct rhine_private *rp = netdev_priv(dev);
>        void __iomem *ioaddr = rp->base;
>        unsigned entry;
> -       unsigned long flags;
>
>        /* Caution: the write order is important here, set the field
>           with the "ownership" bits last. */
> @@ -1529,7 +1649,6 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
>                rp->tx_ring[entry].tx_status = 0;
>
>        /* lock eth irq */
> -       spin_lock_irqsave(&rp->lock, flags);
>        wmb();
>        rp->tx_ring[entry].tx_status |= cpu_to_le32(DescOwn);
>        wmb();
> @@ -1543,15 +1662,12 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
>                BYTE_REG_BITS_ON(1 << 7, ioaddr + TQWake);
>
>        /* Wake the potentially-idle transmit channel */
> -       iowrite8(ioread8(ioaddr + ChipCmd1) | Cmd1TxDemand,
> -              ioaddr + ChipCmd1);
> +       iowrite8(ioread8(ioaddr + ChipCmd1) | Cmd1TxDemand, ioaddr + ChipCmd1);
>        IOSYNC;
>
>        if (rp->cur_tx == rp->dirty_tx + TX_QUEUE_LEN)
>                netif_stop_queue(dev);
>
> -       spin_unlock_irqrestore(&rp->lock, flags);
> -
>        if (debug > 4) {
>                netdev_dbg(dev, "Transmit frame #%d queued in slot %d\n",
>                           rp->cur_tx-1, entry);
> @@ -1565,63 +1681,26 @@ static irqreturn_t rhine_interrupt(int irq, void *dev_instance)
>  {
>        struct net_device *dev = dev_instance;
>        struct rhine_private *rp = netdev_priv(dev);
> -       void __iomem *ioaddr = rp->base;
> -       u32 intr_status;
> -       int boguscnt = max_interrupt_work;
> +       u32 status;
>        int handled = 0;
>
> -       while ((intr_status = get_intr_status(dev))) {
> -               handled = 1;
> -
> -               /* Acknowledge all of the current interrupt sources ASAP. */
> -               if (intr_status & IntrTxDescRace)
> -                       iowrite8(0x08, ioaddr + IntrStatus2);
> -               iowrite16(intr_status & 0xffff, ioaddr + IntrStatus);
> -               IOSYNC;
> -
> -               if (debug > 4)
> -                       netdev_dbg(dev, "Interrupt, status %08x\n",
> -                                  intr_status);
> -
> -               if (intr_status & (IntrRxDone | IntrRxErr | IntrRxDropped |
> -                                  IntrRxWakeUp | IntrRxEmpty | IntrRxNoBuf)) {
> -                       iowrite16(IntrTxAborted |
> -                                 IntrTxDone | IntrTxError | IntrTxUnderrun |
> -                                 IntrPCIErr | IntrStatsMax | IntrLinkChange,
> -                                 ioaddr + IntrEnable);
> +       status = get_intr_status(dev);
>
> -                       napi_schedule(&rp->napi);
> -               }
> +       netif_dbg(rp, intr, dev, "Interrupt, status %08x\n", status);
>
> -               if (intr_status & (IntrTxErrSummary | IntrTxDone)) {
> -                       if (intr_status & IntrTxErrSummary) {
> -                               /* Avoid scavenging before Tx engine turned off */
> -                               RHINE_WAIT_FOR(!(ioread8(ioaddr+ChipCmd) & CmdTxOn));
> -                               if (debug > 2 &&
> -                                   ioread8(ioaddr+ChipCmd) & CmdTxOn)
> -                                       netdev_warn(dev,
> -                                                   "%s: Tx engine still on\n",
> -                                                   __func__);
> -                       }
> -                       rhine_tx(dev);
> -               }
> +       if (status & RHINE_EVENT) {
> +               handled = 1;
>
> -               /* Abnormal error summary/uncommon events handlers. */
> -               if (intr_status & (IntrPCIErr | IntrLinkChange |
> -                                  IntrStatsMax | IntrTxError | IntrTxAborted |
> -                                  IntrTxUnderrun | IntrTxDescRace))
> -                       rhine_error(dev, intr_status);
> +               iowrite16(0x0000, rp->base + IntrEnable);
> +               mmiowb();
> +               napi_schedule(&rp->napi);
> +       }
>
> -               if (--boguscnt < 0) {
> -                       netdev_warn(dev, "Too much work at interrupt, status=%#08x\n",
> -                                   intr_status);
> -                       break;
> -               }
> +       if (status & ~(IntrLinkChange | IntrStatsMax | RHINE_EVENT_NAPI)) {
> +               netif_err(rp, intr, dev, "Something Wicked happened! %08x\n",
> +                         status);
>        }
>
> -       if (debug > 3)
> -               netdev_dbg(dev, "exiting interrupt, status=%08x\n",
> -                          ioread16(ioaddr + IntrStatus));
>        return IRQ_RETVAL(handled);
>  }
>
> @@ -1632,8 +1711,6 @@ static void rhine_tx(struct net_device *dev)
>        struct rhine_private *rp = netdev_priv(dev);
>        int txstatus = 0, entry = rp->dirty_tx % TX_RING_SIZE;
>
> -       spin_lock(&rp->lock);
> -
>        /* find and cleanup dirty tx descriptors */
>        while (rp->dirty_tx != rp->cur_tx) {
>                txstatus = le32_to_cpu(rp->tx_ring[entry].tx_status);
> @@ -1687,8 +1764,6 @@ static void rhine_tx(struct net_device *dev)
>        }
>        if ((rp->cur_tx - rp->dirty_tx) < TX_QUEUE_LEN - 4)
>                netif_wake_queue(dev);
> -
> -       spin_unlock(&rp->lock);
>  }
>
>  /**
> @@ -1839,20 +1914,8 @@ static int rhine_rx(struct net_device *dev, int limit)
>        return count;
>  }
>
> -/*
> - * Clears the "tally counters" for CRC errors and missed frames(?).
> - * It has been reported that some chips need a write of 0 to clear
> - * these, for others the counters are set to 1 when written to and
> - * instead cleared when read. So we clear them both ways ...
> - */
> -static inline void clear_tally_counters(void __iomem *ioaddr)
> +static void rhine_restart_tx(struct net_device *dev)
>  {
> -       iowrite32(0, ioaddr + RxMissed);
> -       ioread16(ioaddr + RxCRCErrs);
> -       ioread16(ioaddr + RxMissed);
> -}
> -
> -static void rhine_restart_tx(struct net_device *dev) {
>        struct rhine_private *rp = netdev_priv(dev);
>        void __iomem *ioaddr = rp->base;
>        int entry = rp->dirty_tx % TX_RING_SIZE;
> @@ -1880,8 +1943,7 @@ static void rhine_restart_tx(struct net_device *dev) {
>                iowrite8(ioread8(ioaddr + ChipCmd1) | Cmd1TxDemand,
>                       ioaddr + ChipCmd1);
>                IOSYNC;
> -       }
> -       else {
> +       } else {
>                /* This should never happen */
>                if (debug > 1)
>                        netdev_warn(dev, "%s() Another error occurred %08x\n",
> @@ -1890,72 +1952,46 @@ static void rhine_restart_tx(struct net_device *dev) {
>
>  }
>
> -static void rhine_error(struct net_device *dev, int intr_status)
> +static void rhine_slow_event_task(struct work_struct *work)
>  {
> -       struct rhine_private *rp = netdev_priv(dev);
> +       struct rhine_private *rp =
> +               container_of(work, struct rhine_private, slow_event_task);
> +       struct net_device *dev = rp->dev;
>        void __iomem *ioaddr = rp->base;
> +       u32 intr_status;
>
> -       spin_lock(&rp->lock);
> +       if (!netif_running(dev))
> +               return;
> +
> +       mutex_lock(&rp->task_lock);
> +
> +       intr_status = get_intr_status(dev);
> +       rhine_ack_event(rp, intr_status & RHINE_EVENT_SLOW);
>
>        if (intr_status & IntrLinkChange)
>                rhine_check_media(dev, 0);
> -       if (intr_status & IntrStatsMax) {
> -               dev->stats.rx_crc_errors += ioread16(ioaddr + RxCRCErrs);
> -               dev->stats.rx_missed_errors += ioread16(ioaddr + RxMissed);
> -               clear_tally_counters(ioaddr);
> -       }
> -       if (intr_status & IntrTxAborted) {
> -               if (debug > 1)
> -                       netdev_info(dev, "Abort %08x, frame dropped\n",
> -                                   intr_status);
> -       }
> -       if (intr_status & IntrTxUnderrun) {
> -               if (rp->tx_thresh < 0xE0)
> -                       BYTE_REG_BITS_SET((rp->tx_thresh += 0x20), 0x80, ioaddr + TxConfig);
> -               if (debug > 1)
> -                       netdev_info(dev, "Transmitter underrun, Tx threshold now %02x\n",
> -                                   rp->tx_thresh);
> -       }
> -       if (intr_status & IntrTxDescRace) {
> -               if (debug > 2)
> -                       netdev_info(dev, "Tx descriptor write-back race\n");
> -       }
> -       if ((intr_status & IntrTxError) &&
> -           (intr_status & (IntrTxAborted |
> -            IntrTxUnderrun | IntrTxDescRace)) == 0) {
> -               if (rp->tx_thresh < 0xE0) {
> -                       BYTE_REG_BITS_SET((rp->tx_thresh += 0x20), 0x80, ioaddr + TxConfig);
> -               }
> -               if (debug > 1)
> -                       netdev_info(dev, "Unspecified error. Tx threshold now %02x\n",
> -                                   rp->tx_thresh);
> -       }
> -       if (intr_status & (IntrTxAborted | IntrTxUnderrun | IntrTxDescRace |
> -                          IntrTxError))
> -               rhine_restart_tx(dev);
>
> -       if (intr_status & ~(IntrLinkChange | IntrStatsMax | IntrTxUnderrun |
> -                           IntrTxError | IntrTxAborted | IntrNormalSummary |
> -                           IntrTxDescRace)) {
> -               if (debug > 1)
> -                       netdev_err(dev, "Something Wicked happened! %08x\n",
> -                                  intr_status);
> -       }
> +       /* FIXME: ignore ? */
> +       if (intr_status & IntrPCIErr)
> +               netif_warn(rp, hw, dev, "PCI error\n");
>
> -       spin_unlock(&rp->lock);
> +       napi_disable(&rp->napi);
> +       iowrite16(0x0000, ioaddr + IntrEnable);
> +       mmiowb();
> +       /* Slow and safe. Consider __napi_schedule as a replacement ? */
> +       napi_enable(&rp->napi);
> +       napi_schedule(&rp->napi);
> +
> +       mutex_unlock(&rp->task_lock);
>  }
>
>  static struct net_device_stats *rhine_get_stats(struct net_device *dev)
>  {
>        struct rhine_private *rp = netdev_priv(dev);
> -       void __iomem *ioaddr = rp->base;
> -       unsigned long flags;
>
> -       spin_lock_irqsave(&rp->lock, flags);
> -       dev->stats.rx_crc_errors += ioread16(ioaddr + RxCRCErrs);
> -       dev->stats.rx_missed_errors += ioread16(ioaddr + RxMissed);
> -       clear_tally_counters(ioaddr);
> -       spin_unlock_irqrestore(&rp->lock, flags);
> +       spin_lock_bh(&rp->lock);
> +       rhine_update_rx_crc_and_missed_errord(rp);
> +       spin_unlock_bh(&rp->lock);
>
>        return &dev->stats;
>  }
> @@ -2022,9 +2058,9 @@ static int netdev_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
>        struct rhine_private *rp = netdev_priv(dev);
>        int rc;
>
> -       spin_lock_irq(&rp->lock);
> +       mutex_lock(&rp->task_lock);
>        rc = mii_ethtool_gset(&rp->mii_if, cmd);
> -       spin_unlock_irq(&rp->lock);
> +       mutex_unlock(&rp->task_lock);
>
>        return rc;
>  }
> @@ -2034,10 +2070,10 @@ static int netdev_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
>        struct rhine_private *rp = netdev_priv(dev);
>        int rc;
>
> -       spin_lock_irq(&rp->lock);
> +       mutex_lock(&rp->task_lock);
>        rc = mii_ethtool_sset(&rp->mii_if, cmd);
> -       spin_unlock_irq(&rp->lock);
>        rhine_set_carrier(&rp->mii_if);
> +       mutex_unlock(&rp->task_lock);
>
>        return rc;
>  }
> @@ -2073,6 +2109,7 @@ static void rhine_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
>        if (!(rp->quirks & rqWOL))
>                return;
>
> +       /* FIXME: huh ? */
>        spin_lock_irq(&rp->lock);
>        wol->supported = WAKE_PHY | WAKE_MAGIC |
>                         WAKE_UCAST | WAKE_MCAST | WAKE_BCAST;  /* Untested */
> @@ -2092,6 +2129,7 @@ static int rhine_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
>        if (wol->wolopts & ~support)
>                return -EINVAL;
>
> +       /* FIXME: huh (sic) ? */
>        spin_lock_irq(&rp->lock);
>        rp->wolopts = wol->wolopts;
>        spin_unlock_irq(&rp->lock);
> @@ -2119,10 +2157,10 @@ static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
>        if (!netif_running(dev))
>                return -EINVAL;
>
> -       spin_lock_irq(&rp->lock);
> +       mutex_lock(&rp->task_lock);
>        rc = generic_mii_ioctl(&rp->mii_if, if_mii(rq), cmd, NULL);
> -       spin_unlock_irq(&rp->lock);
>        rhine_set_carrier(&rp->mii_if);
> +       mutex_unlock(&rp->task_lock);
>
>        return rc;
>  }
> @@ -2133,11 +2171,10 @@ static int rhine_close(struct net_device *dev)
>        void __iomem *ioaddr = rp->base;
>
>        napi_disable(&rp->napi);
> +       cancel_work_sync(&rp->slow_event_task);
>        cancel_work_sync(&rp->reset_task);
>        netif_stop_queue(dev);
>
> -       spin_lock_irq(&rp->lock);
> -
>        if (debug > 1)
>                netdev_dbg(dev, "Shutting down ethercard, status was %04x\n",
>                           ioread16(ioaddr + ChipCmd));
> @@ -2151,8 +2188,6 @@ static int rhine_close(struct net_device *dev)
>        /* Stop the chip's Tx and Rx processes. */
>        iowrite16(CmdStop, ioaddr + ChipCmd);
>
> -       spin_unlock_irq(&rp->lock);
> -
>        free_irq(rp->pdev->irq, dev);
>        free_rbufs(dev);
>        free_tbufs(dev);
> @@ -2239,10 +2274,12 @@ static int rhine_suspend(struct pci_dev *pdev, pm_message_t state)
>        netif_device_detach(dev);
>        pci_save_state(pdev);
>
> +       /* FIXME: ? */
>        spin_lock_irqsave(&rp->lock, flags);
>        rhine_shutdown(pdev);
>        spin_unlock_irqrestore(&rp->lock, flags);
>
> +       /* FIXME: nuclear workaround ? Check the commit log. */
>        free_irq(dev->irq, dev);
>        return 0;
>  }
> @@ -2257,6 +2294,7 @@ static int rhine_resume(struct pci_dev *pdev)
>        if (!netif_running(dev))
>                return 0;
>
> +       /* FIXME: see above. */
>        if (request_irq(dev->irq, rhine_interrupt, IRQF_SHARED, dev->name, dev))
>                netdev_err(dev, "request_irq failed\n");
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[David Miller]

From: Francois Romieu <romieu@fr.zoreil.com>
Date: Thu, 29 Dec 2011 19:39:02 +0100

> Francois Romieu <romieu@fr.zoreil.com> :
> [...]
>> Feel free to try the extra "new year's eve week end is getting closer"
>> patch below (against net-next).
>> 
>> You have been warned.
> 
> The one below could be a bit better.

Obviously you'll need to resolve the use of rp->lock as an IRQ safe lock
in {get,src}_wol() so that this lock is consistently used as a BH safe
lock.  Your "Huh?" comments make it clear you are aware of this TODO. :-)

But other than that this patch looks good to me, just some testing and
debugging are needed.

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/29 David Miller <davem@davemloft.net>:
> From: Francois Romieu <romieu@fr.zoreil.com>
> Date: Thu, 29 Dec 2011 19:39:02 +0100
>
>> Francois Romieu <romieu@fr.zoreil.com> :
>> [...]
>>> Feel free to try the extra "new year's eve week end is getting closer"
>>> patch below (against net-next).
>>>
>>> You have been warned.
>>
>> The one below could be a bit better.
>
> Obviously you'll need to resolve the use of rp->lock as an IRQ safe lock
> in {get,src}_wol() so that this lock is consistently used as a BH safe
> lock.  Your "Huh?" comments make it clear you are aware of this TODO. :-)
>
> But other than that this patch looks good to me, just some testing and
> debugging are needed.

I got it to boot, had to revert another patch introduced in 3.1.5
that's causing alot of people problems, but thats another talk :)

It boots fine, untill it tries to bring up an interface, which causes
it to fails with a kernel panic.
Here is the output of the panic:
http://dev.gentoo.org/~gurligebis/kernel/panic.txt

/Bjarke

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/30 Bjarke Istrup Pedersen <gurligebis@gentoo.org>:
> 2011/12/29 David Miller <davem@davemloft.net>:
>> From: Francois Romieu <romieu@fr.zoreil.com>
>> Date: Thu, 29 Dec 2011 19:39:02 +0100
>>
>>> Francois Romieu <romieu@fr.zoreil.com> :
>>> [...]
>>>> Feel free to try the extra "new year's eve week end is getting closer"
>>>> patch below (against net-next).
>>>>
>>>> You have been warned.
>>>
>>> The one below could be a bit better.
>>
>> Obviously you'll need to resolve the use of rp->lock as an IRQ safe lock
>> in {get,src}_wol() so that this lock is consistently used as a BH safe
>> lock.  Your "Huh?" comments make it clear you are aware of this TODO. :-)
>>
>> But other than that this patch looks good to me, just some testing and
>> debugging are needed.
>
> I got it to boot, had to revert another patch introduced in 3.1.5
> that's causing alot of people problems, but thats another talk :)
>
> It boots fine, untill it tries to bring up an interface, which causes
> it to fails with a kernel panic.
> Here is the output of the panic:
> http://dev.gentoo.org/~gurligebis/kernel/panic.txt
>
> /Bjarke

Hmm, it seems like this problem isn't related to the driver, but
related to the current HEAD of net-next :)

Would it be a problem if I try and use the patch on top of a clean
3.1.6 kernel ?

/Bjarke

>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[David Miller]

From: Bjarke Istrup Pedersen <gurligebis@gentoo.org>
Date: Fri, 30 Dec 2011 00:37:17 +0100

> Hmm, it seems like this problem isn't related to the driver, but
> related to the current HEAD of net-next :)

Please try this patch.

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 30de9e7..4a62c47 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -129,11 +129,14 @@ static struct neighbour *ip6_neigh_lookup(const struct dst_entry *dst, const voi
 	return neigh_create(&nd_tbl, daddr, dst->dev);
 }
 
-static int rt6_bind_neighbour(struct rt6_info *rt)
+static int rt6_bind_neighbour(struct rt6_info *rt, struct net_device *dev)
 {
-	struct neighbour *n = ip6_neigh_lookup(&rt->dst, &rt->rt6i_gateway);
-	if (IS_ERR(n))
-		return PTR_ERR(n);
+	struct neighbour *n = __ipv6_neigh_lookup(&nd_tbl, dev, &rt->rt6i_gateway);
+	if (!n) {
+		n = neigh_create(&nd_tbl, &rt->rt6i_gateway, dev);
+		if (IS_ERR(n))
+			return PTR_ERR(n);
+	}
 	dst_set_neighbour(&rt->dst, n);
 
 	return 0;
@@ -746,7 +749,7 @@ static struct rt6_info *rt6_alloc_cow(const struct rt6_info *ort,
 #endif
 
 	retry:
-		if (rt6_bind_neighbour(rt)) {
+		if (rt6_bind_neighbour(rt, rt->dst.dev)) {
 			struct net *net = dev_net(rt->dst.dev);
 			int saved_rt_min_interval =
 				net->ipv6.sysctl.ip6_rt_gc_min_interval;
@@ -1397,7 +1400,7 @@ int ip6_route_add(struct fib6_config *cfg)
 		rt->rt6i_prefsrc.plen = 0;
 
 	if (cfg->fc_flags & (RTF_GATEWAY | RTF_NONEXTHOP)) {
-		err = rt6_bind_neighbour(rt);
+		err = rt6_bind_neighbour(rt, dev);
 		if (err)
 			goto out;
 	}
@@ -2084,7 +2087,7 @@ struct rt6_info *addrconf_dst_alloc(struct inet6_dev *idev,
 		rt->rt6i_flags |= RTF_ANYCAST;
 	else
 		rt->rt6i_flags |= RTF_LOCAL;
-	err = rt6_bind_neighbour(rt);
+	err = rt6_bind_neighbour(rt, rt->dst.dev);
 	if (err) {
 		dst_free(&rt->dst);
 		return ERR_PTR(err);

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/30 David Miller <davem@davemloft.net>:
> From: Bjarke Istrup Pedersen <gurligebis@gentoo.org>
> Date: Fri, 30 Dec 2011 00:37:17 +0100
>
>> Hmm, it seems like this problem isn't related to the driver, but
>> related to the current HEAD of net-next :)
>
> Please try this patch.
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 30de9e7..4a62c47 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -129,11 +129,14 @@ static struct neighbour *ip6_neigh_lookup(const struct dst_entry *dst, const voi
>        return neigh_create(&nd_tbl, daddr, dst->dev);
>  }
>
> -static int rt6_bind_neighbour(struct rt6_info *rt)
> +static int rt6_bind_neighbour(struct rt6_info *rt, struct net_device *dev)
>  {
> -       struct neighbour *n = ip6_neigh_lookup(&rt->dst, &rt->rt6i_gateway);
> -       if (IS_ERR(n))
> -               return PTR_ERR(n);
> +       struct neighbour *n = __ipv6_neigh_lookup(&nd_tbl, dev, &rt->rt6i_gateway);
> +       if (!n) {
> +               n = neigh_create(&nd_tbl, &rt->rt6i_gateway, dev);
> +               if (IS_ERR(n))
> +                       return PTR_ERR(n);
> +       }
>        dst_set_neighbour(&rt->dst, n);
>
>        return 0;
> @@ -746,7 +749,7 @@ static struct rt6_info *rt6_alloc_cow(const struct rt6_info *ort,
>  #endif
>
>        retry:
> -               if (rt6_bind_neighbour(rt)) {
> +               if (rt6_bind_neighbour(rt, rt->dst.dev)) {
>                        struct net *net = dev_net(rt->dst.dev);
>                        int saved_rt_min_interval =
>                                net->ipv6.sysctl.ip6_rt_gc_min_interval;
> @@ -1397,7 +1400,7 @@ int ip6_route_add(struct fib6_config *cfg)
>                rt->rt6i_prefsrc.plen = 0;
>
>        if (cfg->fc_flags & (RTF_GATEWAY | RTF_NONEXTHOP)) {
> -               err = rt6_bind_neighbour(rt);
> +               err = rt6_bind_neighbour(rt, dev);
>                if (err)
>                        goto out;
>        }
> @@ -2084,7 +2087,7 @@ struct rt6_info *addrconf_dst_alloc(struct inet6_dev *idev,
>                rt->rt6i_flags |= RTF_ANYCAST;
>        else
>                rt->rt6i_flags |= RTF_LOCAL;
> -       err = rt6_bind_neighbour(rt);
> +       err = rt6_bind_neighbour(rt, rt->dst.dev);
>        if (err) {
>                dst_free(&rt->dst);
>                return ERR_PTR(err);

Hey David,

That patch works, now it boots fine :)
I tried the driver on the 3.1.6 kernel to make sure nothing not tested
is causing problems.

/Bjarke

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Francois Romieu]

Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
[...]
> Would it be a problem if I try and use the patch on top of a clean
> 3.1.6 kernel ?

You will have to "cd drivers/net && patch -p5 ..." as the paths are
different. Avoid suspend/resume/runtime PM.

-- 
Ueimor

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/30 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Would it be a problem if I try and use the patch on top of a clean
>> 3.1.6 kernel ?
>
> You will have to "cd drivers/net && patch -p5 ..." as the paths are
> different. Avoid suspend/resume/runtime PM.

The driver seems to be working fine on 3.1.6 - tried VPN'ing into my
network using my phone, and tried copying some files, which seems to
be working.
Not sure if it will be a problem when there is more bandwidth, but so
far it looks alot better.

The log is flodded with this debug info, in case you need it:

Dec 30 01:19:27 woodpecker kernel: via-rhine 0000:00:08.0: lan0:
Interrupt, status 00000002
Dec 30 01:19:27 woodpecker kernel: via-rhine 0000:00:06.0: wan0:
Interrupt, status 00000001
Dec 30 01:19:27 woodpecker kernel: via-rhine 0000:00:06.0: wan0:
Interrupt, status 00000001
Dec 30 01:19:27 woodpecker kernel: via-rhine 0000:00:08.0: lan0:
Interrupt, status 00000002
Dec 30 01:19:27 woodpecker kernel: via-rhine 0000:00:08.0: lan0:
Interrupt, status 00000002
Dec 30 01:19:27 woodpecker kernel: via-rhine 0000:00:08.0: lan0:
Interrupt, status 00000001
Dec 30 01:19:27 woodpecker kernel: via-rhine 0000:00:06.0: wan0:
Interrupt, status 00000002

Looking forward to seeing this in mainline, without the log spamming :)

/Bjarke

> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/30 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Would it be a problem if I try and use the patch on top of a clean
>> 3.1.6 kernel ?
>
> You will have to "cd drivers/net && patch -p5 ..." as the paths are
> different. Avoid suspend/resume/runtime PM.

Sorry for asking a probably obvious question, but whats next to get it
fixed and ready for inclusion?

/Bjarke

> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Francois Romieu]

Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
[...]
> Sorry for asking a probably obvious question, but whats next to get it
> fixed and ready for inclusion?

It should be there for testing and deeper review within a pair of hours.
I ended up vandalizing most of the locking. From a testing viewpoint it
means some amount of work (not mine :o) ) to get a decent coverage.

Once it is known tested and reviewed - it may need a few iterations - it
can be formally submitted for inclusion.

-- 
Ueimor

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/30 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Sorry for asking a probably obvious question, but whats next to get it
>> fixed and ready for inclusion?
>
> It should be there for testing and deeper review within a pair of hours.
> I ended up vandalizing most of the locking. From a testing viewpoint it
> means some amount of work (not mine :o) ) to get a decent coverage.

Great, let me know when there is more to test :)

/Bjarke

> Once it is known tested and reviewed - it may need a few iterations - it
> can be formally submitted for inclusion.
>
> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Francois Romieu]

Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
[...]
> Great, let me know when there is more to test :)

It's done. I have removed linux-kernel from the Cc: where patches are sent
since there is netdev.

If you don't crash trivially, feel free to :
1. loop ip link up / down + module removal / insertion under traffic
2. loop ethtool / vlan commands under traffic
3. do the same as above and perform suspend / resume

-- 
Ueimor

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/30 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Great, let me know when there is more to test :)
>
> It's done. I have removed linux-kernel from the Cc: where patches are sent
> since there is netdev.
>
> If you don't crash trivially, feel free to :
> 1. loop ip link up / down + module removal / insertion under traffic
> 2. loop ethtool / vlan commands under traffic
> 3. do the same as above and perform suspend / resume

I can try and do some ip link up and down. - the kernel is built
without module support, to make it as compact as possible.
I can also try ethtool and see if that breaks anything.
It's a headless router, so it doesn't have support for suspend and resume :)

/Bjarke

> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/30 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Great, let me know when there is more to test :)
>
> It's done. I have removed linux-kernel from the Cc: where patches are sent
> since there is netdev.
>
> If you don't crash trivially, feel free to :
> 1. loop ip link up / down + module removal / insertion under traffic
> 2. loop ethtool / vlan commands under traffic
> 3. do the same as above and perform suspend / resume

Hey,

Going to test it later tonight.
Just applied it to HEAD of net-next, and got the following compiler warning:
drivers/net/ethernet/via/via-rhine.c:2182:13: warning:
'rhine_task_enable' defined but not used

Just so you know that too :)

/Bjarke

> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Francois Romieu]

Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
[...]
> Just applied it to HEAD of net-next, and got the following compiler warning:
> drivers/net/ethernet/via/via-rhine.c:2182:13: warning:
> 'rhine_task_enable' defined but not used

You are running with CONFIG_PM disabled and a down / up loop will not
work. The patch below should hide the latter, at least as long as the
relevant workqueue does not stall for too long... Extra cancel_work in
task_enable could work too but it will look strange.

> Just so you know that too :)

Thanks for testing.

diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
index c57e1da..89ced1b 100644
--- a/drivers/net/ethernet/via/via-rhine.c
+++ b/drivers/net/ethernet/via/via-rhine.c
@@ -1536,6 +1536,7 @@ static int rhine_open(struct net_device *dev)
 	alloc_rbufs(dev);
 	alloc_tbufs(dev);
 	rhine_chip_reset(dev);
+	rhine_task_enable(rp);
 	init_registers(dev);
 	if (debug > 2)
 		netdev_dbg(dev, "%s() Done - status %04x MII status: %04x\n",
-- 
Ueimor

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/30 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Just applied it to HEAD of net-next, and got the following compiler warning:
>> drivers/net/ethernet/via/via-rhine.c:2182:13: warning:
>> 'rhine_task_enable' defined but not used
>
> You are running with CONFIG_PM disabled and a down / up loop will not
> work. The patch below should hide the latter, at least as long as the
> relevant workqueue does not stall for too long... Extra cancel_work in
> task_enable could work too but it will look strange.

Applied the patch, testing later :)
You are saying that with CONFIG_PM disabled, I wont be able to bring
the interface down and up again?

>> Just so you know that too :)
>
> Thanks for testing.
>
> diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
> index c57e1da..89ced1b 100644
> --- a/drivers/net/ethernet/via/via-rhine.c
> +++ b/drivers/net/ethernet/via/via-rhine.c
> @@ -1536,6 +1536,7 @@ static int rhine_open(struct net_device *dev)
>        alloc_rbufs(dev);
>        alloc_tbufs(dev);
>        rhine_chip_reset(dev);
> +       rhine_task_enable(rp);
>        init_registers(dev);
>        if (debug > 2)
>                netdev_dbg(dev, "%s() Done - status %04x MII status: %04x\n",
> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Francois Romieu]

Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
[...]
> Applied the patch, testing later :)
> You are saying that with CONFIG_PM disabled, I wont be able to bring
> the interface down and up again?

No. You would not see the message with CONFIG_PM but the down / up problem
would still be there. The patch should fix the down / up problem.

/away

-- 
Ueimor

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/30 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Just applied it to HEAD of net-next, and got the following compiler warning:
>> drivers/net/ethernet/via/via-rhine.c:2182:13: warning:
>> 'rhine_task_enable' defined but not used
>
> You are running with CONFIG_PM disabled and a down / up loop will not
> work. The patch below should hide the latter, at least as long as the
> relevant workqueue does not stall for too long... Extra cancel_work in
> task_enable could work too but it will look strange.
>
>> Just so you know that too :)

Does not build with patch:

drivers/net/ethernet/via/via-rhine.c: In function 'rhine_open':
drivers/net/ethernet/via/via-rhine.c:1539:2: error: implicit
declaration of function 'rhine_task_enable'
drivers/net/ethernet/via/via-rhine.c: At top level:
drivers/net/ethernet/via/via-rhine.c:2183:13: warning: conflicting
types for 'rhine_task_enable'
drivers/net/ethernet/via/via-rhine.c:2183:13: error: static
declaration of 'rhine_task_enable' follows non-static declaration
drivers/net/ethernet/via/via-rhine.c:1539:2: note: previous implicit
declaration of 'rhine_task_enable' was here
drivers/net/ethernet/via/via-rhine.c:2183:13: warning:
'rhine_task_enable' defined but not used

Wouldn't it be an idea to move the declarations to a via-rhine.h file,
and include it, to work around it, and seperate it like it should be?

/Bjarke

> Thanks for testing.
>
> diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
> index c57e1da..89ced1b 100644
> --- a/drivers/net/ethernet/via/via-rhine.c
> +++ b/drivers/net/ethernet/via/via-rhine.c
> @@ -1536,6 +1536,7 @@ static int rhine_open(struct net_device *dev)
>        alloc_rbufs(dev);
>        alloc_tbufs(dev);
>        rhine_chip_reset(dev);
> +       rhine_task_enable(rp);
>        init_registers(dev);
>        if (debug > 2)
>                netdev_dbg(dev, "%s() Done - status %04x MII status: %04x\n",
> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Francois Romieu]

Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
[...]
> Does not build with patch:

Neither did it here. My head is already elsewhere.

[...]
> Wouldn't it be an idea to move the declarations to a via-rhine.h file,
> and include it, to work around it, and seperate it like it should be?

Code could be correctly ordered too. Forward declare it. It's good enough
in the meantime.

-- 
Ueimor

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/30 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Does not build with patch:
>
> Neither did it here. My head is already elsewhere.
>
> [...]
>> Wouldn't it be an idea to move the declarations to a via-rhine.h file,
>> and include it, to work around it, and seperate it like it should be?
>
> Code could be correctly ordered too. Forward declare it. It's good enough
> in the meantime.

Forward declaring it makes it compile just fine.

I'll try and do some stress test of it when the rest of the house goes
to sleep, so I don't cut off the net :)

/Bjarke

> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/30 Bjarke Istrup Pedersen <gurligebis@gentoo.org>:
> 2011/12/30 Francois Romieu <romieu@fr.zoreil.com>:
>> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
>> [...]
>>> Does not build with patch:
>>
>> Neither did it here. My head is already elsewhere.
>>
>> [...]
>>> Wouldn't it be an idea to move the declarations to a via-rhine.h file,
>>> and include it, to work around it, and seperate it like it should be?
>>
>> Code could be correctly ordered too. Forward declare it. It's good enough
>> in the meantime.
>
> Forward declaring it makes it compile just fine.
>
> I'll try and do some stress test of it when the rest of the house goes
> to sleep, so I don't cut off the net :)
>
> /Bjarke

Okay, this is beginning to look great.
Just tried randomly up'ing/down'ing the interfaces on the router while
there was alot of trafic going between them (a client downloading a
knoppix live dvd using bittorrent) - survived without a single bump.
Also tried connect a machine to one of the ports, and copying a large
file across (something that before could make it lock up within 15
secords) - also worked without any problems - CPU usage around 15%
(mostly "top" using that), so thats not bad at all.

Is there any other testcases that I should try out?

/Bjarke

>> --
>> Ueimor
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Francois Romieu]

Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
[...]
> Also tried connect a machine to one of the ports, and copying a large
> file across (something that before could make it lock up within 15
> secords) - also worked without any problems - CPU usage around 15%
> (mostly "top" using that), so thats not bad at all.
> 
> Is there any other testcases that I should try out?

Same thing + random ethtool link management commands.
Same thing + random cable unplug/plug.
Same thing + pktgen facing the lan interface.

Everything at the same time.

-- 
Ueimor

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/31 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Also tried connect a machine to one of the ports, and copying a large
>> file across (something that before could make it lock up within 15
>> secords) - also worked without any problems - CPU usage around 15%
>> (mostly "top" using that), so thats not bad at all.
>>
>> Is there any other testcases that I should try out?
>
> Same thing + random ethtool link management commands.
> Same thing + random cable unplug/plug.
> Same thing + pktgen facing the lan interface.
>
> Everything at the same time.

I tried messing around with plugging cables and playing with ethtool
at the same time, while there was alot of trafic, which went fine :)
Happy new year btw :)

/Bjarke

> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/31 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Also tried connect a machine to one of the ports, and copying a large
>> file across (something that before could make it lock up within 15
>> secords) - also worked without any problems - CPU usage around 15%
>> (mostly "top" using that), so thats not bad at all.
>>
>> Is there any other testcases that I should try out?
>
> Same thing + random ethtool link management commands.
> Same thing + random cable unplug/plug.
> Same thing + pktgen facing the lan interface.
>
> Everything at the same time.

Btw. - I have split up the via-rhine.c file (with all the changes you
have sent me) into via-rhine.c and via-rhine.h , like it should be :-)
I'll send it shortly after this mail - could you incorporate it into your work?

/Bjarke

> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2011/12/31 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Also tried connect a machine to one of the ports, and copying a large
>> file across (something that before could make it lock up within 15
>> secords) - also worked without any problems - CPU usage around 15%
>> (mostly "top" using that), so thats not bad at all.
>>
>> Is there any other testcases that I should try out?
>
> Same thing + random ethtool link management commands.
> Same thing + random cable unplug/plug.
> Same thing + pktgen facing the lan interface.
>
> Everything at the same time.

Hey,

Is there anything else I should try out?
Also, would it be possible to push these changes to mainline as
another CONFIG option (not sure if thats possible), so more people can
enable it and test it? :)

/Bjarke

> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Francois Romieu]

Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
[...]
> Is there anything else I should try out?

Your computer is supposed to be moderately underpowered. Find someone
with an heavily overpowered one.

> Also, would it be possible to push these changes to mainline as

Probably. There has not been much objection nor comments so far.

> another CONFIG option (not sure if thats possible), so more people can
> enable it and test it? :)

I'd rather not. It would not be terribly elegant and more CONFIG options
would not imply more testing.

-- 
Ueimor

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2012/1/3 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Is there anything else I should try out?
>
> Your computer is supposed to be moderately underpowered. Find someone
> with an heavily overpowered one.

Hmm, thats gonna be difficult, since it's an on-board chip in the box,
so it's no possible to move it to something more powerful :)

>> Also, would it be possible to push these changes to mainline as
>
> Probably. There has not been much objection nor comments so far.

Nice, any objections David? :)

>> another CONFIG option (not sure if thats possible), so more people can
>> enable it and test it? :)
>
> I'd rather not. It would not be terribly elegant and more CONFIG options
> would not imply more testing.

I agree, but if we push these changes to mainline directly, there is
no need - it was more as a way of getting it to mainline, while still
keeping the old way around, if it was needed.

/Bjarke

> --
> Ueimor

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2012/1/4 Bjarke Istrup Pedersen <gurligebis@gentoo.org>:
> 2012/1/3 Francois Romieu <romieu@fr.zoreil.com>:
>> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
>> [...]
>>> Is there anything else I should try out?
>>
>> Your computer is supposed to be moderately underpowered. Find someone
>> with an heavily overpowered one.
>
> Hmm, thats gonna be difficult, since it's an on-board chip in the box,
> so it's no possible to move it to something more powerful :)
>
>>> Also, would it be possible to push these changes to mainline as
>>
>> Probably. There has not been much objection nor comments so far.
>
> Nice, any objections David? :)

Sorry for repeating myself David, but would it be possible to get this
into net-next for 3.4 inclusion?
(I'm thinking that now is the right time for it to have a complete
release cycle in net-next before inclusion) :)

/Bjarke

>>> another CONFIG option (not sure if thats possible), so more people can
>>> enable it and test it? :)
>>
>> I'd rather not. It would not be terribly elegant and more CONFIG options
>> would not imply more testing.
>
> I agree, but if we push these changes to mainline directly, there is
> no need - it was more as a way of getting it to mainline, while still
> keeping the old way around, if it was needed.
>
> /Bjarke
>
>> --
>> Ueimor

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Francois Romieu]

Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
[...]
> Sorry for repeating myself David, but would it be possible to get this
> into net-next for 3.4 inclusion ?

You should not expect #net-next for 3.4 to open before the merge window
for 3.3 is closed.

With due consideration for his freedom of will and action, David can not
take the current patchkit because he needs a formal submission of a complete
set where #3 is fixed. Our last friday's exchange does not count as one.

I understand it is important for you, it will be sent, ok ?

For future reference, you can see what is going on here :

http://patchwork.ozlabs.org/project/netdev/list/?state=*&q=via-rhine

/me returns to paperwork...

-- 
Ueimor

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[Bjarke Istrup Pedersen]

2012/1/5 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Sorry for repeating myself David, but would it be possible to get this
>> into net-next for 3.4 inclusion ?
>
> You should not expect #net-next for 3.4 to open before the merge window
> for 3.3 is closed.

Okay, I'm still pretty new to how the kernel development model works,
and what opens when and for how long, but hey, you learn something
everyday :)

> With due consideration for his freedom of will and action, David can not
> take the current patchkit because he needs a formal submission of a complete
> set where #3 is fixed. Our last friday's exchange does not count as one.

Sure, it wasn't meant like that (Sorry if it was written that way, can
be difficult from time to time to get it across on paper) :)

> I understand it is important for you, it will be sent, ok ?

Yep :)

/Bjarke

> For future reference, you can see what is going on here :
>
> http://patchwork.ozlabs.org/project/netdev/list/?state=*&q=via-rhine
>
> /me returns to paperwork...
>
> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

[David Miller]

From: Bjarke Istrup Pedersen <gurligebis@gentoo.org>
Date: Thu, 5 Jan 2012 13:01:41 +0100

> Sorry for repeating myself David, but would it be possible to get this
> into net-next for 3.4 inclusion?

Francois will formally submit it to me when he feels it is
appropriate, please be patient.