netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: kumarkr@linux.vnet.ibm.com
Cc: hadi@cyberus.ca, herbert@gondor.apana.org.au,
	peter.p.waskiewicz.jr@intel.com, tgraf@suug.ch, kaber@trash.net,
	netdev@vger.kernel.org
Subject: Re: [PATCH 1/2 - rev2] qdisc_restart - readability changes plus one bug fix.
Date: Sun, 24 Jun 2007 19:56:43 -0700 (PDT)	[thread overview]
Message-ID: <20070624.195643.132459975.davem@davemloft.net> (raw)
In-Reply-To: <1182142284.6082.10.camel@localhost.localdomain>

From: Krishna Kumar <kumarkr@linux.vnet.ibm.com>
Date: Mon, 18 Jun 2007 10:21:24 +0530

> New changes :
> 
> - Incorporated Peter Waskiewicz's comments.
> - Re-added back one warning message (on driver returning wrong value).
> 
> Previous changes :
> 
> - Converted to use switch/case code which looks neater.
> 
> - "if (ret == NETDEV_TX_LOCKED && lockless)" is buggy, and the lockless
>   check should be removed, since driver will return NETDEV_TX_LOCKED only
>   if lockless is true and driver has to do the locking. In the original
>   code as well as the latest code, this code can result in a bug where
>   if LLTX is not set for a driver (lockless == 0) but the driver is written
>   wrongly to do a trylock (despite LLTX being set), the driver returns
>   LOCKED. But since lockless is zero, the packet is requeue'd instead of
>   calling collision code which will issue warning and free up the skb.
>   Instead this skb will be retried with this driver next time, and the same
>   result will ensue. Removing this check will catch these driver bugs instead
>   of hiding the problem. I am keeping this change to readability section
>   since :
>   	a. it is confusing to check two things as it is; and
>   	b. it is difficult to keep this check in the changed 'switch' code.
> 
> - Changed some names, like try_get_tx_pkt to dev_dequeue_skb (as that is
>   the work being done and easier to understand) and do_dev_requeue to
>   dev_requeue_skb, merged handle_dev_cpu_collision and tx_islocked to
>   dev_handle_collision (handle_dev_cpu_collision is a small routine with only
>   one caller, so there is no need to have two separate routines which also
>   results in getting rid of two macros, etc.
> 
> - Removed an XXX comment as it should never fail (I suspect this was related
>   to batch skb WIP, Jamal ?). Converted some functions to original coding
>   style of having the return values and the function name on same line, eg
>   prio2list.
> 
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>

Looks good, applied and pushed to net-2.6.23


      parent reply	other threads:[~2007-06-25  2:56 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-06-13  8:40 [PATCH 1/2] qdisc_restart - readability changes, one bug fix Krishna Kumar
2007-06-15 18:01 ` Waskiewicz Jr, Peter P
2007-06-18  4:11   ` Krishna Kumar
2007-06-18  4:51 ` [PATCH 1/2 - rev2] qdisc_restart - readability changes plus " Krishna Kumar
2007-06-18 17:51   ` [PATCH 1/2 - rev2] qdisc_restart - readability changes plus onebug fix Waskiewicz Jr, Peter P
2007-06-19  3:35     ` Krishna Kumar2
2007-06-19  3:58       ` David Miller
2007-06-19  4:00         ` Krishna Kumar2
2007-06-19 16:15         ` jamal
2007-06-25  2:48           ` Krishna Kumar
2007-06-25  2:54             ` David Miller
2007-06-25  2:56   ` David Miller [this message]

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=20070624.195643.132459975.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=hadi@cyberus.ca \
    --cc=herbert@gondor.apana.org.au \
    --cc=kaber@trash.net \
    --cc=kumarkr@linux.vnet.ibm.com \
    --cc=netdev@vger.kernel.org \
    --cc=peter.p.waskiewicz.jr@intel.com \
    --cc=tgraf@suug.ch \
    /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).