OSv has various types which have special alignment requirements - e.g., some objects are aligned to cache-line size for performance, some objects are aligned because of processor requirements (e.g., fpu state needs to be 64-byte aligned) or ABI requirements (stacks need to be 16-byte aligned).
While C++11 correctly aligns these object on the stack, when created on the heap with "new", it actually ignores the requested alignment! The reason is that "new" calls "operator new" which cannot specify the desired alignment. We're lucky that in practice missing the alignment didn't matter: In some cases the alignment was just supposed to be a performance optimization that is not mandatory. In other cases like the thread object, because it is large, it anyway allocated entire pages and ends up getting a 4096 byte alignment. But Gcc starting in release 7, warns about using "new" on a type with a non-standard alignment requirement. What we do in this patch is to create a template function aligned_new<T>(...) which can be used instead of new, and uses aligned_alloc() to implement the properly aligned allocation. When we switch to C++17 in the future, its "new" fully supports any alignment (and to do that, it has a version of "operator new" taking an alignment - see http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3396.htm) so we will be able to revert this patch. Signed-off-by: Nadav Har'El <n...@scylladb.com> --- arch/x64/smp.cc | 3 ++- bsd/sys/netinet/tcp_input.cc | 3 ++- core/async.cc | 3 ++- drivers/virtio-net.cc | 2 +- drivers/vmxnet3.cc | 3 ++- include/osv/percpu_xmit.hh | 3 ++- include/osv/sched.hh | 13 ++++++++++++- 7 files changed, 23 insertions(+), 7 deletions(-) diff --git a/arch/x64/smp.cc b/arch/x64/smp.cc index dab78d8d..073ef206 100644 --- a/arch/x64/smp.cc +++ b/arch/x64/smp.cc @@ -21,6 +21,7 @@ extern "C" { #include <osv/barrier.hh> #include <osv/prio.hh> #include "osv/percpu.hh" +#include <osv/aligned_new.hh> extern "C" { void smp_main(void); } @@ -76,7 +77,7 @@ void smp_init() parse_madt(); sched::current_cpu = sched::cpus[0]; for (auto c : sched::cpus) { - c->incoming_wakeups = new sched::cpu::incoming_wakeup_queue[sched::cpus.size()]; + c->incoming_wakeups = aligned_array_new<sched::cpu::incoming_wakeup_queue>(sched::cpus.size()); } smpboot_cr0 = read_cr0(); smpboot_cr4 = read_cr4(); diff --git a/bsd/sys/netinet/tcp_input.cc b/bsd/sys/netinet/tcp_input.cc index 642cfeb3..3726ec40 100644 --- a/bsd/sys/netinet/tcp_input.cc +++ b/bsd/sys/netinet/tcp_input.cc @@ -90,6 +90,7 @@ #include <machine/in_cksum.h> #include <osv/poll.h> #include <osv/net_trace.hh> +#include <osv/aligned_new.hh> TRACEPOINT(trace_tcp_input_ack, "%p: We've got ACK: %u", void*, unsigned int); @@ -3226,7 +3227,7 @@ static ipv4_tcp_conn_id tcp_connection_id(tcpcb* tp) void tcp_setup_net_channel(tcpcb* tp, struct ifnet* intf) { - auto nc = new net_channel([=] (mbuf *m) { tcp_net_channel_packet(tp, m); }); + auto nc = aligned_new<net_channel>([=] (mbuf *m) { tcp_net_channel_packet(tp, m); }); tp->nc = nc; tp->nc_intf = intf; intf->add_net_channel(nc, tcp_connection_id(tp)); diff --git a/core/async.cc b/core/async.cc index 98596125..a592ca27 100644 --- a/core/async.cc +++ b/core/async.cc @@ -18,6 +18,7 @@ #include <boost/intrusive/set.hpp> #include <boost/intrusive/parent_from_member.hpp> #include <osv/timer-set.hh> +#include <osv/aligned_new.hh> namespace async { @@ -247,7 +248,7 @@ private: static PERCPU(async_worker*, _percpu_worker); static sched::cpu::notifier _notifier([] () { - *_percpu_worker = new async_worker(sched::cpu::current()); + *_percpu_worker = aligned_new<async_worker>(sched::cpu::current()); }); static inline async_worker& get_worker() diff --git a/drivers/virtio-net.cc b/drivers/virtio-net.cc index 3d1a82b8..b820c36c 100644 --- a/drivers/virtio-net.cc +++ b/drivers/virtio-net.cc @@ -861,7 +861,7 @@ hw_driver* net::probe(hw_device* dev) if (opt_maxnic && maxnic-- <= 0) { return nullptr; } else { - return new net(*pci_dev); + return aligned_new<net>(*pci_dev); } } } diff --git a/drivers/vmxnet3.cc b/drivers/vmxnet3.cc index 92ea965b..c4608c62 100644 --- a/drivers/vmxnet3.cc +++ b/drivers/vmxnet3.cc @@ -39,6 +39,7 @@ #include <osv/sched.hh> #include <osv/trace.hh> +#include <osv/aligned_new.hh> #include "drivers/clock.hh" #include "drivers/clockevent.hh" @@ -428,7 +429,7 @@ hw_driver* vmxnet3::probe(hw_device* dev) if (opt_maxnic && maxnic-- <= 0) { return nullptr; } else { - return new vmxnet3(*pci_dev); + return aligned_new<vmxnet3>(*pci_dev); } } } diff --git a/include/osv/percpu_xmit.hh b/include/osv/percpu_xmit.hh index 952b9b24..0e7571ca 100644 --- a/include/osv/percpu_xmit.hh +++ b/include/osv/percpu_xmit.hh @@ -14,6 +14,7 @@ #include <osv/clock.hh> #include <osv/migration-lock.hh> +#include <osv/aligned_new.hh> #include <bsd/sys/sys/mbuf.h> @@ -210,7 +211,7 @@ public: std::string worker_name_base(name + "-"); for (auto c : sched::cpus) { - _cpuq.for_cpu(c)->reset(new cpu_queue_type); + _cpuq.for_cpu(c)->reset(aligned_new<cpu_queue_type>()); _all_cpuqs.push_back(_cpuq.for_cpu(c)->get()); _worker.for_cpu(c)->me = diff --git a/include/osv/sched.hh b/include/osv/sched.hh index e9a1af96..dada8f5d 100644 --- a/include/osv/sched.hh +++ b/include/osv/sched.hh @@ -406,7 +406,18 @@ public: // view of the application's page table (see issue #790). template <typename... Args> static thread* make(Args&&... args) { - return new thread(std::forward<Args>(args)...); + // As explained in <osv/aligned_new.hh>, in C++11 we cannot use + // new() on an object which has non-default alignment requirements. + // We cannot use the tool aligned_new<thread> from that header + // because of the private constructor, so need to repeat the trick. + // Note that avoiding new() is is not *really* important because + // sizeof(thread) very large (over 20 KB) and would get a 4096-byte + // alignment anyway, even if we allocated it with normal new. + void *p = aligned_alloc(alignof(thread), sizeof(thread)); + if (!p) { + return nullptr; + } + return new(p) thread(std::forward<Args>(args)...); } private: explicit thread(std::function<void ()> func, attr attributes = attr(), -- 2.13.0 -- 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.
