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 D193CA0579; Mon, 3 May 2021 14:58:45 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0F5EB410DE; Mon, 3 May 2021 14:58:41 +0200 (CEST) Received: from NAM12-BN8-obe.outbound.protection.outlook.com (mail-bn8nam12on2136.outbound.protection.outlook.com [40.107.237.136]) by mails.dpdk.org (Postfix) with ESMTP id 626A1410D7 for ; Thu, 29 Apr 2021 23:31:55 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=HvgGd4LtLkiUFeZLqMo4cuQ2fDV3hl/RO3e11UFc9u93gzZgIrt6w5BQmNS6xVMTERg4oURJEeX4cgd/0KIB/mNcUpwhLFogZ5sWAjgWmz+q5zExTN+8oy3UevtogcWvA1ZdrS1m/cgOxUPW6jVFK3WqY0Fo1vijYRFNS1QrgrK7v8E6bjyiNID8bN81WNmmdM1Z7DBXszl6l7U7azTOvp53ceoVJV+wiSQ6qusVBp5blVN6+xydM7SsR54fNVoCOjB+9wph7sMrMJrmK3NQTWgDraJQMW3Dlrqe7UdS82mSmURFr33ZIDbAqTeh/Ysuez9jdZFTkwEZpHWom997lw== 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-SenderADCheck; bh=byCtfzxDBsW5sR3Ok/nRC1yNnaZ/PPn/KqRt1Bz7dkc=; b=k0SRl+Db5AdPdojlQ9vuUUuE94QKvSZO0W3wBVjwv/y/L25W2JPTmxbFINdaALNyPQVOzLuR0yazjSSzscv5JzAs/pGcK7nKRuD4f4KC8ozMsgc9A9ReOjBUGyz/U/EBUsA9p2sGpXv1PJ3bK0UbbGr2JYpQPpMkBMi4LiOXrFWBEDGmDhFszYsb3Ih2lTOMXzw/eobnDKwBR8SUXm5NoYev+s1oStbn1epT+az7/JMKv4bIVkRxkwz2+MDp42RSpHnoukD3gCURQvYbffqV+LkGewNd4tESFaKwa/+P8PzHWNi1nNZZA4LXMsSfSa4bQKdX1T3vNG3frfc27Zxrug== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=microsoft.com; dmarc=pass action=none header.from=microsoft.com; dkim=pass header.d=microsoft.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=byCtfzxDBsW5sR3Ok/nRC1yNnaZ/PPn/KqRt1Bz7dkc=; b=J/dW4YQYphmPyC5MAOQbyQHcqzYnIQYtWLMo/wYgS7B1ZdOLaZFFwx0zyXCIfppscKuEGz3UOGdczfas9RZGniCEX3vTRWAlV7mY8W6g/B2w0ehcPunFb0d19xCIJ3EPMyRudSrlBgEXRieFMb5ZUYACppT2rtlQSGLc2EhQr54= Received: from MN2PR21MB1423.namprd21.prod.outlook.com (2603:10b6:208:20c::12) by MN2PR21MB1184.namprd21.prod.outlook.com (2603:10b6:208:38::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4108.2; Thu, 29 Apr 2021 21:31:52 +0000 Received: from MN2PR21MB1423.namprd21.prod.outlook.com ([fe80::c98f:719d:438d:d25]) by MN2PR21MB1423.namprd21.prod.outlook.com ([fe80::c98f:719d:438d:d25%7]) with mapi id 15.20.4108.011; Thu, 29 Apr 2021 21:31:52 +0000 From: Dmitry Malloy To: Dmitry Kozlyuk , Narcisa Ana Maria Vasile CC: "dev@dpdk.org" , thomas , Khoa To , Narcisa Ana Maria Vasile , Tyler Retzlaff , "talshn@nvidia.com" , Omar Cardona , "bruce.richardson@intel.com" , "david.marchand@redhat.com" , "Kadam, Pallavi" Thread-Topic: [EXTERNAL] Re: [PATCH v6 06/10] eal: add thread lifetime management Thread-Index: AQHXKCon7nn0LZv0+EGa3Q/QEhVEW6rMIIAAgAAL/RA= Date: Thu, 29 Apr 2021 21:31:52 +0000 Message-ID: References: <1617057640-24301-2-git-send-email-navasile@linux.microsoft.com> <1617413948-10504-1-git-send-email-navasile@linux.microsoft.com> <1617413948-10504-7-git-send-email-navasile@linux.microsoft.com> <20210429234414.73ce39d7@sovereign> In-Reply-To: <20210429234414.73ce39d7@sovereign> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ActionId=e5857146-b11a-4913-b829-2def5c734d88; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=true; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Standard; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Name=Internal; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2021-04-29T21:27:08Z; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47; authentication-results: gmail.com; dkim=none (message not signed) header.d=none;gmail.com; dmarc=none action=none header.from=microsoft.com; x-originating-ip: [2001:4898:80e8:1:6187:5f1a:9b46:5d11] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 8dbda147-fa37-4701-7cb0-08d90b563485 x-ms-traffictypediagnostic: MN2PR21MB1184: x-ms-exchange-transport-forked: True x-ld-processed: 72f988bf-86f1-41af-91ab-2d7cd011db47,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 2WD+tjyVq4gkMwJrRId3FMl5mqEkLRgRCy227tETWvF3ppKJc98NxqY5zxsF+9Aqc1LclViz5i6MM+Kjpc/A/Q1HOwJZxiXF8a1aj0u8pUJuSnwX87ZUZolXG5tWIyBWBXW0EV00DQKm27ynhP5SKH+mLwq734D49qLiZO/Q1BeEPD7BmYuBS7sXymX4+NFnYNB4Gd9B97+AWVQr8YZjMj+WDmMPqeADhwlcPxcy6lT0csYR7m+MwODxIAE30WQUujkuKDceNdYSqZLtWHqtwmVwTHX52Se0/XU8hPa8zGMpjbwwxGSHTb1Os7yoEaNMVf4uRaXs2jl3ixThOlvUe8MiB6EoNTJJ0FiscjYhO4veYnn8aQvyeaEAxo7H+ydN0M4+kn3pkL9SBO6lvRjzs8O8j4+q9lJSXfmPCpQGkgV73iM1q9K1eXktP09/Qgstuj7vHhuUAxlelkmL2MRfLZ538AP1IQuzP66XxePQb8gpKx1pmzIi9fbHe9RY3hPmyy5MIorHlyL6y7DEnqui7Y+ITayNCP1Ciyeg+ewfduNg0eapTUvYAEOZ9fSkSHiHhLwK0/ie1wuelYMJBnBPdfxGIOyMbUDGLA1rxJiNoPg= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MN2PR21MB1423.namprd21.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(66476007)(66556008)(66946007)(66446008)(64756008)(55016002)(110136005)(122000001)(478600001)(5660300002)(76116006)(8936002)(71200400001)(2906002)(83380400001)(10290500003)(52536014)(54906003)(8676002)(53546011)(33656002)(86362001)(316002)(82960400001)(82950400001)(4326008)(6506007)(186003)(8990500004)(9686003)(38100700002)(7696005); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 2 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?AhrlZOK3Tt0nGLM7M2zQ/rAXehCrOmTzpTkYuIjldkZss2F1vxp44TIVNA9H?= =?us-ascii?Q?48nDPDyzUXGimQY8XLlvF1g+GsiBvawlVtVCPtrYqqUBi5yy9K1VCl5n56Jm?= =?us-ascii?Q?VNsygnP1DsvFh6fa49hGjlPDR5g+J1EHD0vg/x2U3O10sbcl5hgozf55/POo?= =?us-ascii?Q?Q815hNc6zxtPnpQa0RUSZn5AsDFTt2UYVD3sxSGrUYd1R46Q38OluTbPLnF8?= =?us-ascii?Q?p41Oedok74+NUmEu9fPNsYkBK6ctW0wMRB3Y7N9a+QxGXAIVeuiiKDH95gg3?= =?us-ascii?Q?vc9THuc8O6we/l0nBnw/kIn8jZ1grsoH/B/aCzvQtsPD/Nwy0cAZrVvKi25d?= =?us-ascii?Q?osbQEBF0B1cS1Iit9H6phiXR2PH5oxCEk66q8zA7tUyRTU8adBFv1RU7nfLN?= =?us-ascii?Q?HuZIPCsL0sor/g1LV42Pla8KHZQaP0eDIj1Exznafqf7ZoARhefhAS9xPMxs?= =?us-ascii?Q?VFlKGMKqU2XFqIkh3M6dfCcWSAFJO0BgjLDg3SDYhTYBqmEJwZh9O0f/NH8d?= =?us-ascii?Q?xc9tJAyd7HCvOKZn1Q3y4IdKfKrA2C3tMQNas9LWNXx//zRD+q7Wvf6AsfvX?= =?us-ascii?Q?eeWVDE1Mk2T/BSOcGUdqapGK4RmWvPQRo4x34e5xpchvZyQ5m0Si3sBY7p+U?= =?us-ascii?Q?l9bGNev1H6ayfB3lfRpxMOnAtv3TbVBSYDZvHRiXKhgHZBGvrzuG+cEydPdp?= =?us-ascii?Q?I8ax4m6rh2VfGxHTQAFYV1pVevJF3oQHDmxZLrvsy810YCEXrytxBhF20XRH?= =?us-ascii?Q?Rg8PlpK5BacC83JqrgYRvwTidm2CfXBA1IUwrT1TafvI39xuR7koQGRkIVwS?= =?us-ascii?Q?Uh6NeTfKqByUFJjXZW5ik6IG8hcMtrHq529gaPwwud0TzrdgDZz6dNQJwu63?= =?us-ascii?Q?nigKYwU1Q95oPZuIuMl/Z3EWal5yeZsXOvyGACW0t3ASUi1gdHiAmL+0nCPY?= =?us-ascii?Q?liZM4zQqlPT7fBLpaGRv9DNdkPf8D5kuBgXw4l8Y?= x-ms-exchange-antispam-messagedata-1: 4IVF4PEwoN/jU0n+CNmuqZNCBRi1tZLco+VBltV8lnzAD+MQZGUuMwuQH9601F4h4HX8hoRnHrPkb8umTM7B9XTUrVNoFdD+sWqf5rGUYrR8YQMp7XDNOKWJ8GRCVzTrgZac4XDnM1SWtWfl3nczAwXD4yJ+X1oG+Fay5Er/KLvs0ifwckeP26GB3kOkRqogUwADMbWdOMappVC/sBoW0hxDsb02UuKi+/8SFygSUWOAoGHNXNaej2/ECg3iFtYqRjIT4dhcU4nHOpsXDxNVdRuSs884BU96tLq+Qs40NZOrgFQNtqaXh4ks12Nay17MHQOxZltNOUYWIXtsFYA9B1bKJBQDQm5alHkvp81gKgU1AdWV76R4pfMxl6MI1cSblBM= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MN2PR21MB1423.namprd21.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8dbda147-fa37-4701-7cb0-08d90b563485 X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Apr 2021 21:31:52.3074 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: BH8m4ef+VHR1gqdI7KTkK8AwkmfLiBdeVkZmZ417s+BoHtOh8QJILnfpHniJFNjWSmbTI8xKRUYmN/6xMd39LEdLCNHkRKffcDIrVr7p/KE= X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR21MB1184 X-Mailman-Approved-At: Mon, 03 May 2021 14:58:38 +0200 Subject: Re: [dpdk-dev] [EXTERNAL] Re: [PATCH v6 06/10] eal: add thread lifetime management 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 Sender: "dev" Thread cancellation is a pain point. Emulating it properly is nearly imposs= ible (without hooking into various OS calls which are supposed to be "cance= llation points"). I do like the cancellation token idea, but I'm not sure h= ow existing clients, who rely on existing phtread_cancel() semantic, will r= eact to that - it will require a code re-write.=20 How about we defer fixing this to another follow-up change?=20 -----Original Message----- From: Dmitry Kozlyuk =20 Sent: Thursday, April 29, 2021 1:44 PM To: Narcisa Ana Maria Vasile Cc: dev@dpdk.org; thomas ; Khoa To ; Narcisa Ana Maria Vasile ; Dmitry Malloy <= dmitrym@microsoft.com>; Tyler Retzlaff ; talshn@nvi= dia.com; Omar Cardona ; bruce.richardson@intel.com;= david.marchand@redhat.com; Kadam, Pallavi Subject: [EXTERNAL] Re: [PATCH v6 06/10] eal: add thread lifetime managemen= t 2021-04-02 18:39 (UTC-0700), Narcisa Ana Maria Vasile: [...] > diff --git a/lib/librte_eal/windows/rte_thread.c=20 > b/lib/librte_eal/windows/rte_thread.c > index f61103bbc..86bbd7bc2 100644 > --- a/lib/librte_eal/windows/rte_thread.c > +++ b/lib/librte_eal/windows/rte_thread.c > @@ -329,6 +329,131 @@ rte_thread_attr_set_priority(rte_thread_attr_t *thr= ead_attr, > return 0; > } > =20 > +int > +rte_thread_create(rte_thread_t *thread_id, > + const rte_thread_attr_t *thread_attr, > + void *(*thread_func)(void *), void *args) { > + int ret =3D 0; > + HANDLE thread_handle =3D NULL; > + GROUP_AFFINITY thread_affinity; > + > + thread_handle =3D CreateThread(NULL, 0, > + (LPTHREAD_START_ROUTINE)(ULONG_PTR)thread_func, thread_func returns void* (8 bytes), LPTHREAD_START_ROUTING returns DWORD (= 4 byte), is this cast safe? It seems to be on x86, can't say for ARM64. > + args, 0, thread_id); > + if (thread_handle =3D=3D NULL) { > + ret =3D rte_thread_translate_win32_error(); > + RTE_LOG_WIN32_ERR("CreateThread()"); > + goto cleanup; > + } After this point, in case of errors the thread remains running, while the f= unction reports failure. It's better to create the thread suspended, config= ure its attributes (destroying the thread on failure), then resume (start) = the thread. > + > + if (thread_attr !=3D NULL) { > + if (CPU_COUNT(&thread_attr->cpuset) > 0) { > + ret =3D rte_convert_cpuset_to_affinity(&thread_attr->cpuset, &thread_= affinity); > + if (ret !=3D 0) { > + RTE_LOG(DEBUG, EAL, "Unable to convert cpuset to thread affinity\n")= ; > + goto cleanup; > + } > + > + if (!SetThreadGroupAffinity(thread_handle, &thread_affinity, NULL)) { > + ret =3D rte_thread_translate_win32_error(); > + RTE_LOG_WIN32_ERR("SetThreadGroupAffinity()"); > + goto cleanup; > + } > + } > + ret =3D rte_thread_set_priority(*thread_id, thread_attr->priority); > + if (ret !=3D 0) { > + RTE_LOG(DEBUG, EAL, "Unable to set thread priority\n"); > + goto cleanup; > + } > + } > + > + return 0; > + > +cleanup: > + if (thread_handle !=3D NULL) { > + CloseHandle(thread_handle); > + thread_handle =3D NULL; > + } > + return ret; > +} [...] > + > +int > +rte_thread_cancel(rte_thread_t thread_id) { > + int ret =3D 0; > + HANDLE thread_handle =3D NULL; > + > + thread_handle =3D OpenThread(THREAD_TERMINATE, FALSE, thread_id); > + if (thread_handle =3D=3D NULL) { > + ret =3D rte_thread_translate_win32_error(); > + RTE_LOG_WIN32_ERR("OpenThread()"); > + goto cleanup; > + } > + > + /* > + * TODO: Behavior is different between POSIX and Windows threads. > + * POSIX threads wait for a cancellation point. > + * Current Windows emulation kills thread at any point. > + */ > + ret =3D TerminateThread(thread_handle, 0); This is not acceptable. There is a handful of pthread_cancel() usages in DPDK. The threads they cancel take spinlocks and mutexes, so interrupt at arbitra= ry point may cause a deadlock. User Cancellation points Notes ---- ------------------- ----- net/ipn3ke libfdt functions? net/kni usleep Linux-only raw/ifpga open/read/write/close vdpa/ifc read vdpa/mlx5 usleep, nanosleep lib/eventdev rte_epoll_wait no pthread CP Possible solutions: 1. Make specific DPDK functions cancellation points for this API and ensure= DPDK users of pthread_cancel() call it. This way is almost non-invasive, = but it's more work in EAL. 2. Divert from pthread style: add cancellation token concept, so that DPDK = users can check themselves if they're cancelled. This is invasive, but very= explicit and flexible. > + if (ret !=3D 0) { > + ret =3D rte_thread_translate_win32_error(); > + RTE_LOG_WIN32_ERR("TerminateThread()"); > + goto cleanup; > + } > + > +cleanup: > + if (thread_handle !=3D NULL) { > + CloseHandle(thread_handle); > + thread_handle =3D NULL; This line is not needed. > + } > + return ret; > +} > + > int > rte_thread_key_create(rte_thread_key *key, > __rte_unused void (*destructor)(void *))