* Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, 9 Jan 2009, Ingo Molnar wrote:
> >
> > So, should we not remove CONFIG_OPTIMIZE_INLINING, then the correct
> > one would be to mark it __always_inline [__asm_inline is senseless
> > there], or the second patch below that changes the bit parameter to
> > unsigned int.
>
> Well, I certainly don't want to _remove_ the "inline" like your patch
> did. Other gcc versions will care. But I committed the pure "change to
> unsigned" part.
>
> But we should fix the cmpxchg (and perhaps plain xchg too), shouldn't
> we?
>
> That your gcc version gets it right doesn't change the fact that Chris'
> gcc version didn't, and out-of-lined it all. So we'll need some
> __always_inlines there too..
Yeah. I'll dig out an older version of gcc (latest distros are all 4.3.x
based) and run the checks to see which inlines make a difference.
> And no, I don't think it makes any sense to call them "__asm_inline".
> Even when there are asms hidden in between the C statements, what's the
> difference between "always" and "asm"? None, really.
Well, the difference is small, nitpicky and insignificant: the thing is
there are two logically separate categories of __always_inline:
1) the places where __always_inline means that in this universe no sane
compiler ever can end up thinking to move that function out of line.
2) inlining for_correctness_ reasons: things like vreads or certain
paravirt items. Stuff where the kernel actually crashes if we dont
inline. Here if we do not inline we've got a materially crashy kernel.
The original intention of __always_inline was to only cover the second
category above - and thus self-document all the 'correctness inlines'.
This notion has become bitrotten somewhat: we do use __always_inline in a
few other places like the ticket spinlock inlines for non-correctness
reasons. That bitrot happened because we simply have no separate symbol
for the first category.
So hpa suggested __asm_inline (yesterday, well before all the analysis was
conducted) under the assumption that there would be many such annotations
needed and that they would be all about cases where GCC's inliner gets
confused by inline assembly.
This theory turned out to be a red herring today - asm()s do not seem to
confuse latest GCC. (although they certain confuse earlier versions, so
it's still a practical issue, so i agree that we do need to annotate a few
more places.)
In any case, the __asm_inline name - even if it made some marginal sense
originally - is totally moot now, no argument about that.
The naming problem remains though:
- Perhaps we could introduce a name for the first category: __must_inline?
__should_inline? Not because it wouldnt mean 'always', but because it is
'always inline' for another reason than the correctless __always_inline.
- Another possible approach wuld be to rename the second category to
__force_inline. That would signal it rather forcefully that the inlining
there is an absolute correctness issue.
- Or we could go with the status quo and just conflate those two
categories (as it is happening currently) and document the correctness
inlines via in-source comments?
But these are really nuances that pale in comparison to the fundamental
questions that were asked in this thread, about the pure existence of this
feature.
If the optimize-inlining feature looks worthwile and maintainable to
remain upstream then i'd simply like to see the information of these two
categories preserved in a structured way (in 5 years i'm not sure i'd
remember all the paravirt inlining details), and i dont feel too strongly
about the style how we preserve that information.
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
