Re: [Xen-devel] LLVM optimizations and .init variables usage

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.11.18 at 19:13, <roger.pau@xxxxxxxxxx> wrote:
> As some of you know might already know by discussions on IRC newer
> versions of Xen compiled with LLVM will trigger a page fault shortly
> after boot.
> 
> This is due to the usage of opt_bootscrub in init_heap_pages.
> opt_bootscrub is defined in the .init section, while init_heap_pages
> is not an init function it reads opt_bootscrub depending on the
> system_state. LLVM generated code will attempt to load opt_bootscrub
> regardless of the value of system_state, under the assumption that
> loading a regular global variable have no side effects.
> 
> There have been several suggestions about how to solve this:
> 
>  - Mark the variable as volatile, or use something like ACCESS_ONCE.
>  - Use the weak attribute.
>  - Don't declare opt_bootscrub as init. This is AFAIK what Linux would
>    do, since init variables can only be used in init functions.

Except that Linux also has __ref, specifically to allow references
otherwise moped about by the section mismatch checker. Uses
like the one you mention (unless something has changed in Linux
relatively recently) would be the typical case of Linux adding __ref
to the function definition, which would make them face the exact
same issue.

> The LLVM bug report is at:
> 
> https://bugs.llvm.org/show_bug.cgi?id=39707 
> 
> I wonder whether we should try to patch this by either using the
> volatile or the weak workaround, but then new usages that trigger
> similar bugs could be introduced again. Maybe Xen should follow suit
> with Linux and limit the usage of init variables to init functions,
> this is likely to be safe (since it's what Linux does), and thus would
> prevent future similar issues.

The already mentioned downside of weak suppressing link time errors
when a symbol is undefined imo makes it the least desirable of the
options. While I dislike abusing volatile for the purpose, it would
presumably be the least intrusive change, as __initdata itself could
be made include it. Although, thinking about it, misplaced __initdata
and alike in definitions is quite common a thing, so we might end up
with volatile getting applied to pointed to types instead of pointer
variables themselves. Hence it wouldn't be entirely transparent a
change either. Plus it would affect all data items in .init.*, not just
uses from non-_init functions.

Which makes me think ACCESS_ONCE() is the way to go, as only
references from non-__init functions to .init.* data need dealing
with. I'm not sure I see a good way of finding all instances, though.
We'll need to pay attention when writing and reviewing new code.

Jan



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