On Mon, Mar 30, 2009 at 5:21 PM, Joakim Tjernlund
[off-list ref] wrote:

pku.leo@gmail.com wrote on 30/03/2009 10:34:47:

quoted

On Thu, Mar 26, 2009 at 1:51 AM, Joakim Tjernlund
[off-list ref] wrote:

quoted

Anton Vorontsov [off-list ref] wrote on 25/03/2009

15:25:40:

quoted

quoted

quoted

On Wed, Mar 25, 2009 at 02:30:49PM +0100, Joakim Tjernlund wrote:

quoted

quoted

quoted

From 1c2f23b1f37f4818c0fd0217b93eb38ab6564840 Mon Sep 17 00:00:00

2001

quoted

quoted

From: Joakim Tjernlund <redacted>
Date: Tue, 24 Mar 2009 10:19:27 +0100
Subject: [PATCH] ucc_geth: Move freeing of TX packets to NAPI

context.

quoted

quoted

quoted

quoted

=C2=A0Also increase NAPI weight somewhat.
=C2=A0This will make the system alot more responsive while
=C2=A0ping flooding the ucc_geth ethernet interaface.

Some time ago I've tried a similar thing for this driver, but during
tcp (or udp I don't quite remember) netperf tests I was getting tx
watchdog timeouts after ~2-5 minutes of work. I was testing with a
gigabit and 100 Mbit link, with 100 Mbit link the issue was not
reproducible.

Though, I recalling I was doing a bit more than your patch: I was
also clearing the TX events in the ucce register before calling
ucc_geth_tx, that way I was trying to avoid stale interrupts. That
helped to increase an overall performance (not only responsiveness),
but as I said my approach didn't pass the tests.

I don't really think that your patch may cause this, but can you
try netperf w/ this patch applied anyway? And see if it really
doesn't cause any issues under stress?

Does the line(in ucc_geth_tx()) look OK to you:
=C2=A0 =C2=A0 =C2=A0 =C2=A0if ((bd =3D=3D ugeth->txBd[txQ]) && (netif_=

queue_stopped(dev) =3D=3D

0))

quoted

quoted

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =

=C2=A0 =C2=A0break;

quoted

quoted

Sure does look fishy to me.

There are two cases when txBd=3DConfBd: the BD ring is full or empty.
The condition used here ensures that it is the empty case. =C2=A0Because=

 in

quoted

hard_start_xmit, the queue will be stopped when the BD ring is full.
Maybe some comment is needed here.

But how do you know that the queue hasn't been stopped by someone else
than
the driver?
If it is stopped by higher layers, the if stmt will fail.

It looks like from existing code that only the driver can legally stop
the queue.  I'm not 100% sure though.  Correct me if I'm wrong.

- Leo