Re: [Xen-devel] [PATCH] xen/typesafe: Force helpers to be always_inline

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.10.2019 22:59, Andrew Cooper wrote:
> On 01/10/2019 09:38, Jan Beulich wrote:
>> On 30.09.2019 21:16, Andrew Cooper wrote:
>>> Clang in particular has a habit of out-of-lining these and creating multiple
>>> local copies of _mfn() and mfn_x(), etc.  Override this behaviour.
>> Is special casing the typesafe helpers then the right approach? The
>> fundamental idea after all is to let the compiler decide. I certainly
>> agree that not inlining such trivial functions despite the inline
>> keyword looks far from optimal, but if there's such a general issue
>> with clang, shouldn't we make "inline" expand to "always_inline"
>> uniformly?
> 
> Inline handing is a mess.
> 
> We currently define inline to __inline__.  Undoing this results in build
> failures.
> 
> Linux currently defines inline to always_inline and they are desperately
> trying to undo this (mis)behaviour.
> 
> There are a few uses of always_inline for safety purposes (the
> speculative helpers).  Most uses of always_inline look to be workarounds
> for the size-of-asm bug/(mis)feature.
> 
> In an ideal world, we wouldn't need it at all, but I definitely don't
> think that taking the Linux approach is a clever move.  We definitely
> have some static inlines which would better not being inline.

IOW your suggested approach (at least for the foreseeable future) is to
do what you do here and convert inline to always_inline as we see fit?
If so, we should at least settle on some sufficiently firm criteria by
which such a conversion would be justifiable.

Seeing that this is primarily to help clang - did you consider
introducing something like clang_inline, expanding to just inline for
gcc, but always_inline for clang? This would at least provide a
sufficiently easy way to undo this if a better clang-side approach can
be found down the road.

Furthermore, wouldn't the livepatch aspect of this be taken care of
by my plan to prefix filenames (in turn prefixing static symbol names
in our kallsyms) with their (relative) paths? If so, rather than
furthering the mess here, should I see about actually making this
work (addressing a wider range of cases, including gcc compiled code
in a few instances)?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel