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 2153FA09FF; Wed, 6 Jan 2021 16:05:31 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9C6FB160977; Wed, 6 Jan 2021 16:05:29 +0100 (CET) Received: from mail-lf1-f41.google.com (mail-lf1-f41.google.com [209.85.167.41]) by mails.dpdk.org (Postfix) with ESMTP id 3748716094C for ; Wed, 6 Jan 2021 16:05:28 +0100 (CET) Received: by mail-lf1-f41.google.com with SMTP id a12so7201642lfl.6 for ; Wed, 06 Jan 2021 07:05:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=0ebj6xawGnyxYG/Q6k5It4oK0+UoWHCOws6TuBL5YQk=; b=pmjCJDUYXLM6XIBkG4/n8p5VpfPrJ1x9rSP6XFClxO+SSESjCj7mBOAfWflzDZTMjU YqeeIqkx7+ZABM6sIjPwWs1NlLuJZG+iosoPRbkJI6gkD+cR28V2PhSXkodm4A8NxMTi YDJMyCuAe+ohYSOLNXddCPx5ErmzTteLzqxuNdx6eHNqz+jx1G1G+TKbVxnKBOwNzmS1 Invx36NxAHdjVnPtJSx15PD2hf/Cu7cz7irFX3QX7s4Xlo5Jgq6+8Z13hOOI3bh1hLPt 5ZhKtfQnAhmLxfubbbW/uHPH23hQebNUozf2ONsJTijsAgpoOAUNyd3B5lwAcDHqAbmn J1IQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=0ebj6xawGnyxYG/Q6k5It4oK0+UoWHCOws6TuBL5YQk=; b=Rz+3iZK7jIdHjsKhUIEr5HZkMeQSBrLs1mObRd6TYkR/jfVI1ren1u/yf43B0tdgAt fXWhcHXhXX3Al3ll8+wiNiikbBwLFAM8OvkbgasgDQMaaqQVVcfWkG15FoQLZueeV2rx JP4HUMP4v4p5sswISxTuBwusYNu71uEJ/ZWLJKyt6yWRtZ179DRZUs3vy/iFCaWwP4Lw nHLyT2K8/r/WxZD4FSKHo9qia+HAtRe0f4cEtSusKA6W/hbYlfWUkqOn1yNvAWrdBpJI 9AMwuXavfImI7p8bPo+YX4XIK1rGBr3IoNcW2mmzUkX1shauGeMZIw4skxJbwUhzqWZ4 NwkQ== X-Gm-Message-State: AOAM532+ktcNHBsxt1Aar9HM5RfPHYDHwlQFVuwrf7M7d7ZbUchr6MSO yUHQ32vltAhCWYyouDkB1k8= X-Google-Smtp-Source: ABdhPJzbO+vqkI8jo+CGYkMVvWDWPsktx16M4i0Dh67dsPkbuULU8hMsBdK1pysI8Kz0QQbgEnQ7OQ== X-Received: by 2002:a05:6512:3586:: with SMTP id m6mr1941068lfr.318.1609945527726; Wed, 06 Jan 2021 07:05:27 -0800 (PST) Received: from sovereign (broadband-37-110-65-23.ip.moscow.rt.ru. [37.110.65.23]) by smtp.gmail.com with ESMTPSA id p6sm425003lfh.50.2021.01.06.07.05.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Jan 2021 07:05:26 -0800 (PST) Date: Wed, 6 Jan 2021 18:05:25 +0300 From: Dmitry Kozlyuk To: Tal Shnaiderman Cc: dev@dpdk.org, thomas@monjalon.net, pallavi.kadam@intel.com, navasile@linux.microsoft.com, dmitrym@microsoft.com, david.marchand@redhat.com Message-ID: <20210106180525.3f4dd1a3@sovereign> In-Reply-To: <20210105170635.6212-3-talshn@nvidia.com> References: <20201226160848.9824-1-talshn@nvidia.com> <20210105170635.6212-1-talshn@nvidia.com> <20210105170635.6212-3-talshn@nvidia.com> X-Mailer: Claws Mail 3.17.6 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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" 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 "Function to" is redundant both here and in Doxygen comments. > TLS key will be defined by the new type rte_tls_key. Please use present tense when describing patch content. > The API Allocates the thread local storage (TLS) key. Typo: "allocates". > 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. Err, I assumed we're abstracting pthread, otherwise the capability has already been there. Also, a typo: "resource". > 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. librte_eal/unix [...] > +/** > + * Opaque pointer for TLS key. > + */ > +typedef struct eal_tls_key *rte_tls_key; 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). > + > /** > * 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 threads. The sentences repeat each other and are not separated. > + * 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 expires. "expires" -> "exits", here and below. No need to repeat parameter description in function description. > + * > + * @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 *)); > + > +/** > + * 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 "Will"? It's an rte_tls_key key allocated by rte_thread_tls_key_create(). > + * > + * @return > + * On success, zero. > + * On failure, a negative number. > + */ > +__rte_experimental > +int rte_thread_tls_key_delete(rte_tls_key key); > +/** > + * Function to set value bound to the tls key on behalf of the calling thread "tls" -> "TLS" > + * > + * @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 "tls" -> "TLS" > + * > + * @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_ */ > 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 = malloc(sizeof(struct eal_tls_key)); sizeof(*key) > + if ((*key) == NULL) { > + RTE_LOG(DEBUG, EAL, "Cannot allocate tls key."); "tls" -> "TLS" > + return -1; > + } > + (*key)->thread_index = TlsAlloc(); > + if ((*key)->thread_index == 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"); "tls" -> "TLS" Messages should start with a capital letter. To which "function"? > + return -1; > + } > + if (!TlsFree(key->thread_index)) { > + RTE_LOG_WIN32_ERR("TlsFree()"); > + free(key); > + return -1; > + } > + free(key); > + return 0; > +}