From: Nadav Har'El <n...@scylladb.com>
Committer: Nadav Har'El <n...@scylladb.com>
Branch: master
tracepoints: fix build error in debug mode on recent gcc
This patch fixes a link error in debug mode with gcc 8.3.1 (e.g, Fedora 29).
The error lists several cases of tracepoint code which refers to a
"discarded section".
Good practice suggests that tracepoints be defined in source files, not in
header files. The issue is that when a (static) tracepoint is defined in a
header file, multiple copies of it are created, one for each of the source
files which include this header file. This is in fact legal and we support
it, but the separate copies waste memory and discouraged.
Nevertheless, in some rare cases we did decide to have a tracepoint in the
header file - in particular when inline code in that header file needs to
use a tracepoint. But when we do this, the function that uses the
tracepoint *must* be inlined. If the compiler decides not to inline it,
several copies of the function will be compiled for different source
files, and these copies are *not* identical (each refers to a different
copy of the tracepoint variable!) but the linker will assume they are,
and discard all of the copies except one.
So this problem happens when the compiler decides to *not* inline an
inline function. Apparently in gcc 8.3.1's debug mode, not inline is
more common than it used to be, so we started seeing this problem.
The easy workaround is to use __attribute__((always_inline)) to force
inlining of the problematic functions, in any case. Doing this fixes
the debug build.
This workaround still leaves us in violation of the ODR ("one definition
rule"), i.e., multiple translation units define the same method in
different ways (since each writes to a different tracepoint copy).
This violation is what caused this bug in the first place. So a cleaner
solution would be to eliminate the ODR violation. In other words,
do not define tracepoints in header files at all, and rather define
them in a source file and only "extern" them in the header file.
But let's leave that for a future patch.
Fixes #1029.
Signed-off-by: Nadav Har'El <n...@scylladb.com>
---
diff --git a/bsd/sys/netinet/tcp_var.h b/bsd/sys/netinet/tcp_var.h
--- a/bsd/sys/netinet/tcp_var.h
+++ b/bsd/sys/netinet/tcp_var.h
@@ -228,6 +228,7 @@ class tcpcb {
void *t_pspare2[4]; /* 4 TBD */
uint64_t _pad[6]; /* 6 TBD (1-2 CC/RTT?) */
public:
+ __attribute__((always_inline)) // Necessary because of issue #1029
inline void set_state(int state) {
trace_tcp_state(this, t_state, state);
t_state = state;
diff --git a/drivers/virtio-vring.hh b/drivers/virtio-vring.hh
--- a/drivers/virtio-vring.hh
+++ b/drivers/virtio-vring.hh
@@ -21,9 +21,9 @@
#define virtio_w(...) tprintf_w(virtio_tag, __VA_ARGS__)
#define virtio_e(...) tprintf_e(virtio_tag, __VA_ARGS__)
-TRACEPOINT(trace_vring_get_buf_finalize, "vring=%p:
_used_ring_host_head %d",
+static TRACEPOINT(trace_vring_get_buf_finalize, "vring=%p:
_used_ring_host_head %d",
void*, int);
-TRACEPOINT(trace_vring_update_used_event, "vring=%p:
_used_ring_host_head %d",
+static TRACEPOINT(trace_vring_update_used_event, "vring=%p:
_used_ring_host_head %d",
void*, int);
namespace virtio {
@@ -147,6 +147,7 @@ class virtio_driver;
*
* @param update_host if TRUE - update the host as well
*/
+ __attribute__((always_inline)) inline // Necessary because of
issue #1029
void get_buf_finalize(bool update_host = true) {
_used_ring_host_head++;
@@ -157,6 +158,7 @@ class virtio_driver;
}
}
+ __attribute__((always_inline)) inline // Necessary because of
issue #1029
void update_used_event() {
// only let the host know about our used idx in case irq are
enabled
if (_avail->interrupt_on()) {
diff --git a/include/osv/mmu.hh b/include/osv/mmu.hh
--- a/include/osv/mmu.hh
+++ b/include/osv/mmu.hh
@@ -177,6 +177,7 @@ inline bool pte_is_cow(pt_element<0> pte)
static TRACEPOINT(trace_clear_pte, "ptep=%p, cow=%d, pte=%x", void*, bool,
uint64_t);
template<int N>
+__attribute__((always_inline)) // Necessary because of issue #1029
inline pt_element<N> clear_pte(hw_ptep<N> ptep)
{
auto old = ptep.exchange(make_empty_pte<N>());
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
