Re: [PATCH] Do not inline virNumaNodeIsAvailable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 :|

Attachment: pgpxDc7WAuOia.pgp
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]