From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 437A4A09FD; Fri, 18 Dec 2020 20:37:21 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 13573CAD1; Fri, 18 Dec 2020 20:37:19 +0100 (CET) Received: from hqnvemgate24.nvidia.com (hqnvemgate24.nvidia.com [216.228.121.143]) by dpdk.org (Postfix) with ESMTP id 4D73CCACF for ; Fri, 18 Dec 2020 20:37:17 +0100 (CET) Received: from hqmail.nvidia.com (Not Verified[216.228.121.13]) by hqnvemgate24.nvidia.com (using TLS: TLSv1.2, AES256-SHA) id ; Fri, 18 Dec 2020 11:37:15 -0800 Received: from HQMAIL101.nvidia.com (172.20.187.10) by HQMAIL111.nvidia.com (172.20.187.18) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Fri, 18 Dec 2020 19:37:10 +0000 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.169) by HQMAIL101.nvidia.com (172.20.187.10) with Microsoft SMTP Server (TLS) id 15.0.1473.3 via Frontend Transport; Fri, 18 Dec 2020 19:37:10 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VeYXXD5Vl/I6BfBsUw/qFQRwAFDUfRHvUPj8wctciyEudoImiSVWdhVBpo+597EJLYA+kDYmIsRr1sV6Z/xLv1qFBFXu5X9Usp2d3jLhE5XNhH0hRT+dgvwbECxnkhtqoDVsKEWZXiCzYXNJWPpb2qyTq1uxDzIe5anpm83brhJ4xfhhxUHKfLHjzVSmjOlIl03c0nYr05rlYihkKPxA7NE3ZJdbTKTHF39IXO/rd8wBP0RbXpswiw1UPm/Ik+RWKU2gkpSTRa1Y/3k0dAA0z/WRntyr0KnlzUu4+W3bGcu1/z5hUUSz3NAIsjKnuOAIgoemlbSxkjpq9VnSeADxrA== 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=pm8OL8JsE3TLnxSGNEjkxF6yWDgHzIGzGusreGcoRfU=; b=mqSc0hUAAc67iKsDonw3dtRFSaKfUiFgwMLEmEeDeenI8BczTC1VRG2aWIHa/q30d0l8Tc2AQ5X7M0gU7LuurT7jXsvn8zrpx69T+YJzePRY9gymyxWrEXTUH8WsXLCspxOkeL+pCz6ELFLbbIXUZadr6ZTuJoX8h/ElBAbUaoNjmiSgytcVtRC0+ECj/dYezV2aVS4/YsVDeogQAIW2rgRN/ga0HnvQD78GWiGKjiXwdfGZ/a3vDY/ML1w8ToVkieK2uEETNd/5Cb3G0ZLHnyO0TIQZwn0H8pptLacFelLD63zFaZ5NyYvsP7a9iNchxRX6RiqvVrkZpSEihYhzYA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none Received: from DM6PR12MB3945.namprd12.prod.outlook.com (2603:10b6:5:1c2::27) by DM6PR12MB4187.namprd12.prod.outlook.com (2603:10b6:5:212::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3654.20; Fri, 18 Dec 2020 19:37:08 +0000 Received: from DM6PR12MB3945.namprd12.prod.outlook.com ([fe80::fc92:3e29:29d1:27b2]) by DM6PR12MB3945.namprd12.prod.outlook.com ([fe80::fc92:3e29:29d1:27b2%6]) with mapi id 15.20.3654.028; Fri, 18 Dec 2020 19:37:08 +0000 From: Tal Shnaiderman To: Dmitry Kozlyuk CC: "dev@dpdk.org" , NBU-Contact-Thomas Monjalon , "pallavi.kadam@intel.com" , "navasile@linux.microsoft.com" , "dmitrym@microsoft.com" , "david.marchand@redhat.com" , Khoa To Thread-Topic: [PATCH v3] eal: add generic thread-local-storage functions Thread-Index: AQHW1Lcf6D/0Y7sryEKMbIQoz4zllan9Mi6g Date: Fri, 18 Dec 2020 19:37:07 +0000 Message-ID: References: <20201213202437.12880-1-talshn@nvidia.com> <20201217174913.14280-1-talshn@nvidia.com> <20201217235630.51f67e72@sovereign> In-Reply-To: <20201217235630.51f67e72@sovereign> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: gmail.com; dkim=none (message not signed) header.d=none;gmail.com; dmarc=none action=none header.from=nvidia.com; x-originating-ip: [77.137.117.64] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: e2633595-fa91-4045-97b3-08d8a38c4e78 x-ms-traffictypediagnostic: DM6PR12MB4187: x-ld-processed: 43083d15-7273-40c1-b7db-39efd9ccc17a,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: eNmPErDIThgbESYeBcuyFke0ct1h997zAdvcgdmi7Se0Gpr4hj2BQm4BmdClUHFdAHrd/+wYb35ukgWGh3W+RUS7xtJ41wunPfOOgv0VZELdrEbWht3VlaMrxdrmT2XQKBRL5yzndRNNlCKOsLMgTIM2z51oazCs4w76KDQ55son8DtiV3ZP5/y4whoLOvDSp6KPHAISgi7sNOZsYQdBzQz9X2oHjIEl9SOBVyA1yP2CH13Qf0m+PPueNPdaoW9HkndWyWroVV7jPtbMXkdHhpKwUQBBVmjMK9t5SXqS06wFBhQ1TZ+ROhVbvZwnOx/bjiiHxsaYrW0jV3WqJlJuGhHGl2MmuhhMwy9cJFqe/X/0kov26EAt7mUSTN0fV27c x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM6PR12MB3945.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(396003)(346002)(376002)(39860400002)(366004)(136003)(316002)(33656002)(8676002)(6506007)(55016002)(478600001)(4326008)(8936002)(6916009)(86362001)(26005)(7696005)(54906003)(2906002)(66946007)(71200400001)(52536014)(66476007)(186003)(64756008)(5660300002)(83380400001)(66556008)(9686003)(76116006)(66446008)(41533002); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?2l1r9NsfYE+bI+U6obUdFKkPS2eJ2myxCvtj41Oly0KNVeByfWBSrTCCPne+?= =?us-ascii?Q?RObEY0FiWQxubvnmp5oSoK8YrkIQ/vuXbivxhZ17MiiKxWUxsOFpH9l5cO6w?= =?us-ascii?Q?IQ2kkSC7Ir/HXKR3vdPN9FnKF0kLwtHi47SgtTkaqzvRlG88f4DtVufUlADE?= =?us-ascii?Q?nr14Q5jfUk91+mlFh2zfzQpWlscPeetU0w6pBnvUoL1iedjPavXJbE51Ks4R?= =?us-ascii?Q?n8Ha4IH4Ye3xoTMmV6UzS7IptaMYrKbM5QXljS/RWqZkXGqQrs+E3Et032Ow?= =?us-ascii?Q?DYxW6krUVYJix4vEIqmLpponD5dIsutf11un9K9U73NDPFrkURcX7/E53k8r?= =?us-ascii?Q?vucFUqil8ePbm4Sr3j1qcmPlnGUJCYL6h3KTtJhJqbg2SZlK03EBTmtxamLf?= =?us-ascii?Q?FGjPteHrKx57eooPwk8Jn7qbosIG1UHoouCxUqls0c3bT6WZB5Pqp0qiRz0N?= =?us-ascii?Q?E/7LlQfTWlS0+fpJy/JL7G1qMGPBnB9p4pyTKyBHHeTE+WcTHbOTbIjLn/Cx?= =?us-ascii?Q?QWpPVa8WlxGlaXcpvjFvpCBIQg4y83OZyUU1EcMc1R8R6WTfiKOVMc6OFTUM?= =?us-ascii?Q?X2FkR4sBJJ3/HEYXj1kjhK02GFRYG/nUvr+HzIEM0mo8iocpU3OeWqjT3DXw?= =?us-ascii?Q?ZVYW0zQDgJdTjgWLI+WwyXSnrxXz+9sfbna8YtkdMv0NCmMYW6mZZ8GZrNX0?= =?us-ascii?Q?uqIPmfX1etjm+GQoAdcXqy4J+lvjZ23H8afrwtcYRWF9lMJ2L2FDnE2z/XfV?= =?us-ascii?Q?LXA1ktMj8YW1S+XesXv6gexEK9TPm3UaZXy+mkw/hNEQCYt7h3cQ0d0858Wq?= =?us-ascii?Q?fLU/L7KfWx6TbLG8FH1fXh28gzr9w8QDbbXUp99CYcCLdQ+cm81DgL1Qf32a?= =?us-ascii?Q?ZSX+f27nJpA7SNjID6Xgq9wicP0ggAzX47tSLedI3VJUAGRl9cpc8EBrqxSN?= =?us-ascii?Q?fgK1r6y+l6MD0cE+z0gR0MquxTH8ixLEElLhCUlto2I=3D?= x-ms-exchange-transport-forked: True Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3945.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: e2633595-fa91-4045-97b3-08d8a38c4e78 X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Dec 2020 19:37:07.8611 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 0l4jh0GePrU+k7KPElqzHwSdcbKtZrKQO9Fq5IxFp6TdvmdbALJ+HbQYo8wfBJqAmy1u7DMGcGeE/RUXOCPPHg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4187 X-OriginatorOrg: Nvidia.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1608320235; bh=pm8OL8JsE3TLnxSGNEjkxF6yWDgHzIGzGusreGcoRfU=; h=ARC-Seal:ARC-Message-Signature:ARC-Authentication-Results:From:To: CC:Subject:Thread-Topic:Thread-Index:Date:Message-ID:References: In-Reply-To:Accept-Language:Content-Language:X-MS-Has-Attach: X-MS-TNEF-Correlator:authentication-results:x-originating-ip: x-ms-publictraffictype:x-ms-office365-filtering-correlation-id: x-ms-traffictypediagnostic:x-ld-processed: x-microsoft-antispam-prvs:x-ms-oob-tlc-oobclassifiers: x-ms-exchange-senderadcheck:x-microsoft-antispam: x-microsoft-antispam-message-info:x-forefront-antispam-report: x-ms-exchange-antispam-messagedata:x-ms-exchange-transport-forked: Content-Type:Content-Transfer-Encoding:MIME-Version: X-MS-Exchange-CrossTenant-AuthAs: X-MS-Exchange-CrossTenant-AuthSource: X-MS-Exchange-CrossTenant-Network-Message-Id: X-MS-Exchange-CrossTenant-originalarrivaltime: X-MS-Exchange-CrossTenant-fromentityheader: X-MS-Exchange-CrossTenant-id:X-MS-Exchange-CrossTenant-mailboxtype: X-MS-Exchange-CrossTenant-userprincipalname: X-MS-Exchange-Transport-CrossTenantHeadersStamped:X-OriginatorOrg; b=PEKvNr7U+pqmY4x+mXu6oSjQcY1+2dlm0AZuioQ5iKRuK0b8gB8NTa49v5JWLKlvo NUfXoSHhIgAZMDrr2StAepKsSN+RMCn0UR8krjxAzo2mfUkVzo+cL6xiOshiakObrW pjaUweCnk84LCcHtGkGcL8dO96cxGwO2i+ZJCVrSZwSbuc1/17RoMAb1/cUFYVhTWK l0KlolOYwB0gsdDg3kJMxRF3V4XpyOmP7m5aAVxQQn2L18JGPJK96ZbwEu7wqPwmSK Bhhcs0PjvRfPB353fsLenEO4Yah393MSUYcVJWDNBlWZd4GqdhqXVz8tvA4/98nY+q Zz2nBJK1e8sYA== Subject: Re: [dpdk-dev] [PATCH v3] eal: add generic thread-local-storage functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" > Subject: Re: [PATCH v3] eal: add generic thread-local-storage functions >=20 > External email: Use caution opening links or attachments >=20 >=20 > On Thu, 17 Dec 2020 19:49:13 +0200, Tal Shnaiderman wrote: > > Add support for tls functionality in EAL. > > > > The following functions are added: > > rte_tls_create_key - function to create a tls data key. > > rte_tls_delete_key - function to delete a tls data key. > > rte_tls_set_thread_value - function to set value bound to the tls key > > rte_tls_get_thread_value - function to get value bound to the tls key > > > > tls key will be defied by the new type rte_tls_key_t > > > > Signed-off-by: Tal Shnaiderman > > --- > > v3: switch from pthread shim to generic eal implementation [DmitryK] >=20 > Hi Tal, >=20 > Unix code can be placed in common/ directory, so that it can be eventuall= y > used on Windows with external pthread implementation. >=20 > See more comments inline. >=20 > > +++ b/lib/librte_eal/include/rte_tls.h > > @@ -0,0 +1,88 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2020 Mellanox Technologies, Ltd */ > > + > > +#include > > + > > +#ifndef _RTE_TLS_H_ > > +#define _RTE_TLS_H_ > > + > > +/** > > + * @file > > + * > > + * TLS functions > > + * > > + * Simple TLS functionality supplied by eal. > > + */ >=20 > These functions are supposed to be the first part of an API that will be = used > instead of naked pthread in DPDK. Maybe more generic names are in order, > like rte_thread.h and rte_thread_tls_create/destroy/get/set(). In particu= lar, > rte_tls_*() is confusing compared to rte_lcore_*(). >=20 > > + > > +/** > > + * Opaque pointer for tls key. > > + */ > > +typedef struct eal_tls_key *rte_tls_key_t; >=20 > "_t" suffix is reserved by POSIX, "rte_" prefix is sufficient. >=20 > > + > > +/** > > + * Function to create a tls data key visible to all threads in the > > +process >=20 > Typos: "TLS", "EAL". >=20 Didn't find the EAL typo. > > + * function need to be called once to create a key usable by all threa= ds. > > + * rte_tls_key_t is an opaque pointer used to store the allocated key. > > + * and optional destructor can be set to be called when a thread expir= es. > > + * > > + * @param key > > + * The rte_tls_key_t will cantain the allocated key >=20 > Typo: "cantain". I'd say: "Pointer to store the allocated rte_tls_key". >=20 > > + * @param destructor > > + * The function to be called when the thread expires >=20 > Typo: no period (line-break will be removed by Doxygen). >=20 > > + * Not supported on Windows OS. >=20 > Some drivers (net/mlx5, bus/dpaa, bus/fsmlc) rely on this feature. > Admittedly, it would be hard to implement. You know net/mlx5 well, how > will it be affected? Do you plan to stop relying on this feature or imple= ment it > in the future? Anyway, LGTM for now. In net/mlx5 we will not relay on the destructor for the Windows flow and pl= an to do the housekeeping in the PMD, to summarize we save HANDLE for each = thread using rte_tls_set_thread_value in the PMD destruction flow we call t= he needed release function for each terminated thread. I used the opaque pointer in case future development might want to implemen= t It without breaking the API, I assumed additions to the TLS key struct on= Windows side will be needed. Thanks for the review, I'll send a v4 after fixing your comments. >=20 > > + * > > + * @return > > + * On success, zero. > > + * On failure, a negative error number -ENOMEM or -ENOEXEC >=20 > Let's not make POSIX codes part of the API. >=20 > > + */ > > +__rte_experimental > > +int > > +rte_tls_create_key(rte_tls_key_t *key, void (*destructor)(void *)); >=20 > [...] > > +int > > +rte_tls_create_key(rte_tls_key_t *key, void (*destructor)(void *)) { > > + int err; > > + > > + *key =3D malloc(sizeof(struct eal_tls_key)); > > + if ((*key) =3D=3D NULL) { > > + RTE_LOG(DEBUG, EAL, "Cannot allocate tls key."); > > + return -ENOMEM; > > + } > > + err =3D pthread_key_create(&((*key)->thread_index), destructor); > > + if (err) { > > + RTE_LOG(DEBUG, EAL, "pthread_key_create failed: %s\n", > > + rte_strerror(err)); >=20 > Use strerror() for OS error codes here and below. >=20 > > + free(*key); > > + return -ENOEXEC; > > + } > > + return 0; > > +} > > + >=20 > [...] > > +int > > +rte_tls_set_thread_value(rte_tls_key_t key, const void *value) { > > + if (!key) > > + return -EINVAL; > > + /* discard const qualifier */ > > + char *p =3D (char *) (uintptr_t) value; >=20 > Please declare all variables at the top of a block. >=20 > > + > > + if (!TlsSetValue(key->thread_index, p)) { > > + RTE_LOG_WIN32_ERR("TlsSetValue()"); > > + return -ENOEXEC; > > + } > > + return 0; > > +} > > + > > +void * > > +rte_tls_get_thread_value(rte_tls_key_t key) { > > + if (!key) > > + rte_errno =3D EINVAL; > > + void *output =3D TlsGetValue(key->thread_index); >=20 > Same as above. >=20 > > + if (GetLastError() !=3D ERROR_SUCCESS) { > > + RTE_LOG_WIN32_ERR("TlsGetValue()"); > > + rte_errno =3D ENOEXEC; > > + return NULL; > > + } > > + return output; > > +}