From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (xvm-189-124.dc0.ghst.net [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1E36AA09FF; Wed, 6 Jan 2021 17:04:47 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8F3DD1609D9; Wed, 6 Jan 2021 17:04:46 +0100 (CET) Received: from nat-hk.nvidia.com (nat-hk.nvidia.com [203.18.50.4]) by mails.dpdk.org (Postfix) with ESMTP id C43B01609D2 for ; Wed, 6 Jan 2021 17:04:43 +0100 (CET) Received: from HKMAIL101.nvidia.com (Not Verified[10.18.92.100]) by nat-hk.nvidia.com (using TLS: TLSv1.2, AES256-SHA) id ; Thu, 07 Jan 2021 00:04:42 +0800 Received: from HKMAIL103.nvidia.com (10.18.16.12) by HKMAIL101.nvidia.com (10.18.16.10) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Wed, 6 Jan 2021 16:04:36 +0000 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (104.47.59.170) by HKMAIL103.nvidia.com (10.18.16.12) with Microsoft SMTP Server (TLS) id 15.0.1473.3 via Frontend Transport; Wed, 6 Jan 2021 16:04:36 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=BD4eCAnn5NNSiq21v8UydTve4ehKRfGuBH4mFaN84VNBmHm6LiuWWZ3jc9LMQWhP22VBjFiSl3eeeS1i4hwvKev6m/iTflsZVSnPwKbqvpBO5YY9YLBZSrn6xAGbygUHsP5bFaXFk++RvLh/HPv0j+7ah7kmpWlJgKfjUz5BqMGX17qIo5qooJ4XjQ/1FzHchPOrFH2Tdg9jVENGg+9wvqPKcu6mhOAziWfvGkeKk6u0NH+o94jRuu1711xiQllKwoaJj9t7tWWGmnaI8NKgV74xufwMPX3dgFGlcyzFTB7C6c2iSmlgVV76WuhuOYep3fHGtVUIgyqucmMCGrnGdQ== 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=e/OK6q6ubS7HAcyUxMA8QkTQzc4QPR4JJuPEwzOKC28=; b=lI/LbuPzF7e2jYh6CR4EYf1we2oGM8ruy9wtfIHwaY6TJ5DstEI3oPIudF0riGtQmncsG2sRUdbRbipXc/8X20CFlI54j2uJobJ91g3IKLB0c57JvSYLbIPAsyuZP7Q6LxtL+mpcrlG8n9sEj/zJ+zqAyepCBPbtIXK5kqG6/ZBIBMCt6PHNtnKoiG3GFjzJJWRV/DMhWfRB2t2AYRFeST+IVPgYVChaCi8SOprSALRRG+TRPUPZUyLxJRQB8VQf/7HsK9Co/8rDb89nQ+n9fgh712i5SQzsbig+/xtJIBDq1TELgAsjTLSV/AjQnaPO0lUHWw6HvXXUXqPn64tdcw== 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 DM6PR12MB5005.namprd12.prod.outlook.com (2603:10b6:5:1be::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3721.20; Wed, 6 Jan 2021 16:04:34 +0000 Received: from DM6PR12MB3945.namprd12.prod.outlook.com ([fe80::d1dc:9fb2:4724:53df]) by DM6PR12MB3945.namprd12.prod.outlook.com ([fe80::d1dc:9fb2:4724:53df%7]) with mapi id 15.20.3721.024; Wed, 6 Jan 2021 16:04:34 +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" Thread-Topic: [PATCH v7 2/2] eal: add generic thread-local-storage functions Thread-Index: AQHW5D1kaZA1Ekx2HEqiTbFhJusoKKoawn3w Date: Wed, 6 Jan 2021 16:04:33 +0000 Message-ID: References: <20201226160848.9824-1-talshn@nvidia.com> <20210105170635.6212-1-talshn@nvidia.com> <20210105170635.6212-3-talshn@nvidia.com> <20210106180525.3f4dd1a3@sovereign> In-Reply-To: <20210106180525.3f4dd1a3@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.141.17] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: edf3e4ff-f1d1-4ed9-a056-08d8b25cc25e x-ms-traffictypediagnostic: DM6PR12MB5005: x-ld-processed: 43083d15-7273-40c1-b7db-39efd9ccc17a,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:8882; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: SohFBkJPebYl/z+XJZZJlEMClsI8oG+LkOlCpR83vo/Lo2wCj1ZDNmeEjXcxvAWElIkXzF5y86jN+VrvrXPwlJPte+pGTKXawMQ2MirFnKHz4ZQnagjpGaUE/FBsvaJYfjBRe6I7WU4nxmVq4G7Sjd1qZZDBiyMgMU2a4ZG0YyCtSWjunrQu8Fc4uprIv+yu9GRAwj2JPzlhftvFB9gzJ1zswamv9xEK20HmPVIHc1SVu2ymB6SSEDMiv731aWcLQNTkrspq/6kbBDs6HhRc4caP6GGTzvDIvlBfrecK1TrhIzBuW64IraaKPCxSPZyuJ2Ev1Mv13o23l2vvFW5x3HY/qBAlEUft0EQ3tBM1KMk5pDR8d6ITIo+wqxrVA33Q2bUbTtd8pRfNzgAvoPnviqdqI7Xtm1/dCpWtRh7NXn5+OSCPzxGLxR7Tu4Ch/Ivf 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)(136003)(396003)(366004)(376002)(346002)(39860400002)(83380400001)(478600001)(316002)(86362001)(54906003)(7696005)(6916009)(4326008)(9686003)(55016002)(71200400001)(2906002)(8936002)(5660300002)(26005)(64756008)(66476007)(66446008)(76116006)(8676002)(66556008)(186003)(6506007)(66946007)(52536014)(33656002)(41533002); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?gtxgcUQdgk5vfVPzMbi4xCAV8DYfa5B7mfTmO0HifbhTfTWWPvT90dR309re?= =?us-ascii?Q?9RE98y1Q2DMnfBywknmrGGt7LYAVOy0eWkpix+b8apYIb9xoV+FDpf/f3NNt?= =?us-ascii?Q?2FEfjfmNltHI5tWISFL4Xclbj4OyqE+IlGJIqPO7QhMWr7L3MEeZ/b7/mq1Y?= =?us-ascii?Q?yHsW66vqUKPMeskf7LSrYOPrM2YJ2Gq5DR1/yRNYu7jXadICJKwGKTTxyWz6?= =?us-ascii?Q?L42KO/s2hcPqBzSPDKjl4ke92RhFT5yl9Tgj6Shk6OcHHtDkwL8KaWdbkJEc?= =?us-ascii?Q?5F5IE3vrENMriodLUxtAahBPL2Be7Ys62rzH2QIOP2FLVI6iA6qZyN8gDo0j?= =?us-ascii?Q?2S+lt7lljlpWgFCj2YD97CfBPldc7Jw4/HLLuk2L8mp4Dt1uoByjpMWkCe4e?= =?us-ascii?Q?95NhmsDz6roCRt4yv8DzB9QGHKgcyK4VCEGQeBOG3wAN29NXT7D08ADcUsDq?= =?us-ascii?Q?FRMQ82YXQxd1irOZWkH6nMdjY4BJtV4Zcor3zfJCjKXzM7blKPiEVVhgIdBw?= =?us-ascii?Q?8Zq5eGbAe4VGHBrclSXPSZVMaJ2WaBRsFXXcFGDKkJzpNAwPcjWpB7pbqwTZ?= =?us-ascii?Q?EAeQHG9TLApzbCADZfDmnLRTqRKla+bE1hA+xC3aptRVU+6QyOtX8XXsX2gJ?= =?us-ascii?Q?+Ej1cfVML2OeXkJA8qV2LBE5bibxCPxLS/khJGnsqx6rFmmGPd2cvERjuhEr?= =?us-ascii?Q?vPaBOipJ6G14yIhz4rIiC+RFrCrQ3sRe5uVqCwtJBPqRzuifsd8FDwbFWZwp?= =?us-ascii?Q?3MUH4tnY5KYHu1OArgjMXxat0g+zx9ww0sevPQoOXlTJtsbH6DL64w9QCSfG?= =?us-ascii?Q?M+nLC8q7A9dXuQ+jA8nIHM9/FCOQTE/yw8ob/Jub6lIRmuWHX6nokjtGw0pF?= =?us-ascii?Q?cfvvadxyI3bWNnYbwXsgd1vbE04fa2D0mPmmyyWQ/040ZbYxHA0Y3w2FqT59?= =?us-ascii?Q?mYvhrb3kkk4zVUm3K3RovH9Pnbq9nmWzS3RCWn68aYI=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: edf3e4ff-f1d1-4ed9-a056-08d8b25cc25e X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Jan 2021 16:04:33.9352 (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: FCzKJrj/RzJFBiUPlKIfzDBQq4LhQNtAzZ3hn1LUJIenra19Rg9c5o5NHDipxC5PqOy4Wmf9+/vh4brTFExS7A== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB5005 X-OriginatorOrg: Nvidia.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1609949082; bh=e/OK6q6ubS7HAcyUxMA8QkTQzc4QPR4JJuPEwzOKC28=; 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=kORf3Ka5umrFhKf9tuCM/CDZYgJ9CnkZ+tnNF7SdQpkfh4NuMMF2PrhN6kdkDhDoF ieGXU1Kb9P3vkNZxyFd2qjHIwQeax9rqGO6jD6slgcf74Ip+dTd+pktgP4pe1MGRfU QPbNS3sDoPN5hxFLKfGwfqnaqGwZ0VirT2dzstGEhqAivXsm4cS7so2JOYmWPoVeZt 5i4odUHEu+odbe+zxPv13e8CiwXysIlEDBZ2hszjgq53Lto9vgUe67ftctd/1eSDEN qA767zpL7nZGKgU4lqXWPpp0lNKQvixl8olQt7LUqd3p+4H6onEzFMnT7VKjZVmIkT x8VHmkD77KBwQ== Subject: Re: [dpdk-dev] [PATCH v7 2/2] eal: add generic thread-local-storage functions 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" > Subject: Re: [PATCH v7 2/2] eal: add generic thread-local-storage functio= ns >=20 > External email: Use caution opening links or attachments >=20 >=20 > On Tue, 5 Jan 2021 19:06:35 +0200, Tal Shnaiderman wrote: > > Add support for TLS functionality in EAL. > > > > The following functions are added: > > rte_thread_tls_key_create - function to create a TLS data key. > > rte_thread_tls_key_delete - function to delete a TLS data key. > > rte_thread_tls_value_set - function to set value bound to the TLS key > > rte_thread_tls_value_get - function to get value bound to the TLS key >=20 > "Function to" is redundant both here and in Doxygen comments. >=20 > > TLS key will be defined by the new type rte_tls_key. >=20 > Please use present tense when describing patch content. >=20 > > The API Allocates the thread local storage (TLS) key. >=20 > Typo: "allocates". >=20 > > Any thread of the process can subsequently use this key to store and > > retrieve values that are local to the thread. > > > > Those functions are added in addition to TLS capability in > > rte_per_lcore.h to allow user to use the thread reasouce destruction > > capability in rte_thread_tls_key_create. >=20 > Err, I assumed we're abstracting pthread, otherwise the capability has > already been there. Also, a typo: "resource". >=20 > > Windows implementation is under librte_eal/windows and implemented > > using WIN32 API for Windows only. > > > > common implementation is under librte_eal/common and implemented > using > > pthread for UNIX compilation. >=20 > librte_eal/unix >=20 > [...] > > +/** > > + * Opaque pointer for TLS key. > > + */ > > +typedef struct eal_tls_key *rte_tls_key; >=20 > Suggesting "TLS key type, an opaque pointer". Now it's unclear if there's= a > TLS key and a pointer to it or TLS key is an opaque pointer itself (which= is the > case, API-wise). >=20 > > + > > /** > > * Set core affinity of the current thread. > > * Support both EAL and non-EAL thread and update TLS. > > @@ -36,4 +41,67 @@ int rte_thread_set_affinity(rte_cpuset_t *cpusetp); > > */ > > void rte_thread_get_affinity(rte_cpuset_t *cpusetp); > > > > +/** > > + * Function to create a TLS data key visible to all threads in the > > +process > > + * function need to be called once to create a key usable by all threa= ds. >=20 > The sentences repeat each other and are not separated. >=20 > > + * rte_tls_key is an opaque pointer used to store the allocated key. > > + * which is later used to get/set a value. > > + * and optional destructor can be set to be called when a thread expir= es. >=20 > "expires" -> "exits", here and below. >=20 > No need to repeat parameter description in function description. >=20 > > + * > > + * @param key > > + * Pointer to store the allocated rte_tls_key > > + * @param destructor > > + * The function to be called when the thread expires. > > + * Ignored on Windows OS. > > + * > > + * @return > > + * On success, zero. > > + * On failure, a negative number. > > + */ > > + > > +__rte_experimental > > +int rte_thread_tls_key_create(rte_tls_key *key, void > > +(*destructor)(void *)); >=20 > > + > > +/** > > + * Function to delete a TLS data key visible to all threads in the > > +process > > + * rte_tls_key is the opaque pointer allocated by > rte_thread_tls_key_create. > > + * > > + * @param key > > + * The rte_tls_key will contain the allocated key >=20 > "Will"? It's an rte_tls_key key allocated by rte_thread_tls_key_create(). >=20 > > + * > > + * @return > > + * On success, zero. > > + * On failure, a negative number. > > + */ > > +__rte_experimental > > +int rte_thread_tls_key_delete(rte_tls_key key); >=20 > > +/** > > + * Function to set value bound to the tls key on behalf of the > > +calling thread >=20 > "tls" -> "TLS" >=20 > > + * > > + * @param key > > + * The rte_tls_key key allocated by rte_thread_tls_key_create. > > + * @param value > > + * The value bound to the rte_tls_key key for the calling thread. > > + * > > + * @return > > + * On success, zero. > > + * On failure, a negative number. > > + */ > > +__rte_experimental > > +int rte_thread_tls_value_set(rte_tls_key key, const void *value); > > + > > +/** > > + * Function to get value bound to the tls key on behalf of the > > +calling thread >=20 > "tls" -> "TLS" >=20 > > + * > > + * @param key > > + * The rte_tls_key key allocated by rte_thread_tls_key_create. > > + * > > + * @return > > + * On success, value data pointer (can also be NULL). > > + * On failure, NULL and an error number is set in rte_errno. > > + */ > > +__rte_experimental > > +void *rte_thread_tls_value_get(rte_tls_key key); > > + > > #endif /* _RTE_THREAD_H_ */ >=20 > > diff --git a/lib/librte_eal/windows/rte_thread.c > > b/lib/librte_eal/windows/rte_thread.c > > new file mode 100644 > > index 0000000000..4ced283c62 > > --- /dev/null > > +++ b/lib/librte_eal/windows/rte_thread.c > > @@ -0,0 +1,83 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright 2021 Mellanox Technologies, Ltd */ > > + > > +#include > > +#include > > +#include > > +#include > > + > > +struct eal_tls_key { > > + DWORD thread_index; > > +}; > > + > > +int > > +rte_thread_tls_key_create(rte_tls_key *key, > > + __rte_unused void (*destructor)(void *)) { > > + *key =3D malloc(sizeof(struct eal_tls_key)); >=20 > sizeof(*key) >=20 > > + if ((*key) =3D=3D NULL) { > > + RTE_LOG(DEBUG, EAL, "Cannot allocate tls key."); >=20 > "tls" -> "TLS" >=20 > > + return -1; > > + } > > + (*key)->thread_index =3D TlsAlloc(); > > + if ((*key)->thread_index =3D=3D TLS_OUT_OF_INDEXES) { > > + RTE_LOG_WIN32_ERR("TlsAlloc()"); > > + free(*key); > > + return -1; > > + } > > + return 0; > > +} > > + > > +int > > +rte_thread_tls_key_delete(rte_tls_key key) { > > + if (!key) { > > + RTE_LOG(DEBUG, EAL, "invalid tls key passed to > > +function.\n"); >=20 > "tls" -> "TLS" > Messages should start with a capital letter. > To which "function"? >=20 > > + return -1; > > + } > > + if (!TlsFree(key->thread_index)) { > > + RTE_LOG_WIN32_ERR("TlsFree()"); > > + free(key); > > + return -1; > > + } > > + free(key); > > + return 0; > > +} Thanks for the review Dmitry, I'll fix and resend both patches.