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 9294EA0540; Tue, 13 Dec 2022 20:34:12 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 79FB940395; Tue, 13 Dec 2022 20:34:12 +0100 (CET) Received: from wout3-smtp.messagingengine.com (wout3-smtp.messagingengine.com [64.147.123.19]) by mails.dpdk.org (Postfix) with ESMTP id 711F54021D for ; Tue, 13 Dec 2022 20:34:11 +0100 (CET) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id B16FA320092F; Tue, 13 Dec 2022 14:34:07 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Tue, 13 Dec 2022 14:34:08 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm2; t=1670960047; x= 1671046447; bh=l973MxLbS5AXjcoLAkSONWSa1N1/oLoDBXjJ9IRdiw4=; b=y 9KoNuzFs7URvE8quPVBCmBtHSoEfQKlnQNIA8N75w9TIEwcPPyUabQbxFniiGi4X sVQX2ejIT+/E9NQWHCcuiCKyV1H0YFPtbE/JC2XH/Vxr+wCz/PTl1JmM+zj0dMo+ sbSvQTgvaQCJxGhlhJbgSgUpyhCDlxEWAmdjTXb9vrwutGAXYlM0LDu2cAkmEJ+b zoK4xnZLMd9A7oBA/mHd56OLqlJAIyFwr2LtafTDeKi+zjVLrBQQIXzOix5ydMzJ fpiyZJTQtutdAholiJbEhfXUk8/GCirN2vcSDqC5wHNT/RBgHXtjA+tuVkOEnboF hKO+tbywgnqE9UZMi34mg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1670960047; x= 1671046447; bh=l973MxLbS5AXjcoLAkSONWSa1N1/oLoDBXjJ9IRdiw4=; b=R BsQRH7gmWkM0Kr+trajI5r3bdEepxrpepv9wbOfkrfWEAJr+foWAC6SmwCtGoCTU GHPpeBr1hEc2y9wh9/cb07FHGYIPEM7iuB/CLO5DYgIVqSeFDb4b4YBQbjMNzysI 2ZOm6sAFVr4hjx/c4b2G9NI5EQJWh7/RFBfKLtsjNDEq2XynSNtMsavN8pL0XPnX L070D2s7aE27aHaaZhbrQZ+7MicuTRCfTQGihFTo9vsg5eYMSg4Yp12OcoPWpE0D KOvNBdbwS9KUIMniU6+baMnhmvxwgxAyO3FAhDtnSWtYCLUJ8OM89Er9PSHgXU3G wWyd4BPMk360cGHG9oehQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrfedugdehfecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvfevufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepjeejffffgfffkeefffelgfekleetjeffleeludeghfehleffteeh veduffdugfdvnecuffhomhgrihhnpeguphgukhdrohhrghenucevlhhushhtvghrufhiii gvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghlohhn rdhnvght X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 13 Dec 2022 14:34:05 -0500 (EST) From: Thomas Monjalon To: Tyler Retzlaff Cc: Stephen Hemminger , dev@dpdk.org, Morten =?ISO-8859-1?Q?Br=F8rup?= , david.marchand@redhat.com, Bruce Richardson Subject: Re: help with pthread_t deprecation / api changes Date: Tue, 13 Dec 2022 20:34:02 +0100 Message-ID: <3905723.ZaRXLXkqSa@thomas> In-Reply-To: <20221213173843.GA8640@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20221130225427.GA13682@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <5885133.UjTJXf6HLC@thomas> <20221213173843.GA8640@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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 13/12/2022 18:38, Tyler Retzlaff: > On Tue, Dec 13, 2022 at 09:32:06AM +0100, Thomas Monjalon wrote: > > 12/12/2022 18:45, Tyler Retzlaff: > > > On Sun, Dec 11, 2022 at 08:50:48AM +0100, Thomas Monjalon wrote: > > > > 10/12/2022 00:49, Tyler Retzlaff: > > > > > On Fri, Dec 09, 2022 at 10:13:44PM +0100, Thomas Monjalon wrote: > > > > > > 09/12/2022 21:06, Tyler Retzlaff: > > > > > > > On Fri, Dec 09, 2022 at 08:48:14AM -0800, Stephen Hemminger wrote: > > > > > > > > On Fri, 09 Dec 2022 08:53:57 +0100 > > > > > > > > Thomas Monjalon wrote: > > > > > > > > > > > > > > > > > > > If some execution environment doesn't support thread names, it could return a string that makes it possible for a human to identify the thread, e.g. the tread id. Again, this is assuming that it is only used for debugging, trace, and similar. > > > > > > > > > > > > > > > > > > > > i think this raises a good question. is the purpose of setting a thread name > > > > > > > > > > meant to be something we can use from the application or is it something that > > > > > > > > > > is for debugging diagnostics and may be a best effort? > > > > > > > > > > > > > > > > > > > > > > > > Thomas Monjalon wrote: > > > > > > > > > > > > > > > I think yes it is only for debugging. > > > > > > > > So best effort looks to be a good approach. > > > > > > > > I'm not sure you need to replace the functions. > > > > > > > > Can you just complete the implementations? > > > > > > > > > > > > > > the patch series put forward allows a set / get name per-lcore, where > > > > > > > you get implicit (but not exposed via the eal api) call to underlying > > > > > > > platform thread setname. > > > > > > > > > > > > I don't understand how lcore ID and thread ID are connected. > > > > > > You can run multiple control threads on a single lcore. > > > > > > > > > > > > > > > > correct. > > > > > > > > > > the new public api allows the set of a name on an lcore only. as a > > > > > side-effect if the platform supports it the name is also set on the > > > > > thread_id associated with the lcore (from lcore_config[].thread_id). > > > > > > > > > > for control threads you just get the side-effect if the platform > > > > > supports it, otherwise it is a noop. > > > > > > > > What does it mean? which side effect? what must be supported? > > > > > > > > > if we want set / get name at all this seemed the most usable balance > > > > > between application consumption with debug use where available. if this > > > > > isn't acceptable then i would suggest we simply remove both > > > > > rte_thread_{get,set}name because as a debugging facility we cannot offer > > > > > a consistent abstracted api which means it shouldn't be in the eal at > > > > > all. > > > > > > > > Why it cannot be consistent? > > > > Please be more precise. > > > > > > > > > > sorry i thought you had been looking at our implementation, let me > > > summarize. > > > > > > here are the differences between the underlying platform capabilities > > > that prohibit both get and set. it's not a matter of just providing missing > > > implementation for a specific platform. > > > > > > set thread name: > > > freebsd > > > - set reports no failure, but may silently fail > > > - uncertain what name length limit is > > > linux > > > - set not available in older glibc > > > - current rte wrapper silently truncates name length > > > windows > > > - set always available > > > - uncertain what name length limit is > > > > > > get thread name: > > > freebsd > > > - not available at all > > > linux > > > - get not available in older glibc > > > - get can fail > > > windows > > > - get always available > > > - get can fail > > > > > > keep in mind the purpose of an abstraction is that the application *does > > > not* have to do conditional evaluation on a per-platform basis around > > > call sites. once you start putting #ifdef RTE_EXEC_ENV_XXX into code you > > > failed and i presume we want none of the use of eal to be adorned with > > > that. > > > > > > at first glance you might think oh, well if get isn't supported then > > > just return some default string or an empty string but even that is a > > > violation of the abstraction that leaks implementation detail. > > > > > > i.e. assuming success set() success get(set()) should also succeed > > > without conditional compilation of the code. > > > > > > the common abstraction that can be reasonably provided explicitly > > > operating a thread is something like. > > > > > > * for set you can provide a watered down version that doesn't report > > > failure and silently truncates and ignores errors within the > > > implementation. if it works it works if it doesn't you just don't > > > know i.e. best effort. > > > * for get it cannot be provided consistently, the platforms simply > > > aren't providing what is needed. > > > > > > for background one of the request to expose the native platform thread > > > id was to access the best effort behavior for the thread associated with > > > an lcore. discussion on list highlighted the constraint that this should > > > be done without exposing platform specific detail via the eal api. > > > > > > http://mails.dpdk.org/archives/dev/2022-October/253411.html > > > > > > as mentioned in a previous mail i provided a series that accomplishes > > > this as a side effect of an api that can be consistently provided when > > > available on the platform, it has 2 benefits. > > > * it does not expose the platform-specific native thread handle. > > > * for lcores it keeps the name in the application memory so it is > > > available in crash/coredumps. > > > > > > so we have 2 options. > > > > > > 1. a watered down set (no get) that is fire and forget and reports no > > > failure and maybe it works, maybe it doesn't depending on your platform. > > > 2. the lcore set / get which is basically (1) for the threads associated > > > with lcores but provides some additional features that are supportable > > > in the api surface. (set/get, stored in application namespace). > > > > Given thread name is not critical at all, I think best effort is OK. > > We could make clear in the API documentation that it is not reliable. > > > > I don't think implementing thread name in the specific case of > > datapath lcore is a real improvement. > > Okay, just one final confirmation. This is what we would like? > > * completely remove the existing rte_thread_getname api. > - by implication this means remove the 1 use of it in eal in > logging. > > * introduce a new void rte_thread_set_name(rte_thread_t, const char *name) > that: > - returns void (does not fail), but in cases it can be detected will > log a DEBUG level log message. > - quietly truncates the name (if longer) to RTE_MAX_THREAD_NAME_LEN on > all platforms. > - document that it is best effort and only works if the stars align > for the target platform. > > * there will be no unit test, since the set doesn't fail and there is no > get to validate the set. > > once i get confirmation i'll update the series. Just my opinion: this proposal is my preference, yes. What others think?