diff mbox series

[RFC,v2,05/13] Control thread_apps_notify lifetime with specialized quit pipe

Message ID 20170918225206.17725-6-jonathan.rajotte-julien@efficios.com
State Superseded, archived
Delegated to: Jérémie Galarneau
Headers show
Series Sessiond teardown overhaul | expand

Commit Message

Jonathan Rajotte Sept. 18, 2017, 10:51 p.m. UTC
Listen to application longer and quit only when most other thread are
terterminated. This simplifies data interaction with other threads.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
---
 src/bin/lttng-sessiond/lttng-sessiond.h |  2 ++
 src/bin/lttng-sessiond/main.c           | 20 +++++++++++++++++++-
 src/bin/lttng-sessiond/ust-thread.c     | 27 ++++++++++++++++++++-------
 3 files changed, 41 insertions(+), 8 deletions(-)

Comments

Jérémie Galarneau Dec. 14, 2017, 1:55 a.m. UTC | #1
In the subject, replace "specialized" by "dedicated".

On 18 September 2017 at 18:51, Jonathan Rajotte
<jonathan.rajotte-julien at efficios.com> wrote:
> Listen to application longer and quit only when most other thread are

application -> applications

> terterminated. This simplifies data interaction with other threads.

terterminated -> terminated.

>
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
> ---
>  src/bin/lttng-sessiond/lttng-sessiond.h |  2 ++
>  src/bin/lttng-sessiond/main.c           | 20 +++++++++++++++++++-
>  src/bin/lttng-sessiond/ust-thread.c     | 27 ++++++++++++++++++++-------
>  3 files changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/lttng-sessiond.h b/src/bin/lttng-sessiond/lttng-sessiond.h
> index 74552db6..8fbcac60 100644
> --- a/src/bin/lttng-sessiond/lttng-sessiond.h
> +++ b/src/bin/lttng-sessiond/lttng-sessiond.h
> @@ -94,6 +94,8 @@ struct ust_reg_wait_node {
>   */
>  extern int apps_cmd_notify_pipe[2];
>
> +extern int thread_apps_notify_teardown_trigger_pipe[2];
> +
>  /*
>   * Used to notify that a hash table needs to be destroyed by dedicated
>   * thread. Required by design because we don't want to move destroy
> diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
> index 8d9b14b5..3596d7e3 100644
> --- a/src/bin/lttng-sessiond/main.c
> +++ b/src/bin/lttng-sessiond/main.c
> @@ -206,6 +206,7 @@ static int kernel_poll_pipe[2] = { -1, -1 };
>  static int thread_quit_pipe[2] = { -1, -1 };
>  static int thread_health_teardown_trigger_pipe[2] = { -1, -1 };
>  static int thread_apps_teardown_trigger_pipe[2] = { -1, -1 };
> +int thread_apps_notify_teardown_trigger_pipe[2] = { -1, -1 };
>
>  /*
>   * This pipe is used to inform the thread managing application communication
> @@ -489,6 +490,11 @@ static int init_thread_apps_teardown_trigger_pipe(void)
>         return __init_thread_quit_pipe(thread_apps_teardown_trigger_pipe);
>  }
>
> +static int init_thread_apps_notify_teardown_trigger_pipe(void)
> +{
> +       return __init_thread_quit_pipe(thread_apps_notify_teardown_trigger_pipe);
> +}
> +
>  /*
>   * Stop all threads by closing the thread quit pipe.
>   */
> @@ -5763,6 +5769,11 @@ int main(int argc, char **argv)
>                 goto exit_init_data;
>         }
>
> +       if (init_thread_apps_notify_teardown_trigger_pipe()) {
> +               retval = -1;
> +               goto exit_init_data;
> +       }
> +
>         /* Check if daemon is UID = 0 */
>         is_root = !getuid();
>
> @@ -6354,6 +6365,12 @@ exit_dispatch:
>         }
>
>  exit_apps:
> +       /* Instruct the apps_notify thread to quit */
> +       ret = notify_thread_pipe(thread_apps_notify_teardown_trigger_pipe[1]);
> +       if (ret < 0) {
> +               ERR("write error on thread quit pipe");
> +       }
> +
>         ret = pthread_join(apps_notify_thread, &status);
>         if (ret) {
>                 errno = ret;
> @@ -6430,7 +6447,8 @@ exit_init_data:
>         utils_close_pipe(thread_health_teardown_trigger_pipe);
>         /* Apps thread teardown pipe cleanup */
>         utils_close_pipe(thread_apps_teardown_trigger_pipe);
> -
> +       /* Apps notify thread teardown pipe cleanup */
> +       utils_close_pipe(thread_apps_notify_teardown_trigger_pipe);
>         lttng_pipe_destroy(ust32_channel_monitor_pipe);
>         lttng_pipe_destroy(ust64_channel_monitor_pipe);
>         lttng_pipe_destroy(kernel_channel_monitor_pipe);
> diff --git a/src/bin/lttng-sessiond/ust-thread.c b/src/bin/lttng-sessiond/ust-thread.c
> index 7fb18a78..1e7a8229 100644
> --- a/src/bin/lttng-sessiond/ust-thread.c
> +++ b/src/bin/lttng-sessiond/ust-thread.c
> @@ -51,9 +51,15 @@ void *ust_thread_manage_notify(void *data)
>
>         health_code_update();
>
> -       ret = sessiond_set_thread_pollset(&events, 2);
> +       ret = lttng_poll_create(&events, 2, LTTNG_CLOEXEC);
>         if (ret < 0) {
> -               goto error_poll_create;
> +               goto error;
> +       }
> +
> +       /* Add quit pipe */
> +       ret = lttng_poll_add(&events, thread_apps_notify_teardown_trigger_pipe[0], LPOLLIN | LPOLLERR);
> +       if (ret < 0) {
> +               goto error;
>         }
>
>         /* Add notify pipe to the pollset. */
> @@ -99,11 +105,18 @@ restart:
>                                 continue;
>                         }
>
> -                       /* 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_apps_notify_teardown_trigger_pipe[0]) {
> +                               if (revents & LPOLLIN) {
> +                                       /* Normal exit */
> +                                       err = 0;
> +                                       goto exit;
> +                               } else if (revents & LPOLLERR) {
> +                                       ERR("Apps notify quit error");
> +                                       goto error;
> +                               } else {
> +                                       ERR("Unexpected poll events %u for quit pipe", revents);
> +                                       goto error;
> +                               }
>                         }
>
>                         /* Inspect the apps cmd pipe */
> --
> 2.11.0
>
diff mbox series

