All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Ravnborg <sam@ravnborg.org>
To: Adrian Bunk <bunk@kernel.org>
Cc: mingo@elte.hu, linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [2.6 patch] kernel/sched*: optimize inlining
Date: Sun, 11 May 2008 23:44:28 +0200	[thread overview]
Message-ID: <20080511214428.GA1782@uranus.ravnborg.org> (raw)
In-Reply-To: <20080511211902.GI1645@cs181133002.pp.htv.fi>

On Mon, May 12, 2008 at 12:19:02AM +0300, Adrian Bunk wrote:
> On Sun, May 11, 2008 at 10:38:27PM +0200, Sam Ravnborg wrote:
> > > > 
> > > > You continue to fail to acknowledge that it is valueable information
> > > > that we pass gcc a _hint_ that it is a good idea to inline certain
> > > > functions.
> > > > 
> > > > The inline hint is there to tell gcc that it shall inline this function
> > > > in cases where it usual think it should not do so. Which invietably
> > > > will result in a larger codesize in some cases.
> > > 
> > > We also give gcc an explicit "Optimize for size.".
> > 
> > gcc was asked to optimize for size in general as per the commandline option.
> > But on a much more fine grained level gcc is given a hint that
> > this function would be a good idea to inline.
> > 
> > And I really expect gcc to pay most attention to the more specific
> > information provided for a single function than a general commandline option.
> 
> Can you try to get from expectations back to reality?

What I wrote is based on common sense.
Let me know if the gcc community does not agree.

> gcc 4.3 even ignores the unlikely() hint in timespec_add_ns()
> (we now have a workaround for this in the kernel).
I do not follow the logic here.
Gcc may fail in a few cases to do what we expect but that
is far from that we shall assume that it always fails.

> 
> >...
> > > All the "optimized inlining" does is to allow gcc to no longer inline 
> > > functions marked as "inline" if it prefers not to do so.
> > The "optimized inlining" allows gcc (if gcc > 4.0) to make an educated
> > guess if it is worhtwhile to inline a function or not. And when deciding
> > to do so or not gcc may include many different factors inlcuding
> > but not limited to -s.
> > And this is certainly optimized compared to the situation where
> > inline equals to always_inline.
> > Keep in mind that we often perfer to have _less_ inlining than we have
> > today for debugging ease. And this is what we get with optimized inlining
> > compared to farced inlining.
> > 
> > > 
> > > And what exactly is your problem with my patch if you consider the 
> > > general "optimized inlining" approach correct?
> > 
> > I've already listed two things:
> > -> It is no longer a simple kconfig change to try with or without.
> > -> It is independent on gcc version
> 
> I already asked you previously in this thread:

And you fail to comment why both points are not worth considering.

> 
> Do we have any hard data that gcc < 4.0 has a "broken inline algorithm" 
> and all gcc versions >= 4.0 have a "working inline algorithm"?

Is it hard data for you that Linus says that gcc < 4.0 is "broken"
so yes. Search the archives.
If you expect me to show you a lot of disassembly then no.

> 
> > And for fast path code like sched.c I would much assume a proper analysis
> > when it is acceptable to remove the inline hint is almost mandatory.
> >...
> 
> Why didn't you request a proper analysis before the "optimized inlining" 
> stuff hit Linus' tree?
Adrian - stop this bullshit.
We are discussing _your_ patch. Not some other patch that you
seems to have some hard feelings about. And yes I saw the reference
in the initial patch which I saw no reason to comment on as this
was purely bullshit then and still is so.

Was the purpose of this patch just a provocation then?
If so - then I just lost 50% of my Linux time tonight on it!

	Sam

  reply	other threads:[~2008-05-11 21:44 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-11  9:21 [2.6 patch] kernel/sched*: optimize inlining Adrian Bunk
2008-05-11 11:18 ` Andreas Mohr
2008-05-11 12:52   ` Adrian Bunk
2008-05-11 17:22     ` Ray Lee
2008-05-11 17:52 ` Sam Ravnborg
2008-05-11 18:49   ` Adrian Bunk
2008-05-11 19:42     ` Sam Ravnborg
2008-05-11 20:00       ` Adrian Bunk
2008-05-11 20:38         ` Sam Ravnborg
2008-05-11 21:19           ` Adrian Bunk
2008-05-11 21:44             ` Sam Ravnborg [this message]
2008-05-11 22:11               ` Adrian Bunk
2008-05-12  6:57                 ` Sam Ravnborg
2008-05-12  7:45                   ` Adrian Bunk
     [not found] <fa.kBy7Kr77qTR+lDVDv4OKx7n5m1Y@ifi.uio.no>
     [not found] ` <fa.8/KGOoB8CQ+uVBJXmUNHaIfXQSg@ifi.uio.no>
     [not found]   ` <fa.sBJgAJSNbPht40KbEGBXczzW3vQ@ifi.uio.no>
2008-05-11 17:25     ` Robert Hancock

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=20080511214428.GA1782@uranus.ravnborg.org \
    --to=sam@ravnborg.org \
    --cc=akpm@linux-foundation.org \
    --cc=bunk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.