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! >