-
Notifications
You must be signed in to change notification settings - Fork 154
Fix thread-local PooledStackAllocator init order #1019
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,9 +18,11 @@ limitations under the License. | |
| #include <linux/mman.h> | ||
| #endif | ||
| #include <errno.h> | ||
| #include <mutex> | ||
| #include <photon/common/alog.h> | ||
| #include <photon/common/utility.h> | ||
| #include <photon/thread/arch.h> | ||
| #include <photon/thread/thread.h> | ||
| #include <stdlib.h> | ||
| #include <sys/mman.h> | ||
| #include <unistd.h> | ||
|
|
@@ -29,6 +31,8 @@ limitations under the License. | |
|
|
||
| namespace photon { | ||
|
|
||
| static std::once_flag init_once_flag; | ||
|
|
||
| template <size_t MIN_ALLOCATION_SIZE = 4UL * 1024, | ||
| size_t MAX_ALLOCATION_SIZE = 64UL * 1024 * 1024> | ||
| class PooledStackAllocator { | ||
|
|
@@ -169,8 +173,13 @@ size_t pooled_stack_trim_threshold(size_t x) { | |
| return get_pooled_stack_allocator().threshold(x); | ||
| } | ||
|
|
||
| size_t pooled_stack_trim_current_vcpu(size_t keep_size); | ||
| size_t pooled_stack_trim_threshold(size_t x); | ||
| void use_pooled_stack_allocator() { | ||
| get_pooled_stack_allocator(); | ||
| std::call_once(init_once_flag, [] { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since it is a one-way setter and always put same value, is it really need to use a |
||
| photon::set_thread_stack_allocator({&pooled_stack_alloc, nullptr}, | ||
| {&pooled_stack_dealloc, nullptr}); | ||
| }); | ||
| } | ||
|
|
||
| void* default_photon_thread_stack_alloc(void*, size_t stack_size) { | ||
| char* ptr = nullptr; | ||
|
|
@@ -193,4 +202,11 @@ void default_photon_thread_stack_dealloc(void*, void* ptr, size_t size) { | |
| free(ptr); | ||
| } | ||
|
|
||
| void use_default_stack_allocator() { | ||
| std::call_once(init_once_flag, [] { | ||
| photon::set_thread_stack_allocator({&default_photon_thread_stack_alloc, nullptr}, | ||
| {&default_photon_thread_stack_dealloc, nullptr}); | ||
| }); | ||
| } | ||
|
|
||
| } // namespace photon | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -125,10 +125,8 @@ namespace photon | |
| } | ||
| }; | ||
|
|
||
| static Delegate<void*, size_t> photon_thread_alloc( | ||
| &default_photon_thread_stack_alloc, nullptr); | ||
| static Delegate<void, void*, size_t> photon_thread_dealloc( | ||
| &default_photon_thread_stack_dealloc, nullptr); | ||
| static Delegate<void*, size_t> photon_thread_alloc; | ||
| static Delegate<void, void*, size_t> photon_thread_dealloc; | ||
|
|
||
| struct vcpu_t; | ||
| struct thread; | ||
|
|
@@ -2095,6 +2093,10 @@ R"( | |
| } | ||
|
|
||
| int vcpu_init(uint64_t flags) { | ||
| if (unlikely(!photon_thread_alloc || !photon_thread_dealloc)) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only problem is here: that may cause delegate |
||
| // For some test cases that don't use photon::init | ||
| use_default_stack_allocator(); | ||
| } | ||
| uint64_t FLAGS = VCPU_ENABLE_ACTIVE_WORK_STEALING | | ||
| VCPU_ENABLE_PASSIVE_WORK_STEALING; | ||
| if (unlikely(flags & ~FLAGS)) | ||
|
|
@@ -2148,11 +2150,9 @@ R"( | |
| CURRENT->stackful_free(ptr); | ||
| } | ||
|
|
||
| void set_photon_thread_stack_allocator( | ||
| Delegate<void *, size_t> _photon_thread_alloc, | ||
| Delegate<void, void *, size_t> _photon_thread_dealloc) { | ||
| photon_thread_alloc = _photon_thread_alloc; | ||
| photon_thread_dealloc = _photon_thread_dealloc; | ||
| void set_thread_stack_allocator(Delegate<void *, size_t> alloc, Delegate<void, void *, size_t> dealloc) { | ||
| photon_thread_alloc = alloc; | ||
| photon_thread_dealloc = dealloc; | ||
| } | ||
|
|
||
| extern "C" { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
each vcpu will call photon_init, and use_pooled_stack_allocator is a global config.
This change may leads to user set-up pooled-stack-allocator and then unset. (by initialize another vcpu without setting use_pooled_stack_allocator. That is highly risky.