Skip to content
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

Fix re-locking hang found in issue 86684 #88539

Merged
merged 1 commit into from
Apr 12, 2024

Conversation

jpeyton52
Copy link
Contributor

This was initially reported here (including stacktraces): https://stackoverflow.com/questions/78183545/does-compiling-imagick-with-openmp-enabled-in-freebsd-13-2-cause-sched-yield

If __kmp_register_library_startup() detects that another instance of the library is present, __kmp_is_address_mapped() is eventually called. which uses kmpc_alloc() to allocate memory. This function calls __kmp_entry_thread() to access the thread-local memory pool, which is a bad idea during initialization. This macro internally calls __kmp_get_global_thread_id_reg() which sets the bootstrap lock at the beginning (before calling __kmp_register_library_startup()).

The fix is to use KMP_INTERNAL_MALLOC()/KMP_INTERNAL_FREE() instead of kmpc_malloc()/kmpc_free(). KMP_INTERNAL_MALLOC and KMP_INTERNAL_FREE do not use any bootstrap locks. They just translate to malloc()/free() and are meant to be used during library initialization before other library-specific allocators have been initialized.

Fixes: #86684

This was initially reported here (including stacktraces):
https://stackoverflow.com/questions/78183545/does-compiling-
imagick-with-openmp-enabled-in-freebsd-13-2-cause-sched-yield

If __kmp_register_library_startup detects that another instance of the
library is present, __kmp_is_address_mapped is eventually called. It
calls __kmp_str_format, which uses kmpc_alloc to allocate memory for
the string. This function calls __kmp_entry_thread to access the
thread-local memory pool, which seems a bad idea during initialization.
This macro internally calls __kmp_get_global_thread_id_reg which set the
bootstrap look at the beginning (before calling
__kmp_register_library_startup).

The fix is to use KMP_INTERNAL_MALLOC/FREE instead of kmpc_malloc/free.
KMP_INTERNAL_MALLOC and KMP_INTERNAL_FREE do not use any bootstrap lock.
They just translate to malloc()/free().

Fixes: llvm#86684
Copy link
Collaborator

@jprotze jprotze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm
Thanks for the quick fix!

@jpeyton52 jpeyton52 merged commit 5300a67 into llvm:main Apr 12, 2024
6 checks passed
@caliguian
Copy link

Thank you!

bazuzi pushed a commit to bazuzi/llvm-project that referenced this pull request Apr 15, 2024
This was initially reported here (including stacktraces):
https://stackoverflow.com/questions/78183545/does-compiling-imagick-with-openmp-enabled-in-freebsd-13-2-cause-sched-yield

If `__kmp_register_library_startup()` detects that another instance of
the library is present, `__kmp_is_address_mapped()` is eventually
called. which uses `kmpc_alloc()` to allocate memory. This function
calls `__kmp_entry_thread()` to access the thread-local memory pool,
which is a bad idea during initialization. This macro internally calls
`__kmp_get_global_thread_id_reg()` which sets the bootstrap lock at the
beginning (before calling `__kmp_register_library_startup()`).

The fix is to use `KMP_INTERNAL_MALLOC()`/`KMP_INTERNAL_FREE()` instead
of `kmpc_malloc()`/`kmpc_free()`. `KMP_INTERNAL_MALLOC` and
`KMP_INTERNAL_FREE` do not use any bootstrap locks. They just translate
to `malloc()`/`free()` and are meant to be used during library
initialization before other library-specific allocators have been
initialized.

Fixes: llvm#86684
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openmp:libomp OpenMP host runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[OpenMP] runtime might try to take the bootstrap lock twice and deadlock
4 participants