help with pthread_t deprecation / api changes

[Thomas Monjalon]

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 <thomas at monjalon.net> 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 <thomas at monjalon.net> 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.