From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ingo Oeser <netdev@axxeo.de>
Subject: Re: [PATCH] deinline a few large functions in vlan code v2
Date: Tue, 11 Apr 2006 11:49:24 +0200
Message-ID: <200604111149.24862.netdev@axxeo.de>
References: <200604071628.30486.vda@ilport.com.ua> <1144682807.12177.22.camel@dillow.idleaire.com> <200604111028.54813.vda@ilport.com.ua>
Mime-Version: 1.0
Content-Type: text/plain;
  charset="koi8-r"
Content-Transfer-Encoding: 7bit
Cc: Dave Dillow <dave@thedillows.org>, netdev@vger.kernel.org,
	"David S. Miller" <davem@davemloft.net>,
	linux-kernel@vger.kernel.org, jgarzik@pobox.com
Return-path: <linux-kernel-owner+glk-linux-kernel-3=40m.gmane.org-S1750704AbWDKJtr@vger.kernel.org>
To: Denis Vlasenko <vda@ilport.com.ua>
In-Reply-To: <200604111028.54813.vda@ilport.com.ua>
Content-Disposition: inline
Sender: linux-kernel-owner@vger.kernel.org
List-Id: netdev.vger.kernel.org

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