On Mon, Oct 16, 2006 at 06:55:43PM -0500, Russell Cattelan wrote:
> On Tue, 2006-10-17 at 09:22 +1000, David Chinner wrote:
> > On Mon, Oct 16, 2006 at 04:34:34PM -0500, Eric Sandeen wrote:
> > > Eric Sandeen wrote:
> > > > Timothy Shimmin wrote:
> > > >> Okay, started looking :-)
> > >
> > > One other thing, based on the bug on osdl today, some of these larger
> > > newly-static functions should probably be marked noinline to keep gcc
> > > from doing things we don't want it to...
> > <grumble>
> > This is not an obvious compiler hint (compared to, say, likely()) as
> > the functions gcc automatically inlines changes according to
> > compiler version, optimisation level and platform. Hence adding
> > noinline notation will be like playing whack-a-mole and I doubt it
> > will be consistently used or maintained moving forward. It's the
> > wrong solution, IMO.
> > I think we should change the definition of STATIC so we don't have
> > to poison the code to work around some stupid compiler behaviour.
> > That is, unless we specifically say "inline" for static functions,
> > we really mean "noinline".
> So you are proposing?
> /* non-debug */
> #define STATIC static inline
> /* debug */
> #define STATIC noinline
Currently STATIC is defined in fs/xfs/support/debug.h as:
#define STATIC static
I'm proposing that gets changed to:
> #define STATIC static noinline
> That doesn't work either ...
> STATIC inline
> static noline inline?
Fix them - inline functions in header files should always be "static
inline". Inline functions in .c files should always be static as
well - if they need to be accessed from different source files then
they need to be in header files. Hence "STATIC inline" is broken
code and should be fixed anyway. Luckily, there are very few of
these to fix and they are all in .c files:
chook 137% grep -rIw "STATIC inline" fs/xfs | wc -l
SGI Australian Software Group