linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Mahoney <jeffm@suse.de>
To: Andrew Morton <akpm@osdl.org>
Cc: linuxppc-dev@ozlabs.org, reiserfs-dev@namesys.com,
	reiser@namesys.com, Olaf Hering <olh@suse.de>
Subject: Re: [PATCH] make gcc -O1 in fs/reiserfs optional
Date: Thu, 13 Oct 2005 21:17:04 -0400	[thread overview]
Message-ID: <434F0710.50607@suse.de> (raw)
In-Reply-To: <20051013162734.615bba34.akpm@osdl.org>

Andrew Morton wrote:
> Are you sure it's due to inline functions?  I thought the problem was that
> certain versions of gcc did automatic inlining of non-inlined functions and
> we get excessive stack windup due to that.  And iirc we put in global
> compiler options to defeat that behaviour.  Andi would recall.
> 
> Furthermore, we do have infrastructure for detecting the gcc version at
> build time.  It would be better to use that for disabling `-O2', rather
> than a config option.

Andrew -

This "fix" has been in the kernel since I developed the endian safe
patches for ReiserFS in the 2.4 days; This patch just makes it optional
rather than required.

You could be correct regarding the behavior. I noticed it was related to
inline functions and stack usage. Since this was quite a few years ago
now, I don't recall the exact details, just that -O1 worked around it. I
was content blaming gcc and moving on with development. I seem to recall
thinking that was overaggressive aliasing avoidance between temporary
variables in static inline functions. The endian safe code frequently
uses static inlines for conversion of disk order bitfields (like key
type and offset). balance_leaf() is a beast of a function at around 1400
lines or so. When compiled with -O2, it wanted to allocate 6k of stack
space. With -O1, it ended up allocating about 230 bytes. It seemed to be
the only function really affected by the bug, but I wanted to be certain.

I do agree that using the kbuild infrastructure to determine the need
for this option would work quite a bit better.

-Jeff

-- 
Jeff Mahoney
SUSE Labs

  reply	other threads:[~2005-10-14  1:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-04-26 21:10 [PATCH] make gcc -O1 in fs/reiserfs optional Olaf Hering
2005-04-26 21:46 ` Jeff Mahoney
2005-04-27 13:40 ` Hans Reiser
2005-10-11 19:01   ` Olaf Hering
2005-10-11 20:04     ` Hans Reiser
2005-10-13 23:27     ` Andrew Morton
2005-10-14  1:17       ` Jeff Mahoney [this message]
2006-07-24  6:52     ` [PATCH] use gcc -O1 in fs/reiserfs only for ancient gcc versions Olaf Hering

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=434F0710.50607@suse.de \
    --to=jeffm@suse.de \
    --cc=akpm@osdl.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=olh@suse.de \
    --cc=reiser@namesys.com \
    --cc=reiserfs-dev@namesys.com \
    /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).