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
prev 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).