[libvirt] [PATCH] Do not inline virNumaNodeIsAvailable

Martin Kletzander mkletzan at redhat.com
Mon Mar 9 13:03:16 UTC 2015


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.

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

>> >>And too large stack frame size when building the tests with -O0.
>> >>
>>
>> I'm using CFLAGS='-O0 -ggdb -Wno-frame-larger-than=' for this as a
>> parameter for autogen.sh O:-)
>>
>> >> src/internal.h     | 10 ++++++++++
>> >> src/util/virnuma.h |  3 ++-
>> >> 2 files changed, 12 insertions(+), 1 deletion(-)
>> >>
>> >>diff --git a/src/internal.h b/src/internal.h
>> >>index 4d473af..84aa330 100644
>> >>--- a/src/internal.h
>> >>+++ b/src/internal.h
>> >>@@ -139,6 +139,16 @@
>> >> #  endif
>> >>
>> >> /**
>> >>+ * ATTRIBUTE_NOINLINE:
>> >>+ *
>> >>+ * Macro to indicate a function that cannot be inlined
>> >>+ * (e.g. a function that is mocked in the test suite)
>> >>+ */
>> >>+#  ifndef ATTRIBUTE_NOINLINE
>> >>+#   define ATTRIBUTE_NOINLINE __attribute__((__noinline__))
>> >>+#  endif
>> >>+
>> >>+/**
>> >>  * ATTRIBUTE_SENTINEL:
>> >>  *
>> >>  * Macro to check for NULL-terminated varargs lists
>> >>diff --git a/src/util/virnuma.h b/src/util/virnuma.h
>> >>index 1f3c0ad..4ddcc5a 100644
>> >>--- a/src/util/virnuma.h
>> >>+++ b/src/util/virnuma.h
>> >>@@ -37,7 +37,8 @@ virBitmapPtr virNumaGetHostNodeset(void);
>> >> bool virNumaNodesetIsAvailable(virBitmapPtr nodeset);
>> >> bool virNumaIsAvailable(void);
>> >> int virNumaGetMaxNode(void);
>> >>-bool virNumaNodeIsAvailable(int node);
>> >>+bool virNumaNodeIsAvailable(int node)
>> >>+    ATTRIBUTE_NOINLINE;
>> >> int virNumaGetDistances(int node,
>> >>                         int **distances,
>> >>                         int *ndistances);
>> >
>> >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 :|
>> >
>> >--
>> >libvir-list mailing list
>> >libvir-list at redhat.com
>> >https://www.redhat.com/mailman/listinfo/libvir-list
>
>
>
>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/20150309/5ec5e435/attachment-0001.sig>


More information about the libvir-list mailing list