From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id F073641DB2; Thu, 2 Mar 2023 05:08:45 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 93EE040E09; Thu, 2 Mar 2023 05:08:45 +0100 (CET) Received: from EUR03-DBA-obe.outbound.protection.outlook.com (mail-dbaeur03on2046.outbound.protection.outlook.com [40.107.104.46]) by mails.dpdk.org (Postfix) with ESMTP id 7473640DFB for ; Thu, 2 Mar 2023 05:08:44 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=HMBOd2lYP8qHLTq6AQzZlk5CmvKZbaWGgBQxSREUsEU=; b=IuPHKwT65z+cOkV5svhq74SUpAw/6kaqT73rYiCpV1PLwqKtGUOB4sVuZf8p0WjdP0iw/eSm+z60rHuubrDanNGRTu4WOSLeEojp1STAOFkje+dIfBjpmgH89WcM+bwZAytLlP6cbo01cZRZN5QIFqf1rfgPpGVvI+6GQf0+4N8= Received: from DB6P195CA0002.EURP195.PROD.OUTLOOK.COM (2603:10a6:4:cb::12) by AS8PR08MB5894.eurprd08.prod.outlook.com (2603:10a6:20b:23d::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6134.27; Thu, 2 Mar 2023 04:08:42 +0000 Received: from DBAEUR03FT003.eop-EUR03.prod.protection.outlook.com (2603:10a6:4:cb:cafe::b4) by DB6P195CA0002.outlook.office365.com (2603:10a6:4:cb::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6156.18 via Frontend Transport; Thu, 2 Mar 2023 04:08:42 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; pr=C Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by DBAEUR03FT003.mail.protection.outlook.com (100.127.142.89) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6156.19 via Frontend Transport; Thu, 2 Mar 2023 04:08:42 +0000 Received: ("Tessian outbound 2ba0ed2ebb9f:v135"); Thu, 02 Mar 2023 04:08:42 +0000 X-CR-MTA-TID: 64aa7808 Received: from 5bfcffdadad8.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 5439616D-B7D9-4481-A7DB-100BAC53E55F.1; Thu, 02 Mar 2023 04:08:36 +0000 Received: from EUR04-VI1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 5bfcffdadad8.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Thu, 02 Mar 2023 04:08:36 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Ta3nN+bN5oGEmHJCileUX7e1CzsYaGc8awfURwFASEfIv2TAun946v6jH62cCLDZDg78TpGYVzHAew9EFWtWrgV0Ru07cjR29KkwslahOze9V5Rkex7cCFgGvvTeN5ZAm2vC1tYj3z5OWXNkbTQCpe73YXwO0bPVsPWf3gX6EFiFyi3KtlB0XlbLX/0YCMVoCoOdxJk//9NXgS9i7aYPAc0qpWmSCXOPpkeQpN4H5LM/Ezol5cGhsi++eEdSRyeULWH7CW1G/1ySz1SJrJpbCklSVuFCe9+Mjf++V/vt9G/cxPr/Q7VbYzrwtzPBYoq0VZH835OJdZLfKbxM7I0RPg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=HMBOd2lYP8qHLTq6AQzZlk5CmvKZbaWGgBQxSREUsEU=; b=XX0UtF3AuPsjI+aOENrwbNZshXQrh62tLXEMibq2bkcKpob5CHxk9VLdcQbJFbSTgN3Eu/JNkuwl5ryKs9APiNHKHM3A8UiJYsZCAtiL83hPlMS5vHsy0K4RIuytYXCnH96ZU8dSVDgbnpeglnJIO8DV1FlH2L09oyAjHqhbuLPiRc0KJUdKWZBFQhpMh2L3N8DW4fG8X50VQvzHQOtY2WQlb6jSM74bZv2D4olTfzVspj3JgaxkMu6Nqj/c5kQ7QZc7pXLgzUByeGpo2jGucCE77cDteEo4fHDWE/IRgJapwYZUi7RgicrsRwh/5qNRSCIsGQZbhScoc7WBPiwExA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=HMBOd2lYP8qHLTq6AQzZlk5CmvKZbaWGgBQxSREUsEU=; b=IuPHKwT65z+cOkV5svhq74SUpAw/6kaqT73rYiCpV1PLwqKtGUOB4sVuZf8p0WjdP0iw/eSm+z60rHuubrDanNGRTu4WOSLeEojp1STAOFkje+dIfBjpmgH89WcM+bwZAytLlP6cbo01cZRZN5QIFqf1rfgPpGVvI+6GQf0+4N8= Received: from DBAPR08MB5814.eurprd08.prod.outlook.com (2603:10a6:10:1b1::6) by AM9PR08MB6676.eurprd08.prod.outlook.com (2603:10a6:20b:2ff::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6156.19; Thu, 2 Mar 2023 04:08:27 +0000 Received: from DBAPR08MB5814.eurprd08.prod.outlook.com ([fe80::910e:e35f:b1eb:ae9]) by DBAPR08MB5814.eurprd08.prod.outlook.com ([fe80::910e:e35f:b1eb:ae9%4]) with mapi id 15.20.6156.019; Thu, 2 Mar 2023 04:08:26 +0000 From: Honnappa Nagarahalli To: Tyler Retzlaff CC: "dev@dpdk.org" , "david.marchand@redhat.com" , "thomas@monjalon.net" , nd , nd Subject: RE: [PATCH v2] eal: fix thread race in control thread creation Thread-Topic: [PATCH v2] eal: fix thread race in control thread creation Thread-Index: AQHZTIIr58vajsFiwkSJVw8aJEccNq7msuAggAAFlACAACbdIA== Date: Thu, 2 Mar 2023 04:08:22 +0000 Message-ID: References: <1677518230-1194-1-git-send-email-roretzla@linux.microsoft.com> <1677704982-2643-1-git-send-email-roretzla@linux.microsoft.com> <20230302014558.GA9271@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> In-Reply-To: <20230302014558.GA9271@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ts-tracking-id: B68772A729E41F48B998692966F2D59F.0 x-checkrecipientchecked: true Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; x-ms-traffictypediagnostic: DBAPR08MB5814:EE_|AM9PR08MB6676:EE_|DBAEUR03FT003:EE_|AS8PR08MB5894:EE_ X-MS-Office365-Filtering-Correlation-Id: 049fe18f-7230-4c4f-e860-08db1ad3cfb4 x-ld-processed: f34e5979-57d9-4aaa-ad4d-b122a662184d,ExtAddr x-checkrecipientrouted: true nodisclaimer: true X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: UZkBo3WJjki8BNh9942FyL/BB9XKwQz5JE2CgDEDbgx+UfSJq6aWfVYayBcmA96OKBBVYuRh9xiVJ6trf1neubhD7p+aAc52l5bo5lzYNSiv0k/hQBUcMr7YGD2252hgq16zVirOXwDs2hgk6zRuKMVocH2TtqZB1D01OZQDS25/sqqkRyT6TclZZv4B3aJDqjfUe1j6EiCY5C1bpco8W/3arKOho/pCzw+UPl/A7ypU+14AznrzXDrNoiqd9YPWaNv1CEFKlmyZZvzKN7pVUSQqPtvgLL6zSGjk2l7O15cs9aKInipNNUpK2k6dlc87iDO9YoIGOHouzX3kpibmccKsbjzM3NJNaIEBdr8pSptDLS4h0Bw+jwVu/Iq8zTaRt8J6Xed02EXbAq/6s5vesM5/2FdVh64l/OmdVazJXP+kcRnk+yNRgCTJDlo5gQZ9l4Ri01q3WG1bbvvtOow7DJfFbVd6FPHs206fCuRl4qdvkpATgKPDNx4O5ouJnJjw51jRoC6jOkwlPU3QA6ez/ShUbB/5WWM/bhYXTwLEl8Z72c/MxL9z/w/LNEFHvbBvg+5LKe4s2bJIMA3l5v4waxCY8N18YjIQD+GozheP5OjCEjiLiI30GO28DbL0DvEDKrvn5BwpP/EIFNTnV4T1XdskH1JcNPN3UAeABIgoUjFDe+ad0kUugg9P9biv9ifQbAAK2glDgT/YKQVcS+cR3g== X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DBAPR08MB5814.eurprd08.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230025)(4636009)(396003)(39860400002)(376002)(136003)(346002)(366004)(451199018)(5660300002)(66446008)(66476007)(4326008)(64756008)(8936002)(83380400001)(41300700001)(2906002)(66556008)(66946007)(76116006)(55016003)(52536014)(8676002)(316002)(33656002)(122000001)(38100700002)(6916009)(186003)(26005)(9686003)(6506007)(7696005)(71200400001)(6666004)(478600001)(86362001)(54906003)(38070700005); DIR:OUT; SFP:1101; Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM9PR08MB6676 Original-Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: DBAEUR03FT003.eop-EUR03.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: bf9a5cbe-f027-4903-4006-08db1ad3c39b X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Ydwdu6VwhbSApITXmy0wwMZmCeEhIshW+aTJVpXhrZIvznrBXVQG4pd0TvjcnneBcxUXqmq/zq/SQo89zSW0QenX184cpQPe+afbR7u49DkosoVmTYmhFyEUd0GFkGMYFzpkAlcPVre+8pK8vs/HHTDRsfsyo2O1elFbpfuoe+niMdAL3ae/fHwMyAsi38vjBE/F3yf/xb8p/T9U2Oz0SeZbyNQQam2d72/hXQoZmB7FBbn1OvvyXi8kWX5TvxGaEUj7wQPS1rRAbsINdusyNatUB6OzrK4rcZkCj3QO+xRCsyJC1TAig6bGYyMFIySM9znlvGy9MSGV9uAxUPfrQ1UUpQUCjI3MINq6amEQkZ7hbgJiW66xkPwC1dEDBQxdYbUM3mxNi2nI6QEQPYFau9CTUxgWpkNL/ykdT2dE4TO9EujfXg1Odbu78w+b5634czDYHojcss6WICnnTEdLiflTuBHFBPvY+ce2H+K+zZKNKGXfIN9eYSMspSfK886MyOQfvJZpokQrRgu9wgQ7uVTerggmERXeNmLOn8hrs/rveayZmTmh+fdh0+Dx2kfZPmeVQ4MJRQziaVhYjN3hBUiyOzM+hDznu0l6Ztx9YB+hKxxHCK+zvy4ko3c3Fu2TbtVxIa/3MmIwBOxwoE3rNLkBS4wMPAAp/t+p+dCMR2Ud5eQFB1D/k/okPojKKQPlVY0MQ620q+tGeoEDTWT3MA== X-Forefront-Antispam-Report: CIP:63.35.35.123; CTRY:IE; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:64aa7808-outbound-1.mta.getcheckrecipient.com; PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com; CAT:NONE; SFS:(13230025)(4636009)(376002)(396003)(346002)(39860400002)(136003)(451199018)(46966006)(36840700001)(40470700004)(70586007)(8676002)(70206006)(54906003)(6862004)(4326008)(41300700001)(2906002)(52536014)(5660300002)(83380400001)(7696005)(8936002)(6666004)(478600001)(6506007)(26005)(316002)(186003)(9686003)(336012)(55016003)(36860700001)(40480700001)(356005)(47076005)(33656002)(81166007)(82740400003)(86362001)(40460700003)(82310400005); DIR:OUT; SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Mar 2023 04:08:42.8062 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 049fe18f-7230-4c4f-e860-08db1ad3cfb4 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d; Ip=[63.35.35.123]; Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: DBAEUR03FT003.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR08MB5894 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 > > > > > > When ctrl_thread_init transitions params->ctrl_thread_status from > > > CTRL_THREAD_LAUNCHING the creating thread and new thread may run > > > concurrently leading to unsynchronized access to params. > > IMO, the code will be simpler if we did not free 'params' in > 'rte_thread_create_control'/'rte_ctrl_thread_create'. We could avoid crea= ting > the local copies of start_routine and the arg. >=20 > You mean in the success case i assume? it still has to be free'd if > rte_thread_create fails. Yes >=20 > > See more comments below. > > > > > > > > This permits races for both the failure and success paths after > > > ctrl_thread_status is stored. > > > * params->ret may be loaded in ctrl_thread_init failure path > > > * params->arg may be loaded in ctrl_thread_start or > > > control_thread_start when calling start_routine. > > > > > > For ctrl_thread_init remove the params->ret load and just return 1 > > > since it is only interpreted as a indicator of success / failure of > ctrl_thread_init. > > > > > > For {ctrl,control}_thread_start store param->arg in stack allocated > > > storage prior to calling ctrl_thread_init and use the copy when calli= ng > start_routine. > > > > > > For control_thread_start if ctrl_thread_init fails just return 0 > > > instead of loading > > > params->ret, since the value returned is unused when > > > params->ctrl_thread_status is set > > > to CTRL_THREAD_ERROR when ctrl_thread_init fails. > > > > > > Fixes: 878b7468eacb ("eal: add platform agnostic control thread > > > API") > > > > > > Signed-off-by: Tyler Retzlaff > > > Reviewed-by: David Marchand > > > --- > > > lib/eal/common/eal_common_thread.c | 10 ++++++---- > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/lib/eal/common/eal_common_thread.c > > > b/lib/eal/common/eal_common_thread.c > > > index edb9d4e..079a385 100644 > > > --- a/lib/eal/common/eal_common_thread.c > > > +++ b/lib/eal/common/eal_common_thread.c > > > @@ -256,7 +256,7 @@ static int ctrl_thread_init(void *arg) > > > if (params->ret !=3D 0) { > > > __atomic_store_n(¶ms->ctrl_thread_status, > > > CTRL_THREAD_ERROR, __ATOMIC_RELEASE); > > > - return params->ret; > > > + return 1; > > > } > > > > > > __atomic_store_n(¶ms->ctrl_thread_status, > > > @@ -268,23 +268,25 @@ static int ctrl_thread_init(void *arg) static > > > void *ctrl_thread_start(void *arg) { > > > struct rte_thread_ctrl_params *params =3D arg; > > > + void *start_arg =3D params->arg; > > > void *(*start_routine)(void *) =3D params->u.ctrl_start_routine; > > These copies can be avoided, code will be much simpler > > > > > > > > if (ctrl_thread_init(arg) !=3D 0) > > > return NULL; > > > > > > - return start_routine(params->arg); > > > + return start_routine(start_arg); > > We can free 'params' here after 'start_routine' returns. >=20 > I guess it doesn't matter if the allocation is retained for the duration = of > start_routine() which could be ~long. Yes, that's what I thought, it is a small size. >=20 > David/Honnappah let me know what you decide. if you'd prefer to change to > honnappah's suggestion i'll put a new version up. >=20 > > > > > } > > > > > > static uint32_t control_thread_start(void *arg) { > > > struct rte_thread_ctrl_params *params =3D arg; > > > + void *start_arg =3D params->arg; > > > rte_thread_func start_routine =3D params->u.control_start_routine; > > > > > > if (ctrl_thread_init(arg) !=3D 0) > > > - return params->ret; > > > + return 0; > > > > > > - return start_routine(params->arg); > > > + return start_routine(start_arg); > > > } > > > > > > int > > > -- > > > 1.8.3.1