From: Ingo Oeser <netdev@axxeo.de>
To: Denis Vlasenko <vda@ilport.com.ua>
Cc: Dave Dillow <dave@thedillows.org>,
netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
linux-kernel@vger.kernel.org, jgarzik@pobox.com
Subject: Re: [PATCH] deinline a few large functions in vlan code v2
Date: Tue, 11 Apr 2006 11:49:24 +0200 [thread overview]
Message-ID: <200604111149.24862.netdev@axxeo.de> (raw)
In-Reply-To: <200604111028.54813.vda@ilport.com.ua>
Hi Denis,
Denis Vlasenko wrote:
> +#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
> if(vlan_tx_tag_present(skb)) {
> first_txd->processFlags |=
> TYPHOON_TX_PF_INSERT_VLAN | TYPHOON_TX_PF_VLAN_PRIORITY;
> @@ -844,6 +849,7 @@ typhoon_start_tx(struct sk_buff *skb, st
> cpu_to_le32(htons(vlan_tx_tag_get(skb)) <<
> TYPHOON_TX_PF_VLAN_TAG_SHIFT);
> }
> +#endif
Wouldn't it be much easier to just do
#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
static inline int vlan_tx_tag_present(...) {
/** get VLAN tag */
}
#else
static inline int vlan_tx_tag_present(...) {return 0;}
#endif
in some header file?
Similiar in typhoon.c:
#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
static inline has_vlan_group(...) {
/* get VLAN group */
}
#else
static inline has_vlan_group(...) {return 0;}
#endif
With this and similiar changes in the drivers,
your patch might be less intrusive and thus more acceptable to maintainers.
Just let the compiler remove the extra code with constant folding and dead
code elemination. The result will be even cleaner code, I think.
What do you think?
Regards
Ingo Oeser
next prev parent reply other threads:[~2006-04-11 9:49 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <200604071628.30486.vda@ilport.com.ua>
[not found] ` <200604101716.58463.vda@ilport.com.ua>
[not found] ` <1144682807.12177.22.camel@dillow.idleaire.com>
2006-04-11 7:28 ` [PATCH] deinline a few large functions in vlan code v2 Denis Vlasenko
2006-04-11 8:02 ` David S. Miller
2006-04-11 9:49 ` Ingo Oeser [this message]
[not found] ` <200604111502.52302.vda@ilport.com.ua>
2006-04-11 13:17 ` Ingo Oeser
2006-04-12 19:32 ` [RFD][PATCH] typhoon and core sample for folding away VLAN stuff (was: Re: [PATCH] deinline a few large functions in vlan code v2) Ingo Oeser
2006-04-12 20:10 ` [RFD][PATCH] typhoon and core sample for folding away VLAN stuff Ben Greear
2006-04-12 20:23 ` Stephen Hemminger
2006-04-12 20:51 ` David S. Miller
2006-04-13 1:24 ` Dave Dillow
2006-04-13 8:38 ` Denis Vlasenko
2006-04-13 15:00 ` Dave Dillow
2006-04-11 13:59 ` [PATCH] deinline a few large functions in vlan code v2 Dave Dillow
2006-04-12 8:55 ` Denis Vlasenko
2006-04-12 17:18 ` Dave Dillow
2006-04-13 6:04 ` Denis Vlasenko
2006-04-13 14:59 ` Dave Dillow
2006-04-13 11:32 ` Ingo Oeser
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=200604111149.24862.netdev@axxeo.de \
--to=netdev@axxeo.de \
--cc=dave@thedillows.org \
--cc=davem@davemloft.net \
--cc=jgarzik@pobox.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=vda@ilport.com.ua \
/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).