Re: help with pthread_t deprecation / api changes

[Tyler Retzlaff <roretzla@linux.microsoft.com>]

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@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@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).

great discussion, please let me know your thoughts and what direction
we'd like to go i'll adjust patches as appropriate.

thanks.