Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Staging/unix thread names #1238

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Staging/unix thread names #1238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d2b407e
iiod: thread-pool: make sure to include iio-config.h
nunojsa Jan 31, 2025
267b9ee
iiod: network: shorten network main thread name
nunojsa Jan 30, 2025
61c8573
iiod: responder: shorten the thread names
nunojsa Jan 30, 2025
341b435
buffer: shorten the thread name
nunojsa Jan 30, 2025
1683b4d
iiod-responder: shorten the thread names
nunojsa Jan 30, 2025
ba7083c
cmake: add an Utilities library
nunojsa Jan 31, 2025
a4437a1
lock: support naming threads
nunojsa Jan 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,13 @@ else ()
endif()

target_link_libraries(iio PRIVATE ${PTHREAD_LIBRARIES})

include(cmake/Utilities.cmake)

CHECK_PTHREAD_SET_NAME(HAS_PTHREAD_SETNAME_NP)
if (HAS_PTHREAD_SETNAME_NP AND ${CMAKE_SYSTEM_NAME} MATCHES "Linux")
set_source_files_properties(lock.c PROPERTIES COMPILE_FLAGS -D_GNU_SOURCE)
endif()
endif()

target_sources(iio PRIVATE lock.c)
Expand Down
2 changes: 1 addition & 1 deletion buffer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ iio_device_create_buffer(const struct iio_device *dev, unsigned int idx,
goto err_free_mask;

buf->worker = iio_task_create(iio_buffer_enqueue_worker, NULL,
"iio_buffer_enqueue_worker");
"enqueue-worker");
err = iio_err(buf->worker);
if (err < 0)
goto err_free_mutex;
Expand Down
11 changes: 11 additions & 0 deletions cmake/Utilities.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
function(check_pthread_set_name HAS_PTHREAD)
include(CheckSymbolExists)
set(CMAKE_REQUIRED_LIBRARIES ${PTHREAD_LIBRARIES})
set(CMAKE_REQUIRED_DEFINITIONS -D_GNU_SOURCE)
set(TMP_FLAGS "${CMAKE_C_FLAGS}")
set(CMAKE_C_FLAGS "")
check_symbol_exists(pthread_setname_np "pthread.h" ${HAS_PTHREAD})
set(CMAKE_C_FLAGS "${TMP_FLAGS}")
set(CMAKE_REQUIRED_LIBRARIES)
set(CMAKE_REQUIRED_DEFINITIONS)
endfunction()
4 changes: 2 additions & 2 deletions iiod-responder.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,14 +670,14 @@ iiod_responder_create(const struct iiod_responder_ops *ops, void *d)
goto err_free_lock;

priv->write_task = iio_task_create(iiod_responder_write, priv,
"iiod-responder-writer-task");
"writer-task");
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");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@nunojsa nunojsa Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

err = iio_err(priv->read_thrd);
if (err)
goto err_free_write_task;
Expand Down
11 changes: 2 additions & 9 deletions iiod/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
cmake_minimum_required(VERSION 3.10)
project(iiod C)

include(CheckSymbolExists)
set(CMAKE_REQUIRED_LIBRARIES ${PTHREAD_LIBRARIES})
set(CMAKE_REQUIRED_DEFINITIONS -D_GNU_SOURCE)
set(TMP_FLAGS "${CMAKE_C_FLAGS}")
set(CMAKE_C_FLAGS "")
check_symbol_exists(pthread_setname_np "pthread.h" HAS_PTHREAD_SETNAME_NP)
set(CMAKE_C_FLAGS "${TMP_FLAGS}")
set(CMAKE_REQUIRED_LIBRARIES)
set(CMAKE_REQUIRED_DEFINITIONS)
include(${CMAKE_SOURCE_DIR}/cmake/Utilities.cmake)
CHECK_PTHREAD_SET_NAME(HAS_PTHREAD_SETNAME_NP)

add_executable(iiod
iiod.c interpreter.c responder.c rw.c thread-pool.c
Expand Down
2 changes: 1 addition & 1 deletion iiod/network.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ int start_network_daemon(struct iio_context *ctx,
goto err_free_pdata;
}

err = thread_pool_add_thread(pool, network_main, pdata, "network_main_thd");
err = thread_pool_add_thread(pool, network_main, pdata, "net_main_thd");
if (!err)
return 0;

Expand Down
6 changes: 3 additions & 3 deletions iiod/responder.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,13 +410,13 @@ static void handle_create_buffer(struct parser_pdata *pdata,
}

entry->enqueue_task = iio_task_create(buffer_enqueue_block, entry,
"buffer-enqueue-thd");
"buffer-enqueue");
ret = iio_err(entry->enqueue_task);
if (ret)
goto err_free_mask;

entry->dequeue_task = iio_task_create(buffer_dequeue_block, entry,
"buffer-dequeue-thd");
"buffer-dequeue");
ret = iio_err(entry->dequeue_task);
if (ret)
goto err_destroy_enqueue_task;
Expand Down Expand Up @@ -970,7 +970,7 @@ static void handle_create_evstream(struct parser_pdata *pdata,
}

entry->task = iio_task_create(evstream_read, entry,
"evstream-read-thd");
"evstream-read");
ret = iio_err(entry->task);
if (ret) {
iio_event_stream_destroy(entry->stream);
Expand Down
1 change: 1 addition & 0 deletions iiod/thread-pool.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Author: Paul Cercueil <[email protected]>
*/

#include "iio-config.h"
#include "thread-pool.h"

#include <errno.h>
Expand Down
25 changes: 24 additions & 1 deletion lock.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,24 @@ struct iio_cond {
struct iio_thrd {
pthread_t thid;
void *d;
/*
* pthread_setname_np(3) does not allow names bigger than 16
* (at least on linux).
*/
char name[16];
int (*func)(void *);
};

#if defined(HAS_PTHREAD_SETNAME_NP)
#if defined(__APPLE__)
#define iio_thrd_create_set_name(thid, name) pthread_setname_np(name)
#else
#define iio_thrd_create_set_name(thid, name) pthread_setname_np(thid, name)
#endif
#else
#define iio_thrd_create_set_name(thid, name) 0
#endif

struct iio_mutex * iio_mutex_create(void)
{
struct iio_mutex *lock = malloc(sizeof(*lock));
Expand Down Expand Up @@ -107,6 +122,12 @@ void iio_cond_signal(struct iio_cond *cond)
static void * iio_thrd_wrapper(void *d)
{
struct iio_thrd *thrd = d;
/*
* For Mac, it seems we need to name the thread from the thread
* itself.
*/
if (thrd->name[0] != '\0')
iio_thrd_create_set_name(thrd->thid, thrd->name);

return (void *)(intptr_t) thrd->func(thrd->d);
}
Expand All @@ -126,6 +147,8 @@ struct iio_thrd * iio_thrd_create(int (*thrd)(void *),

iio_thrd->d = d;
iio_thrd->func = thrd;
if (name)
iio_strlcpy(iio_thrd->name, name, sizeof(iio_thrd->name));

ret = pthread_create(&iio_thrd->thid, NULL,
iio_thrd_wrapper, iio_thrd);
Expand All @@ -134,7 +157,7 @@ struct iio_thrd * iio_thrd_create(int (*thrd)(void *),
return iio_ptr(ret);
}

/* TODO: Set name */


return iio_thrd;
}
Expand Down