2007-04-26 09:40:27

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 7/9] wext: reduce inline abuse

This patch removes a bunch of inline abuse from wext. Most functions
that were marked inline are only used once so the compiler will inline
them anyway, others are used multiple times but there's no requirement
for them to be inline since they aren't in any fast paths.

Signed-off-by: Johannes Berg <[email protected]>

---
net/wireless/wext.c | 29 ++++++++++-------------------
1 file changed, 10 insertions(+), 19 deletions(-)

--- net-2.6.22.orig/net/wireless/wext.c 2007-04-24 19:13:50.288804911 +0200
+++ net-2.6.22/net/wireless/wext.c 2007-04-24 19:18:49.198804911 +0200
@@ -440,10 +440,8 @@ static const int event_type_pk_size[] =
/* ---------------------------------------------------------------- */
/*
* Return the driver handler associated with a specific Wireless Extension.
- * Called from various place, so make sure it remains efficient.
*/
-static inline iw_handler get_handler(struct net_device *dev,
- unsigned int cmd)
+static iw_handler get_handler(struct net_device *dev, unsigned int cmd)
{
/* Don't "optimise" the following variable, it will crash */
unsigned int index; /* *MUST* be unsigned */
@@ -470,7 +468,7 @@ static inline iw_handler get_handler(str
/*
* Get statistics out of the driver
*/
-static inline struct iw_statistics *get_wireless_stats(struct net_device *dev)
+static struct iw_statistics *get_wireless_stats(struct net_device *dev)
{
/* New location */
if ((dev->wireless_handlers != NULL) &&
@@ -500,7 +498,7 @@ static inline struct iw_statistics *get_
* netif_running(dev) test. I'm open on that one...
* Hopefully, the driver will remember to do a commit in "open()" ;-)
*/
-static inline int call_commit_handler(struct net_device * dev)
+static int call_commit_handler(struct net_device *dev)
{
if ((netif_running(dev)) &&
(dev->wireless_handlers->standard[0] != NULL)) {
@@ -622,8 +620,8 @@ static int iw_handler_get_private(struct
/*
* Print one entry (line) of /proc/net/wireless
*/
-static __inline__ void wireless_seq_printf_stats(struct seq_file *seq,
- struct net_device *dev)
+static void wireless_seq_printf_stats(struct seq_file *seq,
+ struct net_device *dev)
{
/* Get stats from the driver */
struct iw_statistics *stats = get_wireless_stats(dev);
@@ -892,10 +890,8 @@ static int ioctl_standard_call(struct ne
* a iw_handler but process it in your ioctl handler (i.e. use the
* old driver API).
*/
-static inline int ioctl_private_call(struct net_device * dev,
- struct ifreq * ifr,
- unsigned int cmd,
- iw_handler handler)
+static int ioctl_private_call(struct net_device *dev, struct ifreq *ifr,
+ unsigned int cmd, iw_handler handler)
{
struct iwreq * iwr = (struct iwreq *) ifr;
const struct iw_priv_args * descr = NULL;
@@ -1134,11 +1130,8 @@ static DECLARE_TASKLET(wireless_nlevent_
* current wireless config. Dumping the wireless config is far too
* expensive (for each parameter, the driver need to query the hardware).
*/
-static inline int rtnetlink_fill_iwinfo(struct sk_buff * skb,
- struct net_device * dev,
- int type,
- char * event,
- int event_len)
+static int rtnetlink_fill_iwinfo(struct sk_buff *skb, struct net_device *dev,
+ int type, char *event, int event_len)
{
struct ifinfomsg *r;
struct nlmsghdr *nlh;
@@ -1172,9 +1165,7 @@ rtattr_failure:
* Andrzej Krzysztofowicz mandated that I used a IFLA_XXX field
* within a RTM_NEWLINK event.
*/
-static inline void rtmsg_iwinfo(struct net_device * dev,
- char * event,
- int event_len)
+static void rtmsg_iwinfo(struct net_device *dev, char *event, int event_len)
{
struct sk_buff *skb;
int size = NLMSG_GOODSIZE;

--



2007-04-26 16:50:38

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH 7/9] wext: reduce inline abuse

On Tue, Apr 24, 2007 at 08:07:39PM +0200, Johannes Berg wrote:
> This patch removes a bunch of inline abuse from wext. Most functions
> that were marked inline are only used once so the compiler will inline
> them anyway, others are used multiple times but there's no requirement
> for them to be inline since they aren't in any fast paths.
>
> Signed-off-by: Johannes Berg <[email protected]>

That's clearly not true of all compilers. All gcc versions
before 4.0 need serious help to inline functions used only once. Our
current minimal requirement for the kernel is gcc 3.2, therefore this
code is still useful.
Note that this is a legitimate use of inline (tell the
compiler to inline the function), not an abuse.

Jean

2007-04-26 21:36:54

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 7/9] wext: reduce inline abuse

From: Jean Tourrilhes <[email protected]>
Date: Thu, 26 Apr 2007 10:15:17 -0700

> On Thu, Apr 26, 2007 at 07:03:27PM +0200, Michael Buesch wrote:
> >
> > Sure, other people have different opinions on that, but I think
> > with my approach we get smallest code with good speed.
>
> Try with gcc-3.3 if you don't trust me. Your patch will
> produce bigger and slower code. Thanks.

A broken compiler is not an argument for keeping all of these
bogus inlines around.

It is the compilers job to optimize things correctly, and current
gcc's do.

We've always had this approach to dealing with compiler "issues".

2007-04-26 17:14:12

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 7/9] wext: reduce inline abuse

On Thu, 2007-04-26 at 09:50 -0700, Jean Tourrilhes wrote:

> That's clearly not true of all compilers. All gcc versions
> before 4.0 need serious help to inline functions used only once. Our
> current minimal requirement for the kernel is gcc 3.2, therefore this
> code is still useful.
> Note that this is a legitimate use of inline (tell the
> compiler to inline the function), not an abuse.

No, the abuse is using inline in the first place because it doesn't
matter, none of this has an actual reason to be inlined.

johannes


Attachments:
signature.asc (190.00 B)
This is a digitally signed message part

2007-04-26 17:03:50

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 7/9] wext: reduce inline abuse

On Thursday 26 April 2007 18:50:32 Jean Tourrilhes wrote:
> On Tue, Apr 24, 2007 at 08:07:39PM +0200, Johannes Berg wrote:
> > This patch removes a bunch of inline abuse from wext. Most functions
> > that were marked inline are only used once so the compiler will inline
> > them anyway, others are used multiple times but there's no requirement
> > for them to be inline since they aren't in any fast paths.
> >
> > Signed-off-by: Johannes Berg <[email protected]>
>
> That's clearly not true of all compilers. All gcc versions
> before 4.0 need serious help to inline functions used only once. Our
> current minimal requirement for the kernel is gcc 3.2, therefore this
> code is still useful.
> Note that this is a legitimate use of inline (tell the
> compiler to inline the function), not an abuse.

By my personal definition _every_ use of inline is abuse, if it's not
in an absolute fastpath and applied to a really tiny function.
Sure, other people have different opinions on that, but I think
with my approach we get smallest code with good speed.
In general I try to avoid inline whereever possible.
I think this patch is OK and should go in.

Often it's even desired to have out of line functions in fastpaths.
See spinlocks.

--
Greetings Michael.

2007-04-26 17:15:25

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH 7/9] wext: reduce inline abuse

On Thu, Apr 26, 2007 at 07:03:27PM +0200, Michael Buesch wrote:
>
> Sure, other people have different opinions on that, but I think
> with my approach we get smallest code with good speed.

Try with gcc-3.3 if you don't trust me. Your patch will
produce bigger and slower code. Thanks.

Jean