Patch

diff --git a/src/bin/lttng-sessiond/lttng-sessiond.h b/src/bin/lttng-sessiond/lttng-sessiond.h
index 74552db6..8fbcac60 100644
--- a/src/bin/lttng-sessiond/lttng-sessiond.h
+++ b/src/bin/lttng-sessiond/lttng-sessiond.h
@@ -94,6 +94,8 @@  struct ust_reg_wait_node {
  */
 extern int apps_cmd_notify_pipe[2];
 
+extern int thread_apps_notify_teardown_trigger_pipe[2];
+
 /*
  * Used to notify that a hash table needs to be destroyed by dedicated
  * thread. Required by design because we don't want to move destroy
diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index 8d9b14b5..3596d7e3 100644
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -206,6 +206,7 @@  static int kernel_poll_pipe[2] = { -1, -1 };
 static int thread_quit_pipe[2] = { -1, -1 };
 static int thread_health_teardown_trigger_pipe[2] = { -1, -1 };
 static int thread_apps_teardown_trigger_pipe[2] = { -1, -1 };
+int thread_apps_notify_teardown_trigger_pipe[2] = { -1, -1 };
 
 /*
  * This pipe is used to inform the thread managing application communication
@@ -489,6 +490,11 @@  static int init_thread_apps_teardown_trigger_pipe(void)
 	return __init_thread_quit_pipe(thread_apps_teardown_trigger_pipe);
 }
 
+static int init_thread_apps_notify_teardown_trigger_pipe(void)
+{
+	return __init_thread_quit_pipe(thread_apps_notify_teardown_trigger_pipe);
+}
+
 /*
  * Stop all threads by closing the thread quit pipe.
  */
@@ -5763,6 +5769,11 @@  int main(int argc, char **argv)
 		goto exit_init_data;
 	}
 
+	if (init_thread_apps_notify_teardown_trigger_pipe()) {
+		retval = -1;
+		goto exit_init_data;
+	}
+
 	/* Check if daemon is UID = 0 */
 	is_root = !getuid();
 
@@ -6354,6 +6365,12 @@  exit_dispatch:
 	}
 
 exit_apps:
+	/* Instruct the apps_notify thread to quit */
+	ret = notify_thread_pipe(thread_apps_notify_teardown_trigger_pipe[1]);
+	if (ret < 0) {
+		ERR("write error on thread quit pipe");
+	}
+
 	ret = pthread_join(apps_notify_thread, &status);
 	if (ret) {
 		errno = ret;
@@ -6430,7 +6447,8 @@  exit_init_data:
 	utils_close_pipe(thread_health_teardown_trigger_pipe);
 	/* Apps thread teardown pipe cleanup */
 	utils_close_pipe(thread_apps_teardown_trigger_pipe);
-
+	/* Apps notify thread teardown pipe cleanup */
+	utils_close_pipe(thread_apps_notify_teardown_trigger_pipe);
 	lttng_pipe_destroy(ust32_channel_monitor_pipe);
 	lttng_pipe_destroy(ust64_channel_monitor_pipe);
 	lttng_pipe_destroy(kernel_channel_monitor_pipe);
diff --git a/src/bin/lttng-sessiond/ust-thread.c b/src/bin/lttng-sessiond/ust-thread.c
index 7fb18a78..1e7a8229 100644
--- a/src/bin/lttng-sessiond/ust-thread.c
+++ b/src/bin/lttng-sessiond/ust-thread.c
@@ -51,9 +51,15 @@  void *ust_thread_manage_notify(void *data)
 
 	health_code_update();
 
-	ret = sessiond_set_thread_pollset(&events, 2);
+	ret = lttng_poll_create(&events, 2, LTTNG_CLOEXEC);
 	if (ret < 0) {
-		goto error_poll_create;
+		goto error;
+	}
+
+	/* Add quit pipe */
+	ret = lttng_poll_add(&events, thread_apps_notify_teardown_trigger_pipe[0], LPOLLIN | LPOLLERR);
+	if (ret < 0) {
+		goto error;
 	}
 
 	/* Add notify pipe to the pollset. */
@@ -99,11 +105,18 @@  restart:
 				continue;
 			}
 
-			/* 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_apps_notify_teardown_trigger_pipe[0]) {
+				if (revents & LPOLLIN) {
+					/* Normal exit */
+					err = 0;
+					goto exit;
+				} else if (revents & LPOLLERR) {
+					ERR("Apps notify quit error");
+					goto error;
+				} else {
+					ERR("Unexpected poll events %u for quit pipe", revents);
+					goto error;
+				}
 			}
 
 			/* Inspect the apps cmd pipe */