Re: help with pthread_t deprecation / api changes

[Thomas Monjalon <thomas@monjalon.net>]

02/12/2022 20:57, Tyler Retzlaff:
> On Fri, Dec 02, 2022 at 09:03:25AM +0100, Morten Brørup wrote:
> > +Bruce, FreeBSD EAL maintainer
> > 
> > 
> > <rant>
> > This is one of the consequences of a bloated EAL.
> > 
> > How is an application supposed to run on top of an EAL that isn't fully implemented by the underlying environments?
> 
> yep, i'm aware of your other thread and i am in complete agreement with
> you. i think as a rule of thumb if we can't reasonably provide an
> abstraction that behaves consistently then it isn't something that
> should be in the eal at all.
> 
> unfortunately i'm dealing with things that pre-date that enlightenment.
> 
> > I have complained about this before, but am not afraid to repeat it:
> > I wish the EAL wasn't treated as some catch-all library where it is easy to throw in new features, which really belong in separate libraries!
> > </rant>
> > 
> > > 
> > > current status.
> > > 
> > > rte_thread_getname
> > >   * freebsd, no implementation and it appears no possible support
> > >   * linux, implementation conditional on __GLIBC_PREREQ(2, 12)
> > >   * windows, can be implemented but isn't, noop success
> > >   * fortunately is marked __rte_experimental
> > >   * called in 1 place only from eal (logging)
> > > 
> > > i would propose to present a consistent abstraction the best thing to
> > > do
> > > here is just remove rte_thread_getname. providing a version that
> > > requires an application to do conditional dances / compilation based on
> > > platform gains nothing.
> > 
> > My initial thought was that it should be provided for symmetry reasons.
> > However, thinking about it, it is probably only used for debugging, trace, and similar. It is probably not used in any meaningful way by applications. So I won't oppose to removing it.
> > 
> > Alternatively:
> > 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?

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?

> right now it is trying to be both and the both is trying to build on
> unaligned platform facilities. one thing we could do here is decouple
> the two.
> 
> if the purpose was to allow a name to be get,set by the application it
> could just be stored with dpdk lcore state and does not need to be fed
> into pthread_{set,get}name_np.  it can continue to be consumed by
> logging/the application.

In rte_ctrl_thread_create() the name is provided and limited to 16.

> extending that we could say there is a
> 'side-effect' that internally if your platform does support
> pthread_{set,get}name_np then it will be called as well, but if it fails
> we don't care.
> 
> > 
> > > 
> > > rte_thread_setname
> > >   * freebsd, implemented, imposes no limit on name length, suppresses
> > > errors
> > >   * linux, implementation conditional on __GLIBC_PREREQ(2, 12), imposes
> > >     limit of 16 (including terminating NUL) on name length, may return
> > > an
> > >     error
> > >   * windows, can be implemented, no explicit limit on name length, may
> > >     return errors
> > >   * unfortunately not marked __rte_experimental
> > > 
> > > i would propose to provide a replacement with the name
> > > rte_thread_set_name with more consistent behavior across the 3
> > > platforms.
> > > 
> > >   * returns errors for platforms that return errors, but the caller
> > >     is free to ignore them.
> > >   * explicit limit of 16 (including terminating NUL) on name length,
> > >     names that are provided that exceed the limit are truncated without
> > >     error.
> > 
> > The function should not silently modify the provided data (i.e. truncate the name). I would prefer doing both: Return ERANGE (like pthread_setname_np()), but use the truncated name anyway. Then the application can choose to ignore or deal with the return code.
> 
> i agree with this, but the current linux implementation is already doing
> it, i'd be inclined to say if we have a limit we should just fail the
> set.
> 
> i'm beginning to think that complete removal of set,get thread name is
> what we want here and if we need names for things like logging we
> provide a facility not tied to the underlying platform threading.
> 
> others feel free to chime in.
> 
> thanks!
>