[lttng-dev] [RFC PATCH v2 01/13] Extend health thread lifetime
Jérémie Galarneau
jeremie.galarneau at efficios.com
Thu Dec 14 01:54:08 UTC 2017
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
<jonathan.rajotte-julien at efficios.com> wrote:
> Allow easier control over the thread by providing a dedicated quit pipe.
over the health thread's lifetime
>
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
> ---
> 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
More information about the lttng-dev
mailing list