From: jamal <hadi@cyberus.ca>
To: Michael Chan <mchan@broadcom.com>
Cc: Matt Carlson <mcarlson@broadcom.com>,
Robert Olsson <Robert.Olsson@data.slu.se>,
Evgeniy Polyakov <johnpol@2ka.mipt.ru>,
Krishna Kumar2 <krkumar2@in.ibm.com>,
Gagan Arneja <gaagaan@gmail.com>, netdev <netdev@vger.kernel.org>,
Rick Jones <rick.jones2@hp.com>,
Sridhar Samudrala <sri@us.ibm.com>,
David Miller <davem@davemloft.net>, Jeff Garzik <jeff@garzik.org>
Subject: Re: [WIP][PATCHES] Network xmit batching - tg3 support
Date: Tue, 03 Jul 2007 09:26:20 -0400 [thread overview]
Message-ID: <1183469180.5159.89.camel@localhost> (raw)
In-Reply-To: <1183422081.4947.14.camel@dell>
On Mon, 2007-02-07 at 17:21 -0700, Michael Chan wrote:
[Matt, please include the count in the fix per previous email]
> Only base_flags and mss are needed and these can be determined right
> before sending the frame. So is it better not to store these in the
> skb->cb at all?
long answer:
My goal with storing these values and computing them was to do certain
things that dont require holding the netif_tx_lock within a batch as
well. Evaluating the packet metadata and formating the packet to be
ready for stashing into DMA was one thing i could do outside of holding
the lock easily - and running that in loop of 100 packets amortizes the
instruction cache and allows me (when i get to it) to hold the lock for
a lot less computation.
In the e1000 for example i was able to go as far as computing how many
descriptors are needed per skb and stashing those in the skb->cb; the
way the tg3 is structured makes doing all that needing some big changes.
So to answer your question: It would be nice if i could keep the skb->cb
for now and we can get rid of it later if deemed a nuisance for
batching.
>
> + dcount = tg3_tx_avail(tp);
> if (unlikely(netif_queue_stopped(tp->dev) &&
> - (tg3_tx_avail(tp) > TG3_TX_WAKEUP_THRESH(tp)))) {
> + (dcount > TG3_TX_WAKEUP_THRESH(tp)))) {
> netif_tx_lock(tp->dev);
> + tp->dev->xmit_win = 1;
> if (netif_queue_stopped(tp->dev) &&
> - (tg3_tx_avail(tp) > TG3_TX_WAKEUP_THRESH(tp)))
> + (dcount > TG3_TX_WAKEUP_THRESH(tp))) {
> netif_wake_queue(tp->dev);
> + tp->dev->xmit_win = dcount;
> + }
> netif_tx_unlock(tp->dev);
> }
> }
>
> This is also not right. tg3_tx() runs without netif_tx_lock().
> tg3_tx_avail() can change after you get the netif_tx_lock() and we must
> get the updated value again. If we just rely on dcount, we can call
> wake_queue() when the ring is full, or there may be no wakeup when the
> ring is empty.
You are right, I was trying to be a smart-ass there so i didnt have to
fold the lines (for readability). Can you or Matt restore it to the way
it was originally while keeping the xmit_win update and just send me a
patch?
Thanks a lot Michael and Matt for looking at this and improving it.
Maybe i should switch all my nics to tg3 from now on ;->
cheers,
jamal
next prev parent reply other threads:[~2007-07-03 13:26 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-06-06 13:49 [WIP][PATCHES] Network xmit batching jamal
2007-06-07 6:16 ` Krishna Kumar2
2007-06-07 11:43 ` jamal
2007-06-07 16:13 ` Evgeniy Polyakov
2007-06-07 22:23 ` jamal
2007-06-08 8:38 ` Evgeniy Polyakov
2007-06-08 11:31 ` jamal
2007-06-08 12:09 ` Evgeniy Polyakov
2007-06-08 13:07 ` jamal
2007-06-08 21:02 ` Evgeniy Polyakov
2007-06-08 5:05 ` Krishna Kumar2
2007-06-19 13:21 ` Evgeniy Polyakov
2007-06-19 13:33 ` Evgeniy Polyakov
2007-06-19 14:00 ` Evgeniy Polyakov
2007-06-19 14:09 ` Evgeniy Polyakov
2007-06-19 16:32 ` jamal
2007-06-19 16:44 ` Evgeniy Polyakov
2007-06-19 16:28 ` jamal
2007-06-19 16:35 ` Evgeniy Polyakov
2007-06-19 16:45 ` Evgeniy Polyakov
2007-06-19 17:35 ` Robert Olsson
2007-06-19 17:48 ` jamal
2007-06-19 17:55 ` Evgeniy Polyakov
2007-06-28 0:05 ` [WIP][PATCHES] Network xmit batching - tg3 support jamal
2007-07-02 21:20 ` Matt Carlson
2007-07-03 0:21 ` Michael Chan
2007-07-03 13:26 ` jamal [this message]
2007-07-04 4:19 ` Krishna Kumar2
2007-07-04 13:22 ` jamal
2007-07-03 13:09 ` jamal
2007-07-03 19:31 ` Matt Carlson
2007-07-04 1:59 ` jamal
2007-07-03 21:30 ` David Miller
2007-06-19 22:28 ` [WIP][PATCHES] Network xmit batching David Miller
2007-06-21 15:54 ` FSCKED clock sources WAS(Re: " jamal
2007-06-21 16:08 ` jamal
2007-06-21 16:55 ` Benjamin LaHaise
2007-06-25 16:59 ` jamal
2007-06-25 17:08 ` Benjamin LaHaise
2007-06-25 17:16 ` jamal
2007-06-21 16:45 ` Evgeniy Polyakov
2007-06-25 16:58 ` jamal
2007-06-19 16:24 ` jamal
2007-06-21 21:00 ` Rick Jones
2007-06-22 9:59 ` Evgeniy Polyakov
2007-06-25 17:35 ` Rick Jones
2007-06-07 8:42 ` Krishna Kumar2
2007-06-07 12:16 ` jamal
2007-06-08 5:06 ` Krishna Kumar2
2007-06-08 11:14 ` jamal
2007-06-08 11:31 ` Krishna Kumar2
2007-06-08 11:43 ` jamal
2007-06-08 18:00 ` Rick Jones
2007-06-08 17:27 ` Rick Jones
2007-06-09 0:17 ` jamal
2007-06-09 0:40 ` Rick Jones
2007-06-07 22:42 ` jamal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1183469180.5159.89.camel@localhost \
--to=hadi@cyberus.ca \
--cc=Robert.Olsson@data.slu.se \
--cc=davem@davemloft.net \
--cc=gaagaan@gmail.com \
--cc=jeff@garzik.org \
--cc=johnpol@2ka.mipt.ru \
--cc=krkumar2@in.ibm.com \
--cc=mcarlson@broadcom.com \
--cc=mchan@broadcom.com \
--cc=netdev@vger.kernel.org \
--cc=rick.jones2@hp.com \
--cc=sri@us.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.