Hi, I rebased this patch set on master.
https://github.com/jgalar/lttng-tools/tree/teardown-rebased Most patches still applied cleanly, but git really messed this first one up when rebasing. The patch applied and the code compiled, but I realized hunks from thread_manage_health() and thread_manage_clients() ended up mixed-up... *Fun* times. See comments below and on the other patches. Thanks! Jérémie On 18 September 2017 at 18:51, Jonathan Rajotte <[email protected]> wrote: > Allow easier control over the thread by providing a dedicated quit pipe. over the health thread's lifetime > > Signed-off-by: Jonathan Rajotte <[email protected]> > --- > src/bin/lttng-sessiond/main.c | 74 > ++++++++++++++++++++++++++++++++----------- > 1 file changed, 55 insertions(+), 19 deletions(-) > > diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c > index 03f695ec..5d7df744 100644 > --- a/src/bin/lttng-sessiond/main.c > +++ b/src/bin/lttng-sessiond/main.c > @@ -204,6 +204,7 @@ static int kernel_poll_pipe[2] = { -1, -1 }; > * for all threads when receiving an event on the pipe. > */ > static int thread_quit_pipe[2] = { -1, -1 }; > +static int thread_health_teardown_trigger_pipe[2] = { -1, -1 }; > > /* > * This pipe is used to inform the thread managing application communication > @@ -477,6 +478,11 @@ static int init_thread_quit_pipe(void) > return __init_thread_quit_pipe(thread_quit_pipe); > } > > +static int init_thread_health_teardown_trigger_pipe(void) > +{ > + return __init_thread_quit_pipe(thread_health_teardown_trigger_pipe); > +} > + > /* > * Stop all threads by closing the thread quit pipe. > */ > @@ -4297,11 +4303,13 @@ static void *thread_manage_health(void *data) > goto error; > } > > - /* > - * Pass 2 as size here for the thread quit pipe and client_sock. > Nothing > - * more will be added to this poll set. > - */ > - ret = sessiond_set_thread_pollset(&events, 2); > + ret = lttng_poll_create(&events, 2, LTTNG_CLOEXEC); Please add a comment about the "2" being used here. > + if (ret < 0) { > + goto error; > + } > + > + /* Add teardown trigger */ > + ret = lttng_poll_add(&events, thread_health_teardown_trigger_pipe[0], > LPOLLIN | LPOLLERR); > if (ret < 0) { > goto error; > } > @@ -4343,10 +4351,18 @@ restart: > } > > /* Thread quit pipe has been closed. Killing thread. > */ > - ret = sessiond_check_thread_quit_pipe(pollfd, > revents); > - if (ret) { > - err = 0; > - goto exit; > + if (pollfd == thread_health_teardown_trigger_pipe[0]) > { > + if (revents & LPOLLIN) { > + /* Normal exit */ > + err = 0; > + goto exit; > + } else if (revents & LPOLLERR) { > + ERR("Health teardown poll error"); > + goto error; > + } else { > + ERR("Unexpected poll events %u for > teardown sock", revents); teardown socket -> pipe > + goto error; > + } > } > > /* Event on the registration socket */ > @@ -4412,8 +4428,13 @@ restart: > } > } > > -exit: > error: > + /* > + * Perform stop_thread only on error path since in a normal teardown > the > + * health thread is in the last thread to terminate. is the last thread to terminate > + */ > + stop_threads(); > +exit: > if (err) { > ERR("Health error occurred in %s", __func__); > } > @@ -4425,9 +4446,7 @@ error: > PERROR("close"); > } > } > - > lttng_poll_clean(&events); > - stop_threads(); > rcu_unregister_thread(); > return NULL; > } > @@ -5631,6 +5650,7 @@ int main(int argc, char **argv) > *ust64_channel_monitor_pipe = NULL, > *kernel_channel_monitor_pipe = NULL; > bool notification_thread_running = false; > + bool health_thread_running = false; > > init_kernel_workarounds(); > > @@ -5717,6 +5737,11 @@ int main(int argc, char **argv) > goto exit_init_data; > } > > + if (init_thread_health_teardown_trigger_pipe()) { > + retval = -1; > + goto exit_init_data; > + } > + > /* Check if daemon is UID = 0 */ > is_root = !getuid(); > > @@ -6119,6 +6144,7 @@ int main(int argc, char **argv) > retval = -1; > goto exit_health; > } > + health_thread_running = true; > > /* notification_thread_data acquires the pipes' read side. */ > notification_thread_handle = notification_thread_handle_create( > @@ -6311,13 +6337,6 @@ exit_dispatch: > > exit_client: > exit_notification: > - ret = pthread_join(health_thread, &status); > - if (ret) { > - errno = ret; > - PERROR("pthread_join health thread"); > - retval = -1; > - } > - > exit_health: > exit_init_data: > /* > @@ -6359,6 +6378,20 @@ exit_init_data: > > notification_thread_handle_destroy(notification_thread_handle); > } > > + if (health_thread_running) { > + ret = > notify_thread_pipe(thread_health_teardown_trigger_pipe[1]); > + if (ret < 0) { > + ERR("write error on thread quit pipe"); > + } > + > + ret = pthread_join(health_thread, &status); > + if (ret) { > + errno = ret; > + PERROR("pthread_join health thread"); > + retval = -1; > + } > + } > + > rcu_thread_offline(); > rcu_unregister_thread(); > > @@ -6366,6 +6399,9 @@ exit_init_data: > if (ret) { > retval = -1; > } > + > + /* Health thread teardown pipe cleanup */ > + utils_close_pipe(thread_health_teardown_trigger_pipe); > lttng_pipe_destroy(ust32_channel_monitor_pipe); > lttng_pipe_destroy(ust64_channel_monitor_pipe); > lttng_pipe_destroy(kernel_channel_monitor_pipe); > -- > 2.11.0 > -- Jérémie Galarneau EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list [email protected] https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
