Waldek, I told you about this patch I had a long time ago, and I'm sending
it again (plus three other patches needed as prerequisites to support this
feature not just in the OSv kernel but also in shared objects - which is
important to be able to test this feature - although I see now I forgot to
include the test itself)
--
Nadav Har'El
n...@scylladb.com
On Mon, Dec 23, 2019 at 8:28 PM Nadav Har'El <n...@scylladb.com> wrote:
> From:
>
> This patch adds the feature of *inherited* thread-local variables, namely
> thread-local variables whose value in a new thread is inherited from the
> parent thread's value, instead of being set by the default value.
>
> An *simple* inherited thread-local variable is defined with:
>
> __thread T name;
> INHERITED_THREAD_LOCAL_SIMPLE(name);
>
> Note that as we use __thread, these "simple" inherited thread-local
> variables
> are limited to types T which __thread supports, namely T with a constexpr
> constructor and a trivial destructor. The next patch will add support
> for inherited thread-local variables with arbitrary types, i.e., types with
> general constructors and destructors.
>
> The copying of the value between the parent and child thread is done while
> the thread object is being created (new sched::thread), using T's copy
> assignment operator. Note that it is indeed necessary to do this copy
> before
> the new thread is started, because after it is started we can no longer be
> sure the parent thread still exists, or even if it does, it is not safe
> to access the parent's thread-local variables in parallel to it running.
>
> Theoretically we could use INHERITED_THREAD_LOCAL_SIMPLE() also on a C++11
> "thread_local" variables instead of "__thread", allowing a general types,
> with a slight performance penalty during use. However, currently we have
> a bug in this support, and moreover, OSv does not correctly run
> thread_local destructors (see issue #373), so doing this is not
> recommended.
> Instead use the general inherited thread-local variables offered by the
> next
> patch.
>
> We correctly support inherited thread-local variables in both the OSv core,
> and in shared objects (the latter is especially useful for the tests :-)).
>
> Signed-off-by: "Nadav Har'El" <n...@scylladb.com>
> ---
> core/inherited.cc | 5 +
> core/sched.cc | 5 +
> include/osv/inherited.hh | 97 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 107 insertions(+)
>
> --- .before/core/sched.cc 2019-11-23 00:47:51.372674300 +0200
> +++ .after/core/sched.cc 2019-11-23 00:47:51.373674300 +0200
> @@ -13,6 +13,7 @@
> #include <osv/debug.hh>
> #include <osv/irqlock.hh>
> #include <osv/align.hh>
> +#include <osv/inherited.hh>
>
> #include <osv/interrupt.hh>
> #include <smp.hh>
> @@ -1125,6 +1126,10 @@ thread::thread(std::function<void ()> fu
> _pinned = true;
> }
>
> + if (s_current) {
> + osv::impl::inherited_thread_local_list::run_copy_funcs(this);
> + }
> +
> if (main) {
> _detached_state->_cpu = attr._pinned_cpu;
> _detached_state->st.store(status::running);
> --- .before/include/osv/inherited.hh 2019-11-23 00:47:51.416674274 +0200
> +++ .after/include/osv/inherited.hh 2019-11-23 00:47:51.416674274 +0200
> @@ -0,0 +1,97 @@
> +/*
> + * Copyright (C) 2014 Cloudius Systems, Ltd.
> + *
> + * This work is open source software, licensed under the terms of the
> + * BSD license as described in the LICENSE file in the top-level
> directory.
> + */
> +
> +#ifndef INCLUDED_INHERITED_HH
> +#define INCLUDED_INHERITED_HH
> +
> +#include <vector>
> +
> +#include <osv/sched.hh>
> +#include <osv/elf.hh>
> +
> +// When a variable is defined as
> +//
> +// __thread T name;
> +// INHERITED_THREAD_LOCAL_SIMPLE(name);
> +//
> +// It becomes an inherited thread-local value: As any thread-local
> variable,
> +// "name" has a separate value per thread. But while thread-local
> variables
> +// have their default value on every new thread, *inherited* means here
> that
> +// the value of the variable in a new thread is copied from the thread
> +// creating it.
> +//
> +// The copying of the value between the parent and child thread is done
> +// while the thread object is being created (not when the thread is
> started),
> +// using T's copy assignment operator.
> +//
> +// INHERITED_THREAD_LOCAL_SIMPLE is applied to gcc's __thread, so is
> bound to
> +// its limitations, namely that T must have a constexpr constructor, and
> no
> +// destructor.
> +//
> +// We can also use INHERITED_THREAD_LOCAL_SIMPLE() on a C++11
> +// "thread_local" variable instead of "__thread", allowing a destructor,
> +// but unfortunately this doesn't work now - both because of a bug we have
> +// while running the copy constructor (which apparently doesn't see the
> +// object constructed) and also because another bug OSv has where
> +// thread_local's destructors aren't run.
> +
> +namespace osv {
> +namespace impl {
> +
> +// TODO: The technique we use for making the list of copy functions
> requires
> +// a bit of boot-time work to run these inherited_thread_local_list
> +// constructors. It is both (very little) wasted time, and an annoyance of
> +// initialization order (values will not be inherited before we run these
> +// constructors). Maybe a link-time alternative is possible, in the form
> of a
> +// separate executable section - see what we did in percpu.hh.
> +
> +class inherited_thread_local_list {
> + // FIXME: We need to use a mutex: If a shared-object using inherited
> + // thread-locals variables is loaded or unloaded in parallel with
> + // threads being created or destroyed, run_copy_funcs() or
> + // run_destroy_funcs() may see a vector while it is being updated.
> +public:
> + typedef void (*copy_func)(sched::thread *);
> + inherited_thread_local_list(copy_func f) : _copy_func(f) {
> + _copy_funcs.push_back(f);
> + }
> + static void run_copy_funcs(sched::thread *t) {
> + for (const copy_func &f : _copy_funcs) {
> + f(t);
> + }
> + }
> + // This destructor will only ever get called when the inherited
> thread-
> + // local feature is used in a shared object. We need to remove the
> + // callbacks we registered above.
> + ~inherited_thread_local_list() {
> + // TODO: Make this less ridiculously inefficient :-)
> + _copy_funcs.erase(std::find(_copy_funcs.begin(),
> _copy_funcs.end(), _copy_func));
> + }
> +private:
> + // Save constructor's parameters for the destructor
> + copy_func _copy_func;
> +
> +private:
> + static std::vector<copy_func> _copy_funcs;
> +};
> +
> +
> +
> +
> +} // namespace impl
> +} // namespace osv
> +
> +#define INHERITED_THREAD_LOCAL_SIMPLE(name) \
> + static osv::impl::inherited_thread_local_list _##name##_itl( \
> + [](sched::thread *t) { \
> + elf::object *obj = elf::get_program()->calling_object(); \
> + t->remote_thread_local_var(name, obj) = name; \
> + } \
> + )
> +
> +
> +#endif /* INCLUDED_INHERITED_HH */
> --- .before/core/inherited.cc 2019-11-23 00:47:51.417674273 +0200
> +++ .after/core/inherited.cc 2019-11-23 00:47:51.417674273 +0200
> @@ -0,0 +1,5 @@
> +#include <osv/inherited.hh>
> +
> +// FIXME: consider which construction priority would be appropriate
> +std::vector<osv::impl::inherited_thread_local_list::copy_func>
> + osv::impl::inherited_thread_local_list::_copy_funcs;
>
--
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.
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/CANEVyjvF%3D%2BMYTc-n-2JTvv7g9eDyHGBs%3D9yuueTPjVSOtvH81w%40mail.gmail.com.
