netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: jamal <hadi@cyberus.ca>
To: "Kok, Auke" <auke-jan.h.kok@intel.com>
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH][E1000E] some cleanups
Date: Wed, 03 Oct 2007 09:18:18 -0400	[thread overview]
Message-ID: <1191417498.4357.23.camel@localhost> (raw)
In-Reply-To: <47028351.7090509@intel.com>

On Tue, 2007-02-10 at 10:43 -0700, Kok, Auke wrote:

> the description of this patch is rather misleading, and the title certainly too.

That was fast - you said weeks, not days;->

> Can you resend this with a bit more elaborate explanation as to why the cb code is
> relevant to use here? Not only do I need to understand this, but others might want
> to as well later on ;)

I am probably repeating something youve seen/know already.
The cleanup is to break up the code so it is functionally more readable
from a perspective of the 4 distinct parts in ->hard_start_xmit():

a) packet formatting (example: vlan, mss, descriptor counting, etc.)
b) chip-specific formatting
c) enqueueing the packet on a DMA ring
d) IO operations to complete packet transmit, tell DMA engine to chew
on, tx completion interrupts, set last tx time, etc.

Each of those steps sitting in different functions accumulates state
that is used in the next steps. cb stores this state because it a
scratchpad the driver owns. You could create some other structure and
pass it around the iteration, but why waste more bytes.

I could stop there with the explanation, but let me go on .. ;->

>From a secondary angle, remember i am pulling these patches out of my
batching work. Thats how we started this discussion ;-> I would like,
once converted the driver to remove LLTX, to do #a without holding the
tx lock. This stands on its own even without batching. Then of course,
once all this is in such good shape it makes it easier to add the
batching code because i could reuse the now functionalized steps.
I hope that provides reasonable and good explanation ;->

cheers,
jamal


  reply	other threads:[~2007-10-03 13:18 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-30 17:41 [PATCH][E1000E] some cleanups jamal
2007-09-30 18:16 ` Kok, Auke
2007-09-30 19:01   ` jamal
2007-09-30 19:23     ` Jeff Garzik
2007-09-30 19:31       ` jamal
2007-10-01  1:59         ` Kok, Auke
2007-10-02 12:25           ` jamal
2007-10-02 17:06             ` Kok, Auke
2007-10-02 17:43 ` Kok, Auke
2007-10-03 13:18   ` jamal [this message]
2007-10-07 16:15     ` jamal
2007-10-08 22:40       ` Kok, Auke
2007-10-09 13:29         ` jamal
2007-10-09 16:02           ` Kok, Auke
2007-10-09 22:18             ` 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=1191417498.4357.23.camel@localhost \
    --to=hadi@cyberus.ca \
    --cc=auke-jan.h.kok@intel.com \
    --cc=netdev@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).