Staging/unix thread names #1238
Conversation
nunojsa
requested review from
cristina-suteu,
dNechita and
mhennerich
as code owners
|
OK, this looks to be not reliable... Not sure if it will be doable at all because it looks like the API just has a different footprint in MAC and that is already discouraging. |
| err = iio_err(priv->write_task); | ||
| if (err) | ||
| goto err_free_io; | ||
|
|
||
| if (!NO_THREADS) { | ||
| priv->read_thrd = iio_thrd_create(iiod_responder_reader_thrd, priv, | ||
| "iiod-responder-reader-thd"); | ||
| "reader-thd"); |
There was a problem hiding this comment.
Given the length constraint, I suggest removing "-thd" since it's already clear that we're naming threads. Instead, those four characters could be used to better describe the thread or distinguish it from others in relevant situations.
There was a problem hiding this comment.
Hmm, it does make sense. I'll see if i can thing on something better... I'm reworking things a bit as I just realized (stupid me) that we do need a "pure" define in here and not something that evaluates to 0 or 1. If the API is not available and place inside an if(0) it will still fail to compile. Dead code removal happens in a later point in time...
There was a problem hiding this comment.
Hmm thinking a bit more I can replace it for something like "job" (we do sabe 1 char). Or "client-reader" but here your argument also applies (we already now it's the client). One thing that could be beneficial would be to differentiate between the clients created on behave of buffer and the main client. But the trouble for that seemed not worth it for me.
nunojsa
force-pushed
the
staging/unix-thread-names
branch
4 times, most recently
from
108b6ad to
ea3eb7b
Compare
If the iio-config header is not properly included, HAS_PTHREAD_SETNAME_NP will never be defined and we we'll never set the thread names. Signed-off-by: Nuno Sá <[email protected]>
pthread_setname_np(3) does not support names bigger than 16 characters, including the terminating null byte, which means it was not properly setting the name of the thread. Signed-off-by: Nuno Sá <[email protected]>
This is in preparation of adding support for naming threads (in unix). This is done since pthread_setname_np(3) does not support names bigger than 16 characters, including the terminating null byte. Signed-off-by: Nuno Sá <[email protected]>
This is in preparation of adding support for naming threads (in unix). This is done since pthread_setname_np(3) does not support names bigger than 16 characters, including the terminating null byte. Signed-off-by: Nuno Sá <[email protected]>
This is in preparation of adding support for naming threads (in unix). This is done since pthread_setname_np(3) does not support names bigger than 16 characters, including the terminating null byte. Signed-off-by: Nuno Sá <[email protected]>
Create a cmake utilities library for functions or macros that will be shared across CMake files. For now, just add a function to check the availability of pthread_setname_np(3). This is in preparation of adding support for thread names where we'll also check this from the top level CMake file. Signed-off-by: Nuno Sá <[email protected]>
Add support for naming threads under Unix. It's done with pthread_setname_np(3) so we need to make sure it's available. On top of that, on linux, we also need to compile lock.c with _GNU_SOURCE. Signed-off-by: Nuno Sá <[email protected]>
nunojsa
force-pushed
the
staging/unix-thread-names
branch
from
ea3eb7b to
a4437a1
Compare
|
Ok, this should be now ready for review. The fact that pthreas_set_name_np() is non portable is far from great but I'm not sure how it can be done in way that tools like ps or top see the names (I mean, I know how I could do it on linux but not on other platforms). I tried anyways to keep things as self contained as possible but it would still be nice to see if this works on MAC (freebsd would also be nice but I guess more difficult) |
|
I will try to do a test on MAC this week. |
PR Description
This adds support for naming threads in Unix (marked as TODO). The first commits are about fixing the names as they cannot be bigger than 16 chars. And one of the patches (on the iiod side) is actually a fix since the name of the main network thread was not being set because of the above.
This is kind of a RFC, since it depends on pthread_setname_np() which is non portable (and needs _GNU_SOURCE to be defined) . Still, it should have no effect on windows and was tested under linux. We also check for the API availability in cmake but it would still be a good thing if someone could test this under MAC.
If it is yet not clear, this only adds support for Unix and does not touches on Windows builds.
PR Type
PR Checklist