pku.leo@gmail.com wrote on 30/03/2009 11:36:36:
quoted text >
> On Mon, Mar 30, 2009 at 5:21 PM, Joakim Tjernlund
> <Joakim.Tjernlund@transmode.se> wrote:
> >
pku.leo@gmail.com wrote on 30/03/2009 10:34:47:
> >>
> >> On Thu, Mar 26, 2009 at 1:51 AM, Joakim Tjernlund
> >> <Joakim.Tjernlund@transmode.se> wrote:
> >> > Anton Vorontsov <avorontsov@ru.mvista.com> wrote on 25/03/2009
> > 15:25:40:
> >> >> On Wed, Mar 25, 2009 at 02:30:49PM +0100, Joakim Tjernlund wrote:
> >> >> > >>From 1c2f23b1f37f4818c0fd0217b93eb38ab6564840 Mon Sep 17
00:00:00
quoted text > >> > 2001
> >> >> > From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> >> >> > Date: Tue, 24 Mar 2009 10:19:27 +0100
> >> >> > Subject: [PATCH] ucc_geth: Move freeing of TX packets to NAPI
> > context.
> >> >> > Also increase NAPI weight somewhat.
> >> >> > This will make the system alot more responsive while
> >> >> > ping flooding the ucc_geth ethernet interaface.
> >> >>
> >> >> Some time ago I've tried a similar thing for this driver, but
during
quoted text > >> >> 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),
quoted text > >> >> 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:
> >> > if ((bd == ugeth->txBd[txQ]) && (netif_queue_stopped(dev) ==
> > 0))
> >> > break;
> >> >
> >> > Sure does look fishy to me.
> >>
> >> There are two cases when txBd=ConfBd: the BD ring is full or empty.
> >> The condition used here ensures that it is the empty case. Because
in
quoted text > >> 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.
I don't know. But the question you should ask is: Does the networking
code promise this now and for the future? If not, you should
fix the driver not to relay on netif_queue_stopped() here.
Jocke
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to
majordomo@vger.kernel.org
More majordomo info at
http://vger.kernel.org/majordomo-info.html
