netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

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