[libvirt] [PATCH] Do not inline virNumaNodeIsAvailable

Tue Mar 10 12:45:38 UTC 2015

On Tue, Mar 10, 2015 at 12:24:03PM +0000, Daniel P. Berrange wrote:
>On Tue, Mar 10, 2015 at 01:05:22PM +0100, Ján Tomko wrote:
>> On Mon, Mar 09, 2015 at 02:03:16PM +0100, Martin Kletzander wrote:
>> > On Mon, Mar 09, 2015 at 10:09:17AM +0000, Daniel P. Berrange wrote:
>> > >On Mon, Mar 09, 2015 at 11:01:31AM +0100, Martin Kletzander wrote:
>> > >> On Thu, Mar 05, 2015 at 11:09:41AM +0000, Daniel P. Berrange wrote:
>> > >> >On Thu, Mar 05, 2015 at 12:05:52PM +0100, Ján Tomko wrote:
>> > >> >>Explicitly request that virNumaNodeIsAvailable not be inlined.
>> > >> >>This fixes the test suite when building with clang (3.5.1).
>> > >> >
>> > >> >Huh, so clang will inline functions, even if they are exported
>> > >> >in the .so library ?  Is there some clang compiler flag we can
>> > >> >use to stop that ?  I'd only expect it to inline stuff which
>> > >> >was declared static, or whose impl body was in the header file
>> > >> >
>> > >>
>> > >> If I understand it correctly, that means that clang is not
>> > >> "compatible" with gcc.
>> > >>
>> > >> Excerpt from gcc online docs [1]:
>> > >>
>> > >>  When a function is both inline and static, if all calls to the
>> > >>  function are integrated into the caller, and the function's address
>> > >>  is never used, then the function's own assembler code is never
>> > >>  referenced.
>> > >>
>> > >> Excerpt from gcc online docs [1]:
>> > >>
>> > >>  By default, Clang builds C code in GNU C11 mode, so it uses standard
>> > >>  C99 semantics for the inline keyword. These semantics are different
>> > >>  from those in GNU C89 mode, which is the default mode in versions of
>> > >>  GCC prior to 5.0.
>> > >>
>> > >> However further reading of the second documentation and c89 semantics
>> > >> it doesn't say anything about the fact that such function should be
>> > >> inlined.
>> > >
>> > >But we haven't added the 'inline' keyword to this function at
>> > >all - it is just a normal function marked for export in the
>> > >.so file, so I'm puzzelled why it is getting inlined.
>> > >
>> >
>> > Exactly, that's what I'm trying to find out as well.
>> >
>> > >>
>> > >> Anyway, is this clang 3.6 specific?  I don't have this problem when
>> > >> compiling with 3.5.  Nor does this show with gcc -std=gnu11.  I'm
>> > >> getting 3.6 to check whether that's the difference.
>> > >>
>> >
>> > After updating clang and llvm from 3.5 to 3.6, I still don't get this
>> > error.  And I have only 4 (fake) nodes available, so it _is_ rewriting
>> > that function.
>>
>> I'm getting the error with 3.5.1, as I said in the commit message.
>>

I missed that, sorry.

>> These are the failing qemuxml2argvtest cases:
>> 60) QEMU XML-2-ARGV hugepages-pages
>> ... libvirt:  error : internal error: NUMA node 1 is unavailable
>> 63) QEMU XML-2-ARGV hugepages-shared
>> ... libvirt:  error : internal error: NUMA node 1 is unavailable
>> 324) QEMU XML-2-ARGV numatune-memnode
>> ... libvirt:  error : internal error: NUMA node 1 is unavailable
>> 326) QEMU XML-2-ARGV numatune-memnode-no-memory
>> ... libvirt:  error : internal error: NUMA node 3 is unavailable
>> 329) QEMU XML-2-ARGV numatune-auto-prefer
>> ... libvirt:  error : internal error: NUMA node 1 is unavailable
>>
>> So with 4 fake nodes, the tests could still pass even if the function is
>> not mocked. Try changing the nodeset in #326 to 4 if it fails.
>>

I tried changing that, it fails.  I tried adjusting the tests to more
nodes, it fails.  After adjusting the mock function again, it works.
So it gets mocked all the time, but I know where the difference is,
probably.  Try building with -O0, it will probably disable the
inlining.  However, if that's the case, I believe it's still clang's
fault since they don't document inlining functions without the
"inline" keyword just because you optimize.

>> >
>> > >> [1] https://gcc.gnu.org/onlinedocs/gcc/Inline.html
>> > >> [2] http://clang.llvm.org/compatibility.html
>> > >>
>> > >> >>---
>> > >> >>This only leaves the mysterious check-protocol failure.
>> > >>
>> > >> That's not that mysterious, it's just that we check the order and
>> > >> clang sorts enums before structs, but gcc doesn't.  Also clang adds
>> > >> "public:" to structs, so it probably treats it as a C++ or C# structs
>> > >> or something?
>> > >>
>> >
>> > By the way if I compile with clang with -std=gnu11 or -std=gnu99, the
>> > "public:" stuff is gone :)
>> >
>>
>> It is mysterious, because it doesn't fail consistently.

Oh, it does fail all the time for me.

>> It was working for me after I tried it again after
>> 'git clean -fxd', today it failed again (though I don't remember if I
>> ran autogen again).
>
>How exactly are you running the build with clang ? Are you just doing
>this
>
>  CC=clang ./autogen.sh && make && make check
>
>Or is there more to it than that ?
>

I have a script that does "export CC=clang" and then ./autogen.sh and
make (everything).

>Regards,
>Daniel
>--
>|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
>|: http://libvirt.org              -o-             http://virt-manager.org :|
>|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
>|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150310/fbbea7d4/attachment-0001.sig>