All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Garzik <jgarzik@pobox.com>
To: Ion Badulescu <ionut@badula.org>
Cc: Linus Torvalds <torvalds@transmeta.com>, linux-kernel@vger.kernel.org
Subject: Re: [net drvr] starfire driver update for 2.5.60
Date: Thu, 13 Feb 2003 19:38:34 -0500	[thread overview]
Message-ID: <3E4C3A8A.3030207@pobox.com> (raw)
In-Reply-To: <Pine.LNX.4.44.0302131859550.13539-100000@guppy.limebrokerage.com>

I'm curious about the ring-wrapping code... the comments indicate you 
may not have fully investigated the ring-wrapping semantics?

There are two predominant styles, the "Becker style", which relies on 
proper unsigned integer subtraction even when your buffer-head index is 
numerically less than your buffer-tail, and the "DaveM" style which hide 
a couple masks behind NEXT_TX() style macros.  Either of these work, and 
are quite well thought out and well tested.

Neither style requires any special handling of "wrap" cases, which your 
patch adds..  Your patch adds things like arbitrary padding of 4 tx 
slots, where you might as well add a comment "/* for luck! */".  Why not 
actually nail down the problems the code is obvious working around? 
Such "knobs" may be tweaked enough to be stable in test setups, but 
without actually knowing why you are having problems with Tx start/stop 
at the edge cases, the driver won't begin to approach true stability.

A minor style point too, "s/struct foodesc/foodesc/" is going the 
opposite of preferred.

This is why I have not applied your patch when it was sent to me...

	Jeff




  reply	other threads:[~2003-02-14  0:29 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-02-14  0:13 [net drvr] starfire driver update for 2.5.60 Ion Badulescu
2003-02-14  0:38 ` Jeff Garzik [this message]
2003-02-14  0:40   ` Jeff Garzik
2003-02-14  3:03     ` Ion Badulescu
2003-02-14  6:03       ` Jeff Garzik
2003-02-14  2:27   ` Ion Badulescu

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=3E4C3A8A.3030207@pobox.com \
    --to=jgarzik@pobox.com \
    --cc=ionut@badula.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@transmeta.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.