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

From: Jan Beulich <jbeulich@suse.com>
To: George Dunlap <george.dunlap@citrix.com>
Cc: Juergen Gross <jgross@suse.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>, WeiLiu <wl@xen.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Tim Deegan <tim@xen.org>, Ian Jackson <ian.jackson@citrix.com>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [Xen-devel] [PATCH] xen/typesafe: Force helpers to be always_inline
Date: Mon, 7 Oct 2019 11:25:04 +0200	[thread overview]
Message-ID: <b2d49e5d-3350-d7f6-6bf2-8a2082e8fc80@suse.com> (raw)
In-Reply-To: <b4258bee-ba9d-80af-93f7-db01db3f2d57@citrix.com>

On 04.10.2019 19:02, George Dunlap wrote:
> On 10/2/19 9:11 AM, Jan Beulich wrote:
>> 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.
> 
> What would be the point of this?  The only reason always_inline isn't
> necessary for gcc (if I'm following the argument) is because it so far
> has always inlined these functions.  If it stopped inlining them, we'd
> need to change it to always_inline anyway; so why not just say so to
> begin with?

The point of this would be to _avoid_ using always_inline as much as
possible. We really shouldn't fight compiler decisions more than
absolutely necessary. Hence also my request for sufficiently firm
criteria when to switch in the first place. Or else would could, as
mentioned as an option elsewhere, make inline expand to always_inline
uniformly. (Or course, even always_inline isn't a guarantee for the
compiler to actually inline a function.)

Jan

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