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 AA924A0547; Mon, 21 Jun 2021 01:53:41 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 24FB140040; Mon, 21 Jun 2021 01:53:41 +0200 (CEST) Received: from mail-lf1-f48.google.com (mail-lf1-f48.google.com [209.85.167.48]) by mails.dpdk.org (Postfix) with ESMTP id 2123C4003F for ; Mon, 21 Jun 2021 01:53:40 +0200 (CEST) Received: by mail-lf1-f48.google.com with SMTP id h15so8525786lfv.12 for ; Sun, 20 Jun 2021 16:53:40 -0700 (PDT) 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=5lPZaaTQ+RouGTXDkuKb965cagPU56iP6kvD/MCkZ/0=; b=L8p7O9KhIMb+swh2Y9tFUJ6ugZdxlMWnREcm6Ve/WAzXnaDHHhCKQ1f3rBIN5nAkRL Ya1PZjq+RWXey8PbAVf6SvjQ3dIGG5lyTsxUNM1Ga+wZwqUR7p/grifWMcmktvpJ2849 k89UP2Q55dJfIDJEcwFxGsmk8h72FtZKqkfn+E+8xVJ9irq2fok9SjE58sXdobOTNxgN bU+eMS8y3XjEK4Az1TOBgDwzAhpDRfWXvkh+22XH+8BkZuf7uqAsJyIOlqReZ/l+UOH6 hdukyxePrkTydmkZ4zb9vKQeTTItjEO35SiLznA1w+rb+BwQwUFb2PGFxR0Z19IhONUV x0/A== 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=5lPZaaTQ+RouGTXDkuKb965cagPU56iP6kvD/MCkZ/0=; b=kWHSVrVpV8r1YaHwwBvK/eG5tw1W/xyLHSmkYrSNmg9aLUhalV7iBSN7ROUUBXVCo9 O00WLoElGaKK2DccM3jgjL1C/L4sPBoYscOP1Up8k8Mwl+mA2ve9KPUCwcskq8l7U6MO VS2KERFdnCczxMpneXeU/dqeF8PdO/XqoZv18IeCC4AtN3eLNwsZzkmQH6AxLkUUPwT7 GgajiZD13l2dazg/o34kg721mVYoRB2YSDdpxqAg6m9eWQln0HUKs5ldt+JTnDl96e4L +yRXaSRbIUosyRtj9GXalxFzDVAgAsWjjj9e+dBgYXbGSOlB6JSpSI+9UbpDRz9kMEdv xC4g== X-Gm-Message-State: AOAM533oyrluUtAy0yeo5h+wQtSz3WM67TjSlsAOaEfBUs/Bf1faN8Hj Wb2rU6wOgTZK8TCcslofd4I= X-Google-Smtp-Source: ABdhPJzxfZINe2q+EHb6YlDLHrva42CigPzc5x2nNPks6+e7jGdEbvUt6UfRrxay6nvDIg1vt7BEGg== X-Received: by 2002:a19:910f:: with SMTP id t15mr4472937lfd.648.1624233219492; Sun, 20 Jun 2021 16:53:39 -0700 (PDT) Received: from sovereign (broadband-37-110-65-23.ip.moscow.rt.ru. [37.110.65.23]) by smtp.gmail.com with ESMTPSA id x16sm1675069lfa.244.2021.06.20.16.53.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 20 Jun 2021 16:53:38 -0700 (PDT) Date: Mon, 21 Jun 2021 02:53:36 +0300 From: Dmitry Kozlyuk To: Narcisa Ana Maria Vasile Cc: dev@dpdk.org, thomas@monjalon.net, khot@microsoft.com, navasile@microsoft.com, dmitrym@microsoft.com, roretzla@microsoft.com, talshn@nvidia.com, ocardona@microsoft.com, bruce.richardson@intel.com, david.marchand@redhat.com, pallavi.kadam@intel.com Message-ID: <20210621025336.3cdde7fa@sovereign> In-Reply-To: <1624067878-2130-2-git-send-email-navasile@linux.microsoft.com> References: <1624053294-31255-1-git-send-email-navasile@linux.microsoft.com> <1624067878-2130-1-git-send-email-navasile@linux.microsoft.com> <1624067878-2130-2-git-send-email-navasile@linux.microsoft.com> X-Mailer: Claws Mail 3.17.8 (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 v2 1/6] eal: add function that sets thread name 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" 2021-06-18 18:57 (UTC-0700), Narcisa Ana Maria Vasile: > From: Narcisa Vasile > > Implement function that sets the name of a thread. > On Windows, SetThreadDescription() is used. Use GetProcAddress() > to obtain the address of the function for MinGW compatibility. > > Depends-on: series-17402 ("eal: Add EAL API for threading") > > Signed-off-by: Narcisa Vasile > --- > lib/eal/common/rte_thread.c | 17 ++++++++++ > lib/eal/include/rte_thread.h | 18 +++++++++++ > lib/eal/version.map | 1 + > lib/eal/windows/rte_thread.c | 60 ++++++++++++++++++++++++++++++++++++ > 4 files changed, 96 insertions(+) [...] > diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h > index 40da83467b..c65cfd8c9e 100644 > --- a/lib/eal/include/rte_thread.h > +++ b/lib/eal/include/rte_thread.h > @@ -24,6 +24,8 @@ extern "C" { > > #include > > +#define RTE_THREAD_MAX_DESCRIPTION_LENGTH 16 > + Why export this constant? > /** > * Thread id descriptor. > */ > @@ -439,6 +441,22 @@ int rte_thread_barrier_wait(rte_thread_barrier *barrier); > __rte_experimental > int rte_thread_barrier_destroy(rte_thread_barrier *barrier); > > +/** > + * Set the name of the thread represented by 'thread_id'. > + * > + * @param thread_id > + * The id of the thread. > + * > + * @param name > + * Thread name to set. > + * > + * @return > + * On success, return 0. > + * On failure, return a positive errno-style error number. Typo: extra space. > + */ > +__rte_experimental > +int rte_thread_name_set(rte_thread_t thread_id, const char *name); > + There is `rte_thread_setname(pthread_t id, const char * name, size_t len)`. I assume it should be deprecated in favor of this new API via a notice in `deprecation.rst`. > /** > * Create a TLS data key visible to all threads in the process. > * the created key is later used to get/set a value. > diff --git a/lib/eal/version.map b/lib/eal/version.map > index 6645f60a78..2a566c04af 100644 > --- a/lib/eal/version.map > +++ b/lib/eal/version.map > @@ -443,6 +443,7 @@ EXPERIMENTAL { > rte_thread_barrier_init; > rte_thread_barrier_wait; > rte_thread_barrier_destroy; > + rte_thread_name_set; > }; > > INTERNAL { > diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c > index b2ff16f51f..995ae2491d 100644 > --- a/lib/eal/windows/rte_thread.c > +++ b/lib/eal/windows/rte_thread.c > @@ -556,6 +556,66 @@ rte_thread_barrier_destroy(rte_thread_barrier *barrier) > return 0; > } > > +typedef HRESULT > +(*SetThreadDescription_type)(HANDLE thread_handle, PCWSTR thread_description); > + > +int > +rte_thread_name_set(rte_thread_t thread_id, const char *name) > +{ > + int ret = 0; > + size_t count; > + HRESULT hr; > + HANDLE thread_handle = NULL; > + WCHAR w_name[RTE_THREAD_MAX_DESCRIPTION_LENGTH]; > + HMODULE kernel_lib = NULL; > + SetThreadDescription_type SetThreadDescription_ptr; > + > + static const char library_name[] = "kernel32.dll"; > + static const char function[] = "SetThreadDescription"; > + > + kernel_lib = LoadLibraryA(library_name); > + if (kernel_lib == NULL) { > + ret = thread_log_last_error("LoadLibraryA(\"kernel32.dll\")"); > + goto cleanup; > + } Rather then locate the function every time (kernel32.dll is always loaded), what do you think of using `RTE_INIT`/`RTE_FINI` for that? > + > + SetThreadDescription_ptr = (SetThreadDescription_type)( > + (void *)GetProcAddress(kernel_lib, function)); > + if (SetThreadDescription_ptr == NULL) { > + ret = thread_log_last_error("GetProcAddress(\"kernel32.dll\", \"SetThreadDescription\")"); > + goto cleanup; > + } > + > + thread_handle = OpenThread(THREAD_SET_LIMITED_INFORMATION, FALSE, > + thread_id.opaque_id); > + if (thread_handle == NULL) { > + ret = thread_log_last_error("OpenThread()"); > + goto cleanup; > + } > + > + count = mbstowcs(w_name, name, RTE_THREAD_MAX_DESCRIPTION_LENGTH); It's better to use `RTE_DIM(w_name)`, this way named constant is not needed. > + if (count < 0) { > + RTE_LOG(DEBUG, EAL, "Invalid thread name!\n"); > + ret = EINVAL; > + goto cleanup; > + } > + > + hr = SetThreadDescription_ptr(thread_handle, w_name); > + if (FAILED(hr)) { > + ret = thread_log_last_error("SetThreadDescription()"); > + goto cleanup; > + } > + > +cleanup: > + if (kernel_lib != NULL) > + FreeLibrary(kernel_lib); > + if (thread_handle != NULL) { > + CloseHandle(thread_handle); > + thread_handle = NULL; Such local variable assignments on cleanup are useless. > + } > + return ret; > +} > + > int > rte_thread_key_create(rte_thread_key *key, > __rte_unused void (*destructor)(void *))