From patchwork Thu Mar 2 18:44:41 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Retzlaff X-Patchwork-Id: 124736 X-Patchwork-Delegate: david.marchand@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id DB11641DB6; Thu, 2 Mar 2023 19:44:51 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 15491427F5; Thu, 2 Mar 2023 19:44:47 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id AC8E040E09; Thu, 2 Mar 2023 19:44:44 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id EB4C020B9C3D; Thu, 2 Mar 2023 10:44:43 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com EB4C020B9C3D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1677782683; bh=xuZK/PT4vBYQyfE7YEavGvdJXFXwl7fUSKj7sgWKYWQ=; h=From:To:Cc:Subject:Date:From; b=JeLp7H0C9auqn8YS5CafRuWEIFHtVU+QwdQC3hmcKJKGD1EAAHFxseY5gMecKierO 9Vznx6nF+tYgoLjH0MxH6NZlil6ROi9JNfjrZPddtEytTimolUfteIoUurs7tFubIR GufApkhKKyOLX0VVokvkN6T3hd/J3lqcBBvU2yys= From: Tyler Retzlaff To: dev@dpdk.org Cc: david.marchand@redhat.com, Tyler Retzlaff , stable@dpdk.org Subject: [PATCH 1/2] eal: fix failure race and behavior of thread create Date: Thu, 2 Mar 2023 10:44:41 -0800 Message-Id: <1677782682-27200-1-git-send-email-roretzla@linux.microsoft.com> X-Mailer: git-send-email 1.8.3.1 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org In rte_thread_create setting affinity after pthread_create may fail. Such a failure should result in the entire rte_thread_create failing but doesn't. Additionally if there is a failure to set affinity a race exists where the creating thread will free ctx and depending on scheduling of the new thread it may also free ctx (double free). Resolve both of the above issues by using the pthread_setaffinity_np prior to thread creation to set the affinity of the created thread. By doing this no failure paths exist after pthread_create returns successfully. Fixes: ce6e911d20f6 ("eal: add thread lifetime API") Cc: stable@dpdk.org Cc: roretzla@linux.microsoft.com Signed-off-by: Tyler Retzlaff Reviewed-by: David Marchand --- lib/eal/unix/rte_thread.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c index 37ebfcf..5bf633b 100644 --- a/lib/eal/unix/rte_thread.c +++ b/lib/eal/unix/rte_thread.c @@ -155,6 +155,17 @@ struct thread_routine_ctx { RTE_LOG(DEBUG, EAL, "pthread_attr_setschedparam failed\n"); goto cleanup; } + + if (CPU_COUNT(&thread_attr->cpuset) > 0) { + ret = pthread_attr_setaffinity_np(attrp, + sizeof(thread_attr->cpuset), + &thread_attr->cpuset); + if (ret != 0) { + RTE_LOG(DEBUG, EAL, + "pthread_attr_setaffinity_np failed\n"); + goto cleanup; + } + } } ret = pthread_create((pthread_t *)&thread_id->opaque_id, attrp, @@ -164,15 +175,6 @@ struct thread_routine_ctx { goto cleanup; } - if (thread_attr != NULL && CPU_COUNT(&thread_attr->cpuset) > 0) { - ret = rte_thread_set_affinity_by_id(*thread_id, - &thread_attr->cpuset); - if (ret != 0) { - RTE_LOG(DEBUG, EAL, "rte_thread_set_affinity_by_id failed\n"); - goto cleanup; - } - } - ctx = NULL; cleanup: free(ctx); From patchwork Thu Mar 2 18:44:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Retzlaff X-Patchwork-Id: 124735 X-Patchwork-Delegate: david.marchand@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 143BF41DB6; Thu, 2 Mar 2023 19:44:46 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EC58C40E09; Thu, 2 Mar 2023 19:44:45 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id A74B5400D6; Thu, 2 Mar 2023 19:44:44 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id 037BD20B9C3E; Thu, 2 Mar 2023 10:44:43 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 037BD20B9C3E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1677782684; bh=MgRUnrUqbr8JPduDFqj2qJKyU96i9BllTD/kfpzjJbc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=WZMTCE6pKHJMzu4HzaOjwczrLEeevddWRo/mvqiCBAJZ1nkzJ/MH0cxeRfmqHqOV/ 4TrP8jN1BAlVMRtcDcjlhhelLVzKMn20rEfJb6pPqF5yuSRycZfkxU/j2a2IELUHOM /oXHknP6dHC2WCkR9iMcXcIqflAEo65//LOvu1oY= From: Tyler Retzlaff To: dev@dpdk.org Cc: david.marchand@redhat.com, Tyler Retzlaff , stable@dpdk.org Subject: [PATCH 2/2] eal/windows: fix create thread failure behavior Date: Thu, 2 Mar 2023 10:44:42 -0800 Message-Id: <1677782682-27200-2-git-send-email-roretzla@linux.microsoft.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1677782682-27200-1-git-send-email-roretzla@linux.microsoft.com> References: <1677782682-27200-1-git-send-email-roretzla@linux.microsoft.com> X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org In rte_thread_create setting affinity after CreateThread may fail. Such a failure is reported but strands the newly created thread in a suspended state. Resolve the above issue by notifying the newly created thread that it should terminate as soon as it is resumed, while still continuing to free the ctx. Fixes: ce6e911d20f6 ("eal: add thread lifetime API") Cc: stable@dpdk.org Cc: roretzla@linux.microsoft.com Signed-off-by: Tyler Retzlaff --- lib/eal/windows/rte_thread.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c index 8556a84..e528ac9 100644 --- a/lib/eal/windows/rte_thread.c +++ b/lib/eal/windows/rte_thread.c @@ -19,6 +19,7 @@ struct eal_tls_key { struct thread_routine_ctx { rte_thread_func thread_func; + bool thread_init_failed; void *routine_args; }; @@ -167,9 +168,13 @@ struct thread_routine_ctx { thread_func_wrapper(void *arg) { struct thread_routine_ctx ctx = *(struct thread_routine_ctx *)arg; + const bool thread_exit = __atomic_load_n(&ctx.thread_init_failed, __ATOMIC_ACQUIRE); free(arg); + if (thread_exit) + return 0; + return (DWORD)ctx.thread_func(ctx.routine_args); } @@ -183,6 +188,7 @@ struct thread_routine_ctx { HANDLE thread_handle = NULL; GROUP_AFFINITY thread_affinity; struct thread_routine_ctx *ctx; + bool thread_exit = false; ctx = calloc(1, sizeof(*ctx)); if (ctx == NULL) { @@ -192,6 +198,7 @@ struct thread_routine_ctx { } ctx->routine_args = args; ctx->thread_func = thread_func; + ctx->thread_init_failed = false; thread_handle = CreateThread(NULL, 0, thread_func_wrapper, ctx, CREATE_SUSPENDED, &tid); @@ -209,23 +216,29 @@ struct thread_routine_ctx { ); if (ret != 0) { RTE_LOG(DEBUG, EAL, "Unable to convert cpuset to thread affinity\n"); - goto cleanup; + thread_exit = true; + goto resume_thread; } if (!SetThreadGroupAffinity(thread_handle, &thread_affinity, NULL)) { ret = thread_log_last_error("SetThreadGroupAffinity()"); - goto cleanup; + thread_exit = true; + goto resume_thread; } } ret = rte_thread_set_priority(*thread_id, thread_attr->priority); if (ret != 0) { RTE_LOG(DEBUG, EAL, "Unable to set thread priority\n"); - goto cleanup; + thread_exit = true; + goto resume_thread; } } +resume_thread: + __atomic_store_n(&ctx->thread_init_failed, thread_exit, __ATOMIC_RELEASE); + if (ResumeThread(thread_handle) == (DWORD)-1) { ret = thread_log_last_error("ResumeThread()"); goto cleanup;