Messages in this thread | | | Date | Tue, 12 May 2015 10:16:36 +0200 | From | Hagen Paul Pfeifer <> | Subject | Re: [PATCH] force inlining of spinlock ops |
| |
* Andrew Morton | 2015-05-11 15:19:13 [-0700]:
>Presumably Hagen didn't see the issue with spinlock functions. I >wonder why not.
I think it is a compiler version thing. Not sure why I didn't see it.
>I suppose we should get both these consolidated into a coherent whole.
+1 (let wait for a moment and delay patch inclusion)
>It's a bit irritating to have to do this: presumably gcc will get fixed >and the huge sprinkling of __always_inline will become less and less >relevant over time and people will have trouble distinguishing "real >__always_inline which was put here for a purpose" from "dopey >__always_inline to work around a short-term gcc glitch". > >__always_inline is one of those things where a usage site should always >be commented, because it's near impossible to work out why someone >chose to use it. Quick, tell me what's happening in include/linux/slab.h. > > >Perhaps we should do > >/* > * Comment goes here. It is very specific about gcc versions. > */ >#define inline_for_broken_gcc __always_inline
yeah, but name it in a compiler independent way. Sometimes we may seen similar misbehaving with clang too. But see my other comments
#define inline_for_broken_cc __always_inline
>and then use inline_for_broken_gcc everywhere. That way, the reason >for the marker is self-explanatory and we can later hunt all these >things down and remvoe them. > >Also, the inline_for_broken_gcc definition can be made dependent on >particular gcc versions, which will allow us to easily keep an eye on >the behaviour of later gcc versions.
Mhh, I am not a big fan of this. I think we maneuver into a unmaintainable area with this approach. We must test, check this for all compiler version, new version, all kinds of compiler flags, etc pp.
Another Idea: we talk roundabout about 50 functions where inlining is mission critical (and correct) but gcc sometimes have trouble to do so. Why not enforce __always_inline there? E.g. annotate these rare function with enforce_inline to highlight that these functions are always inlined. No matter what optimization and what compiler flags:
#define enforce_inline __always_inline
Developers are encouraged to use inline - because then the compiler can decide based on his algorithms/heuristics if a function should be inlined or not. For some really hot & short function the developer can use enforce_inline - but this should be an exception.
Hagen
| |