public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andi Kleen <andi@firstfloor.org>
To: Ingo Molnar <mingo@elte.hu>
Cc: Andi Kleen <andi@firstfloor.org>,
	akpm@linux-foundation.org, x86@kernel.org,
	linux-kernel@vger.kernel.org, "H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH] [3/5] Mark complex bitops.h inlines as __always_inline
Date: Wed, 7 Jan 2009 20:46:14 +0100	[thread overview]
Message-ID: <20090107194613.GL496@one.firstfloor.org> (raw)
In-Reply-To: <20090107132653.GA22229@elte.hu>

Hi Ingo,

> Yes, simple functions should always be inlined, nobody will argue about 
> that. But GCC's inliner will never be fixed (for the kernel domain at 
> least) if we keep working it around and if we keep micro-managing it.

To my knowledge the inliner is already fixed. We're just talking about 
old gccs.

Also to be honest I never quite believed in the basic idea of
"hurt users to get someone else to do something" strategies like
you're advocating here.

> 
> We work around GCC bugs only if it hurts visibly (and generally if it 
> affects default-enabled features/optimizations): if it can be quantified 
> either via stability/robustness problems or there's excesssive size / 
> efficiency problems.

Not inlining bitops qualifies as the later I believe. 

I agree with you if we're talking about random static function
in .c files, but widely used functions in .h files that are 
not expected to change are a little different. The main
reason to not use __always_inline normally is that when people
change the function to contain more code they typically don't 
drop these markers and then it eventually leads to bloat.
But for the bitops style functions I changed here it's not really
expected that they change this way.

Also I don't think adding a few __always_inline for functions which
really need it will hurt maintainability or anything else. In fact
we already have them in a couple of places (e.g. uaccess.h)

> 
> You simply have not made that argument via any numbers here. So i have no 

That's true, I just reacted to Hugh's report. I don't use old
compilers on my own.

-Andi

-- 
ak@linux.intel.com

  reply	other threads:[~2009-01-07 19:32 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-04 23:36 [PATCH] [0/5] Couple of x86 patches for 2.6.29 Andi Kleen
2009-01-04 23:36 ` [PATCH] [1/5] Only scan the root bus in early PCI quirks Andi Kleen
2009-01-04 23:51   ` Yinghai Lu
2009-01-05  1:46     ` Andi Kleen
2009-01-04 23:36 ` [PATCH] [2/5] Allow HPET force enable on ICH10 HPET Andi Kleen
2009-01-04 23:36 ` [PATCH] [3/5] Mark complex bitops.h inlines as __always_inline Andi Kleen
2009-01-06  0:03   ` Andrew Morton
2009-01-06 10:17   ` Ingo Molnar
2009-01-06 14:32     ` Andi Kleen
2009-01-07 13:26       ` Ingo Molnar
2009-01-07 19:46         ` Andi Kleen [this message]
2009-01-06 19:17   ` Hugh Dickins
2009-01-07 13:18     ` Ingo Molnar
2009-01-08  2:16       ` Hugh Dickins
2009-01-08  5:07         ` Andi Kleen
2009-01-08  8:04       ` H. Peter Anvin
2009-01-04 23:36 ` [PATCH] [4/5] Use asm stubs for 32bit sigreturn codes Andi Kleen
2009-01-05  0:08   ` H. Peter Anvin
2009-01-04 23:36 ` [PATCH] [5/5] Avoid theoretical vmalloc fault loop Andi Kleen
2009-01-05 11:47   ` Nick Piggin

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=20090107194613.GL496@one.firstfloor.org \
    --to=andi@firstfloor.org \
    --cc=akpm@linux-foundation.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=x86@kernel.org \
    /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