From 7dc17553e1af9014fd271f2229e98335478092f0 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Thu, 7 Mar 2024 15:15:49 -0800 Subject: [PATCH 01/26] best effort cpu_id --- source/posix/thread.c | 106 +++++++++++++++++++++++++++--------------- 1 file changed, 68 insertions(+), 38 deletions(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index 0b32bebdd..e93c7d92e 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -245,6 +245,54 @@ int aws_thread_init(struct aws_thread *thread, struct aws_allocator *allocator) return AWS_OP_SUCCESS; } +int s_init_pthread_attr(size_t stack_size, int32_t cpu_id, pthread_attr_t *out_attributes) { + int attr_return = 0; + attr_return = pthread_attr_init(out_attributes); + + if (attr_return) { + return attr_return; + } + + if (stack_size > PTHREAD_STACK_MIN) { + attr_return = pthread_attr_setstacksize(out_attributes, stack_size); + + if (attr_return) { + return attr_return; + } + } + +/* AFAIK you can't set thread affinity on apple platforms, and it doesn't really matter since all memory + * NUMA or not is setup in interleave mode. + * Thread affinity is also not supported on Android systems, and honestly, if you're running android on a NUMA + * configuration, you've got bigger problems. */ +#if AWS_AFFINITY_METHOD == AWS_AFFINITY_METHOD_PTHREAD_ATTR + if (options->cpu_id >= 0) { + AWS_LOGF_INFO( + AWS_LS_COMMON_THREAD, + "id=%p: cpu affinity of cpu_id %d was specified, attempting to honor the value.", + (void *)thread, + options->cpu_id); + + cpu_set_t cpuset; + CPU_ZERO(&cpuset); + CPU_SET((uint32_t)options->cpu_id, &cpuset); + + attr_return = pthread_attr_setaffinity_np(attributes_ptr, sizeof(cpuset), &cpuset); + + if (attr_return) { + AWS_LOGF_ERROR( + AWS_LS_COMMON_THREAD, + "id=%p: pthread_attr_setaffinity_np() failed with %d.", + (void *)thread, + attr_return); + return attr_return; + } + } +#endif /* AWS_AFFINITY_METHOD == AWS_AFFINITY_METHOD_PTHREAD_ATTR */ + + return AWS_OP_SUCCESS; +} + int aws_thread_launch( struct aws_thread *thread, void (*func)(void *arg), @@ -261,50 +309,13 @@ int aws_thread_launch( } if (options) { - attr_return = pthread_attr_init(&attributes); + attr_return = s_init_pthread_attr(options->stack_size, options->cpu_id, &attributes); if (attr_return) { goto cleanup; } attributes_ptr = &attributes; - - if (options->stack_size > PTHREAD_STACK_MIN) { - attr_return = pthread_attr_setstacksize(attributes_ptr, options->stack_size); - - if (attr_return) { - goto cleanup; - } - } - -/* AFAIK you can't set thread affinity on apple platforms, and it doesn't really matter since all memory - * NUMA or not is setup in interleave mode. - * Thread affinity is also not supported on Android systems, and honestly, if you're running android on a NUMA - * configuration, you've got bigger problems. */ -#if AWS_AFFINITY_METHOD == AWS_AFFINITY_METHOD_PTHREAD_ATTR - if (options->cpu_id >= 0) { - AWS_LOGF_INFO( - AWS_LS_COMMON_THREAD, - "id=%p: cpu affinity of cpu_id %d was specified, attempting to honor the value.", - (void *)thread, - options->cpu_id); - - cpu_set_t cpuset; - CPU_ZERO(&cpuset); - CPU_SET((uint32_t)options->cpu_id, &cpuset); - - attr_return = pthread_attr_setaffinity_np(attributes_ptr, sizeof(cpuset), &cpuset); - - if (attr_return) { - AWS_LOGF_ERROR( - AWS_LS_COMMON_THREAD, - "id=%p: pthread_attr_setaffinity_np() failed with %d.", - (void *)thread, - attr_return); - goto cleanup; - } - } -#endif /* AWS_AFFINITY_METHOD == AWS_AFFINITY_METHOD_PTHREAD_ATTR */ } wrapper = aws_mem_calloc(thread->allocator, 1, sizeof(struct thread_wrapper)); @@ -332,6 +343,25 @@ int aws_thread_launch( attr_return = pthread_create(&thread->thread_id, attributes_ptr, thread_fn, (void *)wrapper); + if (attr_return == EINVAL && options->cpu_id >= 0) { + /* try without cpu_id */ + AWS_LOGF_DEBUG( + AWS_LS_COMMON_THREAD, + "id=%p: pthread_create() failed with %d and cpu_id:%d, trying without cpu_id", + (void *)thread, + attr_return, + options->cpu_id); + if (attributes_ptr) { + pthread_attr_destroy(attributes_ptr); + } + attr_return = s_init_pthread_attr(options->stack_size, options->cpu_id, &attributes); + if (attr_return) { + goto cleanup; + } + attributes_ptr = &attributes; + attr_return = pthread_create(&thread->thread_id, attributes_ptr, thread_fn, (void *)wrapper); + } + if (attr_return) { AWS_LOGF_ERROR(AWS_LS_COMMON_THREAD, "id=%p: pthread_create() failed with %d", (void *)thread, attr_return); if (is_managed_thread) { From 2573fd97faa94c2c516f164726fe13c0249b4f26 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Thu, 7 Mar 2024 15:18:32 -0800 Subject: [PATCH 02/26] fix typos --- source/posix/thread.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index e93c7d92e..048d93fc8 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -266,16 +266,16 @@ int s_init_pthread_attr(size_t stack_size, int32_t cpu_id, pthread_attr_t *out_a * Thread affinity is also not supported on Android systems, and honestly, if you're running android on a NUMA * configuration, you've got bigger problems. */ #if AWS_AFFINITY_METHOD == AWS_AFFINITY_METHOD_PTHREAD_ATTR - if (options->cpu_id >= 0) { + if (cpu_id >= 0) { AWS_LOGF_INFO( AWS_LS_COMMON_THREAD, "id=%p: cpu affinity of cpu_id %d was specified, attempting to honor the value.", (void *)thread, - options->cpu_id); + cpu_id); cpu_set_t cpuset; CPU_ZERO(&cpuset); - CPU_SET((uint32_t)options->cpu_id, &cpuset); + CPU_SET((uint32_t)cpu_id, &cpuset); attr_return = pthread_attr_setaffinity_np(attributes_ptr, sizeof(cpuset), &cpuset); @@ -354,6 +354,7 @@ int aws_thread_launch( if (attributes_ptr) { pthread_attr_destroy(attributes_ptr); } + wrapper->membind = false; attr_return = s_init_pthread_attr(options->stack_size, options->cpu_id, &attributes); if (attr_return) { goto cleanup; From 90c57d0f91ecc77fa5e6552fe4049d2facb5c8c3 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Thu, 7 Mar 2024 15:30:13 -0800 Subject: [PATCH 03/26] fix again --- source/posix/thread.c | 31 ++++++------------------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index 048d93fc8..e517278ed 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -266,18 +266,18 @@ int s_init_pthread_attr(size_t stack_size, int32_t cpu_id, pthread_attr_t *out_a * Thread affinity is also not supported on Android systems, and honestly, if you're running android on a NUMA * configuration, you've got bigger problems. */ #if AWS_AFFINITY_METHOD == AWS_AFFINITY_METHOD_PTHREAD_ATTR - if (cpu_id >= 0) { + if (options->cpu_id >= 0) { AWS_LOGF_INFO( AWS_LS_COMMON_THREAD, "id=%p: cpu affinity of cpu_id %d was specified, attempting to honor the value.", (void *)thread, - cpu_id); + options->cpu_id); cpu_set_t cpuset; CPU_ZERO(&cpuset); - CPU_SET((uint32_t)cpu_id, &cpuset); + CPU_SET((uint32_t)options->cpu_id, &cpuset); - attr_return = pthread_attr_setaffinity_np(attributes_ptr, sizeof(cpuset), &cpuset); + attr_return = pthread_attr_setaffinity_np(out_attributes, sizeof(cpuset), &cpuset); if (attr_return) { AWS_LOGF_ERROR( @@ -343,28 +343,9 @@ int aws_thread_launch( attr_return = pthread_create(&thread->thread_id, attributes_ptr, thread_fn, (void *)wrapper); - if (attr_return == EINVAL && options->cpu_id >= 0) { - /* try without cpu_id */ - AWS_LOGF_DEBUG( - AWS_LS_COMMON_THREAD, - "id=%p: pthread_create() failed with %d and cpu_id:%d, trying without cpu_id", - (void *)thread, - attr_return, - options->cpu_id); - if (attributes_ptr) { - pthread_attr_destroy(attributes_ptr); - } - wrapper->membind = false; - attr_return = s_init_pthread_attr(options->stack_size, options->cpu_id, &attributes); - if (attr_return) { - goto cleanup; - } - attributes_ptr = &attributes; - attr_return = pthread_create(&thread->thread_id, attributes_ptr, thread_fn, (void *)wrapper); - } - if (attr_return) { - AWS_LOGF_ERROR(AWS_LS_COMMON_THREAD, "id=%p: pthread_create() failed with %d", (void *)thread, attr_return); + if (att) + AWS_LOGF_ERROR(AWS_LS_COMMON_THREAD, "id=%p: pthread_create() failed with %d", (void *)thread, attr_return); if (is_managed_thread) { aws_thread_decrement_unjoined_count(); } From bfcf607c90053633f87b086ebe1ad51d435f6929 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Thu, 7 Mar 2024 15:32:20 -0800 Subject: [PATCH 04/26] Revert "fix again" This reverts commit 90c57d0f91ecc77fa5e6552fe4049d2facb5c8c3. --- source/posix/thread.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index e517278ed..048d93fc8 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -266,18 +266,18 @@ int s_init_pthread_attr(size_t stack_size, int32_t cpu_id, pthread_attr_t *out_a * Thread affinity is also not supported on Android systems, and honestly, if you're running android on a NUMA * configuration, you've got bigger problems. */ #if AWS_AFFINITY_METHOD == AWS_AFFINITY_METHOD_PTHREAD_ATTR - if (options->cpu_id >= 0) { + if (cpu_id >= 0) { AWS_LOGF_INFO( AWS_LS_COMMON_THREAD, "id=%p: cpu affinity of cpu_id %d was specified, attempting to honor the value.", (void *)thread, - options->cpu_id); + cpu_id); cpu_set_t cpuset; CPU_ZERO(&cpuset); - CPU_SET((uint32_t)options->cpu_id, &cpuset); + CPU_SET((uint32_t)cpu_id, &cpuset); - attr_return = pthread_attr_setaffinity_np(out_attributes, sizeof(cpuset), &cpuset); + attr_return = pthread_attr_setaffinity_np(attributes_ptr, sizeof(cpuset), &cpuset); if (attr_return) { AWS_LOGF_ERROR( @@ -343,9 +343,28 @@ int aws_thread_launch( attr_return = pthread_create(&thread->thread_id, attributes_ptr, thread_fn, (void *)wrapper); + if (attr_return == EINVAL && options->cpu_id >= 0) { + /* try without cpu_id */ + AWS_LOGF_DEBUG( + AWS_LS_COMMON_THREAD, + "id=%p: pthread_create() failed with %d and cpu_id:%d, trying without cpu_id", + (void *)thread, + attr_return, + options->cpu_id); + if (attributes_ptr) { + pthread_attr_destroy(attributes_ptr); + } + wrapper->membind = false; + attr_return = s_init_pthread_attr(options->stack_size, options->cpu_id, &attributes); + if (attr_return) { + goto cleanup; + } + attributes_ptr = &attributes; + attr_return = pthread_create(&thread->thread_id, attributes_ptr, thread_fn, (void *)wrapper); + } + if (attr_return) { - if (att) - AWS_LOGF_ERROR(AWS_LS_COMMON_THREAD, "id=%p: pthread_create() failed with %d", (void *)thread, attr_return); + AWS_LOGF_ERROR(AWS_LS_COMMON_THREAD, "id=%p: pthread_create() failed with %d", (void *)thread, attr_return); if (is_managed_thread) { aws_thread_decrement_unjoined_count(); } From 1cd0269caaa93e56c35028c51f9f6675262e9d8e Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Thu, 7 Mar 2024 15:33:22 -0800 Subject: [PATCH 05/26] fix out_attributes --- source/posix/thread.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index 048d93fc8..fc4185ea3 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -277,7 +277,7 @@ int s_init_pthread_attr(size_t stack_size, int32_t cpu_id, pthread_attr_t *out_a CPU_ZERO(&cpuset); CPU_SET((uint32_t)cpu_id, &cpuset); - attr_return = pthread_attr_setaffinity_np(attributes_ptr, sizeof(cpuset), &cpuset); + attr_return = pthread_attr_setaffinity_np(out_attributes, sizeof(cpuset), &cpuset); if (attr_return) { AWS_LOGF_ERROR( From adc766ec82ce9ff150ac436d28fbbecb7447040d Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Thu, 7 Mar 2024 15:35:08 -0800 Subject: [PATCH 06/26] fix log --- source/posix/thread.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index fc4185ea3..1a98bb9b3 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -268,10 +268,7 @@ int s_init_pthread_attr(size_t stack_size, int32_t cpu_id, pthread_attr_t *out_a #if AWS_AFFINITY_METHOD == AWS_AFFINITY_METHOD_PTHREAD_ATTR if (cpu_id >= 0) { AWS_LOGF_INFO( - AWS_LS_COMMON_THREAD, - "id=%p: cpu affinity of cpu_id %d was specified, attempting to honor the value.", - (void *)thread, - cpu_id); + AWS_LS_COMMON_THREAD, "cpu affinity of cpu_id %d was specified, attempting to honor the value.", cpu_id); cpu_set_t cpuset; CPU_ZERO(&cpuset); @@ -280,11 +277,7 @@ int s_init_pthread_attr(size_t stack_size, int32_t cpu_id, pthread_attr_t *out_a attr_return = pthread_attr_setaffinity_np(out_attributes, sizeof(cpuset), &cpuset); if (attr_return) { - AWS_LOGF_ERROR( - AWS_LS_COMMON_THREAD, - "id=%p: pthread_attr_setaffinity_np() failed with %d.", - (void *)thread, - attr_return); + AWS_LOGF_ERROR(AWS_LS_COMMON_THREAD, "pthread_attr_setaffinity_np() failed with %d.", attr_return); return attr_return; } } From 8231828d92ca36052b76dbb332ecc0fdf0411e6f Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Thu, 7 Mar 2024 15:40:40 -0800 Subject: [PATCH 07/26] add test case for invalid cpu_id --- tests/CMakeLists.txt | 1 + tests/thread_test.c | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 391f98fac..d2462469a 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -40,6 +40,7 @@ add_test_case(aws_load_error_strings_test) add_test_case(aws_assume_compiles_test) add_test_case(thread_creation_join_test) +add_test_case(thread_creation_join_invalid_cpu_id_test) add_test_case(thread_atexit_test) add_test_case(test_managed_thread_join) add_test_case(test_managed_thread_join_timeout) diff --git a/tests/thread_test.c b/tests/thread_test.c index 4cf3278b9..b65ba51f9 100644 --- a/tests/thread_test.c +++ b/tests/thread_test.c @@ -70,6 +70,47 @@ static int s_test_thread_creation_join_fn(struct aws_allocator *allocator, void AWS_TEST_CASE(thread_creation_join_test, s_test_thread_creation_join_fn) +static int s_test_thread_creation_join_invalid_cpu_id_fn(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + aws_common_library_init(allocator); + struct thread_test_data test_data = {.allocator = allocator}; + + struct aws_thread thread; + aws_thread_init(&thread, allocator); + + struct aws_thread_options thread_options = *aws_default_thread_options(); + /* invalid CPU id, make sure that cpu id is best effort based */ + thread_options.cpu_id = INT32_MAX; + + ASSERT_SUCCESS( + aws_thread_launch(&thread, s_thread_fn, (void *)&test_data, &thread_options), "thread creation failed"); + ASSERT_INT_EQUALS( + AWS_THREAD_JOINABLE, aws_thread_get_detach_state(&thread), "thread state should have returned JOINABLE"); + ASSERT_SUCCESS(aws_thread_join(&thread), "thread join failed"); + ASSERT_TRUE( + aws_thread_thread_id_equal(test_data.thread_id, aws_thread_get_id(&thread)), + "get_thread_id should have returned the same id as the thread calling current_thread_id"); + ASSERT_INT_EQUALS( + AWS_THREAD_JOIN_COMPLETED, + aws_thread_get_detach_state(&thread), + "thread state should have returned JOIN_COMPLETED"); + + if (AWS_OP_SUCCESS == test_data.get_thread_name_error) { + ASSERT_CURSOR_VALUE_STRING_EQUALS( + aws_byte_cursor_from_c_str("MyThreadName"), test_data.thread_name, "thread name equals"); + } else { + ASSERT_INT_EQUALS(test_data.get_thread_name_error, AWS_ERROR_PLATFORM_NOT_SUPPORTED); + } + + aws_string_destroy(test_data.thread_name); + aws_thread_clean_up(&thread); + aws_common_library_clean_up(); + + return 0; +} + +AWS_TEST_CASE(thread_creation_join_invalid_cpu_id_test, s_test_thread_creation_join_invalid_cpu_id_fn) + static uint32_t s_atexit_call_count = 0; static void s_thread_atexit_fn(void *user_data) { (void)user_data; From c5cd12b882d323717b91725a2858100572bc4134 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 09:16:34 -0800 Subject: [PATCH 08/26] fix for mac --- tests/thread_test.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/thread_test.c b/tests/thread_test.c index b65ba51f9..85b44d34b 100644 --- a/tests/thread_test.c +++ b/tests/thread_test.c @@ -95,13 +95,6 @@ static int s_test_thread_creation_join_invalid_cpu_id_fn(struct aws_allocator *a aws_thread_get_detach_state(&thread), "thread state should have returned JOIN_COMPLETED"); - if (AWS_OP_SUCCESS == test_data.get_thread_name_error) { - ASSERT_CURSOR_VALUE_STRING_EQUALS( - aws_byte_cursor_from_c_str("MyThreadName"), test_data.thread_name, "thread name equals"); - } else { - ASSERT_INT_EQUALS(test_data.get_thread_name_error, AWS_ERROR_PLATFORM_NOT_SUPPORTED); - } - aws_string_destroy(test_data.thread_name); aws_thread_clean_up(&thread); aws_common_library_clean_up(); From aee0b66538f933dcc40ee41a14a00eadbf7845a4 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 09:42:15 -0800 Subject: [PATCH 09/26] fix --- source/posix/thread.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index 1a98bb9b3..e352cbd04 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -348,7 +348,7 @@ int aws_thread_launch( pthread_attr_destroy(attributes_ptr); } wrapper->membind = false; - attr_return = s_init_pthread_attr(options->stack_size, options->cpu_id, &attributes); + attr_return = s_init_pthread_attr(options->stack_size, -1, &attributes); if (attr_return) { goto cleanup; } From 49dd28571d949fba6aeb8d34096830cd133b63e6 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 09:45:42 -0800 Subject: [PATCH 10/26] another fix --- source/posix/thread.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index e352cbd04..cddfd1cb6 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -336,7 +336,7 @@ int aws_thread_launch( attr_return = pthread_create(&thread->thread_id, attributes_ptr, thread_fn, (void *)wrapper); - if (attr_return == EINVAL && options->cpu_id >= 0) { + if (attr_return == EINVAL && options && options->cpu_id >= 0) { /* try without cpu_id */ AWS_LOGF_DEBUG( AWS_LS_COMMON_THREAD, From 60e6c80a6dd0238411cfff9300ed97b36b9d1da5 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 09:48:05 -0800 Subject: [PATCH 11/26] not too invalid --- tests/thread_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/thread_test.c b/tests/thread_test.c index 85b44d34b..d89123751 100644 --- a/tests/thread_test.c +++ b/tests/thread_test.c @@ -80,7 +80,7 @@ static int s_test_thread_creation_join_invalid_cpu_id_fn(struct aws_allocator *a struct aws_thread_options thread_options = *aws_default_thread_options(); /* invalid CPU id, make sure that cpu id is best effort based */ - thread_options.cpu_id = INT32_MAX; + thread_options.cpu_id = 512; ASSERT_SUCCESS( aws_thread_launch(&thread, s_thread_fn, (void *)&test_data, &thread_options), "thread creation failed"); From da585c45938e64778e946b010f9ed909c2be6d98 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 09:56:30 -0800 Subject: [PATCH 12/26] ignore the setaffiinity error --- source/posix/thread.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index cddfd1cb6..521022bb2 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -278,7 +278,7 @@ int s_init_pthread_attr(size_t stack_size, int32_t cpu_id, pthread_attr_t *out_a if (attr_return) { AWS_LOGF_ERROR(AWS_LS_COMMON_THREAD, "pthread_attr_setaffinity_np() failed with %d.", attr_return); - return attr_return; + /* ignore the error */ } } #endif /* AWS_AFFINITY_METHOD == AWS_AFFINITY_METHOD_PTHREAD_ATTR */ From d366164d94c5bb9d52efae0a3cb4e392b03fe732 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 10:03:16 -0800 Subject: [PATCH 13/26] recursion ftw --- source/posix/thread.c | 105 +++++++++++++++++------------------------- 1 file changed, 43 insertions(+), 62 deletions(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index 521022bb2..082a81b6f 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -245,47 +245,6 @@ int aws_thread_init(struct aws_thread *thread, struct aws_allocator *allocator) return AWS_OP_SUCCESS; } -int s_init_pthread_attr(size_t stack_size, int32_t cpu_id, pthread_attr_t *out_attributes) { - int attr_return = 0; - attr_return = pthread_attr_init(out_attributes); - - if (attr_return) { - return attr_return; - } - - if (stack_size > PTHREAD_STACK_MIN) { - attr_return = pthread_attr_setstacksize(out_attributes, stack_size); - - if (attr_return) { - return attr_return; - } - } - -/* AFAIK you can't set thread affinity on apple platforms, and it doesn't really matter since all memory - * NUMA or not is setup in interleave mode. - * Thread affinity is also not supported on Android systems, and honestly, if you're running android on a NUMA - * configuration, you've got bigger problems. */ -#if AWS_AFFINITY_METHOD == AWS_AFFINITY_METHOD_PTHREAD_ATTR - if (cpu_id >= 0) { - AWS_LOGF_INFO( - AWS_LS_COMMON_THREAD, "cpu affinity of cpu_id %d was specified, attempting to honor the value.", cpu_id); - - cpu_set_t cpuset; - CPU_ZERO(&cpuset); - CPU_SET((uint32_t)cpu_id, &cpuset); - - attr_return = pthread_attr_setaffinity_np(out_attributes, sizeof(cpuset), &cpuset); - - if (attr_return) { - AWS_LOGF_ERROR(AWS_LS_COMMON_THREAD, "pthread_attr_setaffinity_np() failed with %d.", attr_return); - /* ignore the error */ - } - } -#endif /* AWS_AFFINITY_METHOD == AWS_AFFINITY_METHOD_PTHREAD_ATTR */ - - return AWS_OP_SUCCESS; -} - int aws_thread_launch( struct aws_thread *thread, void (*func)(void *arg), @@ -302,13 +261,50 @@ int aws_thread_launch( } if (options) { - attr_return = s_init_pthread_attr(options->stack_size, options->cpu_id, &attributes); + attr_return = pthread_attr_init(&attributes); if (attr_return) { goto cleanup; } attributes_ptr = &attributes; + + if (options->stack_size > PTHREAD_STACK_MIN) { + attr_return = pthread_attr_setstacksize(attributes_ptr, options->stack_size); + + if (attr_return) { + goto cleanup; + } + } + +/* AFAIK you can't set thread affinity on apple platforms, and it doesn't really matter since all memory + * NUMA or not is setup in interleave mode. + * Thread affinity is also not supported on Android systems, and honestly, if you're running android on a NUMA + * configuration, you've got bigger problems. */ +#if AWS_AFFINITY_METHOD == AWS_AFFINITY_METHOD_PTHREAD_ATTR + if (options->cpu_id >= 0) { + AWS_LOGF_INFO( + AWS_LS_COMMON_THREAD, + "id=%p: cpu affinity of cpu_id %d was specified, attempting to honor the value.", + (void *)thread, + options->cpu_id); + + cpu_set_t cpuset; + CPU_ZERO(&cpuset); + CPU_SET((uint32_t)options->cpu_id, &cpuset); + + attr_return = pthread_attr_setaffinity_np(attributes_ptr, sizeof(cpuset), &cpuset); + + if (attr_return) { + AWS_LOGF_ERROR( + AWS_LS_COMMON_THREAD, + "id=%p: pthread_attr_setaffinity_np() failed with %d.", + (void *)thread, + attr_return); + goto cleanup; + } + } +#endif /* AWS_AFFINITY_METHOD == AWS_AFFINITY_METHOD_PTHREAD_ATTR */ } wrapper = aws_mem_calloc(thread->allocator, 1, sizeof(struct thread_wrapper)); @@ -336,26 +332,6 @@ int aws_thread_launch( attr_return = pthread_create(&thread->thread_id, attributes_ptr, thread_fn, (void *)wrapper); - if (attr_return == EINVAL && options && options->cpu_id >= 0) { - /* try without cpu_id */ - AWS_LOGF_DEBUG( - AWS_LS_COMMON_THREAD, - "id=%p: pthread_create() failed with %d and cpu_id:%d, trying without cpu_id", - (void *)thread, - attr_return, - options->cpu_id); - if (attributes_ptr) { - pthread_attr_destroy(attributes_ptr); - } - wrapper->membind = false; - attr_return = s_init_pthread_attr(options->stack_size, -1, &attributes); - if (attr_return) { - goto cleanup; - } - attributes_ptr = &attributes; - attr_return = pthread_create(&thread->thread_id, attributes_ptr, thread_fn, (void *)wrapper); - } - if (attr_return) { AWS_LOGF_ERROR(AWS_LS_COMMON_THREAD, "id=%p: pthread_create() failed with %d", (void *)thread, attr_return); if (is_managed_thread) { @@ -409,6 +385,11 @@ int aws_thread_launch( switch (attr_return) { case EINVAL: + if (options && options->cpu_id >= 0) { + struct aws_thread_options new_options = *options; + new_options.cpu_id = -1; + return aws_thread_launch(thread, func, arg, &new_options); + } return aws_raise_error(AWS_ERROR_THREAD_INVALID_SETTINGS); case EAGAIN: return aws_raise_error(AWS_ERROR_THREAD_INSUFFICIENT_RESOURCE); From 1a9a0ac27358ac31603141d998a4e9c4a1ecdb63 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 10:07:34 -0800 Subject: [PATCH 14/26] does the test work? --- source/posix/thread.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index 082a81b6f..bf003b037 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -387,7 +387,7 @@ int aws_thread_launch( case EINVAL: if (options && options->cpu_id >= 0) { struct aws_thread_options new_options = *options; - new_options.cpu_id = -1; + // new_options.cpu_id = -1; return aws_thread_launch(thread, func, arg, &new_options); } return aws_raise_error(AWS_ERROR_THREAD_INVALID_SETTINGS); From 284d80635c569acdcb86a9d31af95c4375506d76 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 10:16:44 -0800 Subject: [PATCH 15/26] now the fix --- source/posix/thread.c | 7 ++++++- tests/thread_test.c | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index bf003b037..f17a9654c 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -386,8 +386,13 @@ int aws_thread_launch( switch (attr_return) { case EINVAL: if (options && options->cpu_id >= 0) { + /* + * Sometimes, `pthread_create` can fail with an `EINVAL` error if the `cpu_id` is restricted. Since + * the pinning to a particular `cpu_id` is supposed to be best effort, try to launch a thread + * again without pinning to a specific cpu_id. + */ struct aws_thread_options new_options = *options; - // new_options.cpu_id = -1; + new_options.cpu_id = -1; return aws_thread_launch(thread, func, arg, &new_options); } return aws_raise_error(AWS_ERROR_THREAD_INVALID_SETTINGS); diff --git a/tests/thread_test.c b/tests/thread_test.c index d89123751..85b44d34b 100644 --- a/tests/thread_test.c +++ b/tests/thread_test.c @@ -80,7 +80,7 @@ static int s_test_thread_creation_join_invalid_cpu_id_fn(struct aws_allocator *a struct aws_thread_options thread_options = *aws_default_thread_options(); /* invalid CPU id, make sure that cpu id is best effort based */ - thread_options.cpu_id = 512; + thread_options.cpu_id = INT32_MAX; ASSERT_SUCCESS( aws_thread_launch(&thread, s_thread_fn, (void *)&test_data, &thread_options), "thread creation failed"); From 68815dca89a1e39d2a30ee5017486b1c7660a984 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 10:17:57 -0800 Subject: [PATCH 16/26] comments update --- source/posix/thread.c | 2 +- tests/thread_test.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index f17a9654c..e66713ebd 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -388,7 +388,7 @@ int aws_thread_launch( if (options && options->cpu_id >= 0) { /* * Sometimes, `pthread_create` can fail with an `EINVAL` error if the `cpu_id` is restricted. Since - * the pinning to a particular `cpu_id` is supposed to be best effort, try to launch a thread + * the pinning to a particular `cpu_id` is supposed to be best-effort, try to launch a thread * again without pinning to a specific cpu_id. */ struct aws_thread_options new_options = *options; diff --git a/tests/thread_test.c b/tests/thread_test.c index 85b44d34b..e38dcf502 100644 --- a/tests/thread_test.c +++ b/tests/thread_test.c @@ -79,7 +79,7 @@ static int s_test_thread_creation_join_invalid_cpu_id_fn(struct aws_allocator *a aws_thread_init(&thread, allocator); struct aws_thread_options thread_options = *aws_default_thread_options(); - /* invalid CPU id, make sure that cpu id is best effort based */ + /* invalid cpu_id. Ensure that the cpu_id is best-effort based. */ thread_options.cpu_id = INT32_MAX; ASSERT_SUCCESS( From 8df71e55dc35ab421d0b786119034d648dc48ca6 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 10:21:12 -0800 Subject: [PATCH 17/26] fix --- source/posix/thread.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index e66713ebd..d4df114dc 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -296,12 +296,11 @@ int aws_thread_launch( attr_return = pthread_attr_setaffinity_np(attributes_ptr, sizeof(cpuset), &cpuset); if (attr_return) { - AWS_LOGF_ERROR( + AWS_LOGF_WARN( AWS_LS_COMMON_THREAD, - "id=%p: pthread_attr_setaffinity_np() failed with %d.", + "id=%p: pthread_attr_setaffinity_np() failed with %d. Continuing without cpu affinity", (void *)thread, attr_return); - goto cleanup; } } #endif /* AWS_AFFINITY_METHOD == AWS_AFFINITY_METHOD_PTHREAD_ATTR */ From b5dada5f26cf2f887429bde17c0355e9c4d5ce43 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 10:24:10 -0800 Subject: [PATCH 18/26] not too invalid --- tests/thread_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/thread_test.c b/tests/thread_test.c index e38dcf502..5efb12f35 100644 --- a/tests/thread_test.c +++ b/tests/thread_test.c @@ -80,7 +80,7 @@ static int s_test_thread_creation_join_invalid_cpu_id_fn(struct aws_allocator *a struct aws_thread_options thread_options = *aws_default_thread_options(); /* invalid cpu_id. Ensure that the cpu_id is best-effort based. */ - thread_options.cpu_id = INT32_MAX; + thread_options.cpu_id = 1024; ASSERT_SUCCESS( aws_thread_launch(&thread, s_thread_fn, (void *)&test_data, &thread_options), "thread creation failed"); From ce7275832c0cfb85947c91f1be1cb20d0ce827b4 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 10:26:16 -0800 Subject: [PATCH 19/26] more high value --- tests/thread_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/thread_test.c b/tests/thread_test.c index 5efb12f35..50085d843 100644 --- a/tests/thread_test.c +++ b/tests/thread_test.c @@ -80,7 +80,7 @@ static int s_test_thread_creation_join_invalid_cpu_id_fn(struct aws_allocator *a struct aws_thread_options thread_options = *aws_default_thread_options(); /* invalid cpu_id. Ensure that the cpu_id is best-effort based. */ - thread_options.cpu_id = 1024; + thread_options.cpu_id = 4096; ASSERT_SUCCESS( aws_thread_launch(&thread, s_thread_fn, (void *)&test_data, &thread_options), "thread creation failed"); From 7947d8459b56a85ab52cfb571cf59e6866f51651 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 11:11:39 -0800 Subject: [PATCH 20/26] add log statement --- source/posix/thread.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index d4df114dc..cef1fa5b1 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -386,10 +386,14 @@ int aws_thread_launch( case EINVAL: if (options && options->cpu_id >= 0) { /* - * Sometimes, `pthread_create` can fail with an `EINVAL` error if the `cpu_id` is restricted. Since + * `pthread_create` can fail with an `EINVAL` error if the `cpu_id` is restricted. Since * the pinning to a particular `cpu_id` is supposed to be best-effort, try to launch a thread * again without pinning to a specific cpu_id. */ + AWS_LOGF_INFO( + AWS_LS_COMMON_THREAD, + "id=%p: Attempting to launch the thread again without specifying cpu_id", + (void *)thread); struct aws_thread_options new_options = *options; new_options.cpu_id = -1; return aws_thread_launch(thread, func, arg, &new_options); From 2a3dd175932e11c86d0530582b28f3c3f04d90ed Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 11:12:24 -0800 Subject: [PATCH 21/26] comment update --- source/posix/thread.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index cef1fa5b1..4b3e2e356 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -392,7 +392,7 @@ int aws_thread_launch( */ AWS_LOGF_INFO( AWS_LS_COMMON_THREAD, - "id=%p: Attempting to launch the thread again without specifying cpu_id", + "id=%p: Attempting to launch the thread again without pinning to a cpu_id", (void *)thread); struct aws_thread_options new_options = *options; new_options.cpu_id = -1; From 9da0fb62f176ec997311c592301a8b22bdc4f150 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 13:00:13 -0800 Subject: [PATCH 22/26] Cater to EAGAIN as well --- source/posix/thread.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index 4b3e2e356..1106fdf1d 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -381,23 +381,22 @@ int aws_thread_launch( if (attr_return) { s_thread_wrapper_destroy(wrapper); - + if ((attr_return == EINVAL || attr_return == EAGAIN) && (options && options->cpu_id >= 0)) { + /* + * `pthread_create` can fail with an `EINVAL` error or `EAGAIN` on freebasd if the `cpu_id` is + * restricted/invalid. Since the pinning to a particular `cpu_id` is supposed to be best-effort, try to + * launch a thread again without pinning to a specific cpu_id. + */ + AWS_LOGF_INFO( + AWS_LS_COMMON_THREAD, + "id=%p: Attempting to launch the thread again without pinning to a cpu_id", + (void *)thread); + struct aws_thread_options new_options = *options; + new_options.cpu_id = -1; + return aws_thread_launch(thread, func, arg, &new_options); + } switch (attr_return) { case EINVAL: - if (options && options->cpu_id >= 0) { - /* - * `pthread_create` can fail with an `EINVAL` error if the `cpu_id` is restricted. Since - * the pinning to a particular `cpu_id` is supposed to be best-effort, try to launch a thread - * again without pinning to a specific cpu_id. - */ - AWS_LOGF_INFO( - AWS_LS_COMMON_THREAD, - "id=%p: Attempting to launch the thread again without pinning to a cpu_id", - (void *)thread); - struct aws_thread_options new_options = *options; - new_options.cpu_id = -1; - return aws_thread_launch(thread, func, arg, &new_options); - } return aws_raise_error(AWS_ERROR_THREAD_INVALID_SETTINGS); case EAGAIN: return aws_raise_error(AWS_ERROR_THREAD_INSUFFICIENT_RESOURCE); From 1325b3b86d282e87fb63e2ef7bcf73b3319382fe Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 13:11:56 -0800 Subject: [PATCH 23/26] Is it EDEADLOCK? --- source/posix/thread.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index 1106fdf1d..cb1869069 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -381,7 +381,7 @@ int aws_thread_launch( if (attr_return) { s_thread_wrapper_destroy(wrapper); - if ((attr_return == EINVAL || attr_return == EAGAIN) && (options && options->cpu_id >= 0)) { + if ((attr_return == EINVAL || attr_return == EDEADLK) && (options && options->cpu_id >= 0)) { /* * `pthread_create` can fail with an `EINVAL` error or `EAGAIN` on freebasd if the `cpu_id` is * restricted/invalid. Since the pinning to a particular `cpu_id` is supposed to be best-effort, try to From c89be7eb741fbc302490c733919ff83589986b81 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 8 Mar 2024 15:05:47 -0800 Subject: [PATCH 24/26] retry without looking at the specific error --- source/posix/thread.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/posix/thread.c b/source/posix/thread.c index cb1869069..2883b846d 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -381,9 +381,9 @@ int aws_thread_launch( if (attr_return) { s_thread_wrapper_destroy(wrapper); - if ((attr_return == EINVAL || attr_return == EDEADLK) && (options && options->cpu_id >= 0)) { + if (options && options->cpu_id >= 0) { /* - * `pthread_create` can fail with an `EINVAL` error or `EAGAIN` on freebasd if the `cpu_id` is + * `pthread_create` can fail with an `EINVAL` error or `EDEADLK` on freebasd if the `cpu_id` is * restricted/invalid. Since the pinning to a particular `cpu_id` is supposed to be best-effort, try to * launch a thread again without pinning to a specific cpu_id. */ From 165ab4047bc6e772b37402aab8125b7a64796726 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Mon, 11 Mar 2024 10:53:49 -0700 Subject: [PATCH 25/26] is CI flaky? From d236d44cf29cb10e6275e44d59a5c9e8107136cd Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Mon, 11 Mar 2024 11:28:54 -0700 Subject: [PATCH 26/26] Keep the goto cleanup --- source/posix/thread.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source/posix/thread.c b/source/posix/thread.c index 2883b846d..af7fac84c 100644 --- a/source/posix/thread.c +++ b/source/posix/thread.c @@ -301,6 +301,7 @@ int aws_thread_launch( "id=%p: pthread_attr_setaffinity_np() failed with %d. Continuing without cpu affinity", (void *)thread, attr_return); + goto cleanup; } } #endif /* AWS_AFFINITY_METHOD == AWS_AFFINITY_METHOD_PTHREAD_ATTR */