[lttng-dev] [PATCH lttng-tools 6/6] Fix: Set thread stack size to ulimit soft value
Jérémie Galarneau
jeremie.galarneau at efficios.com
Wed Jun 29 22:50:49 UTC 2016
Merged the first 5 patches "as-is". This one was merged with a number
of modifications; read on.
Thanks!
Jérémie
On Wed, Jun 15, 2016 at 5:18 PM, Michael Jeanson <mjeanson at efficios.com> wrote:
> Some libc don't honor the limit set for the stack size and use their own
> empirically chosen static value. Detect this behavior by checking if the
> current stack size is smally than the soft limit and in that case set
smally? ;-)
> the pthread stack size to soft limit value.
>
> Signed-off-by: Michael Jeanson <mjeanson at efficios.com>
> ---
> src/bin/lttng-consumerd/lttng-consumerd.c | 17 +++++---
> src/bin/lttng-relayd/live.c | 10 ++---
> src/bin/lttng-relayd/live.h | 2 +-
> src/bin/lttng-relayd/main.c | 14 ++++---
> src/bin/lttng-sessiond/main.c | 27 +++++++------
> src/common/utils.c | 64 +++++++++++++++++++++++++++++++
> src/common/utils.h | 1 +
> 7 files changed, 107 insertions(+), 28 deletions(-)
>
> diff --git a/src/bin/lttng-consumerd/lttng-consumerd.c b/src/bin/lttng-consumerd/lttng-consumerd.c
> index 00660fc..2f1d01c 100644
> --- a/src/bin/lttng-consumerd/lttng-consumerd.c
> +++ b/src/bin/lttng-consumerd/lttng-consumerd.c
> @@ -57,6 +57,8 @@
> static pthread_t channel_thread, data_thread, metadata_thread,
> sessiond_thread, metadata_timer_thread, health_thread;
>
> +static pthread_attr_t *tattr;
> +
> /* to count the number of times the user pressed ctrl+c */
> static int sigintcount = 0;
>
> @@ -351,6 +353,9 @@ int main(int argc, char **argv)
> }
> }
>
> + /* Get stacksize limit */
> + tattr = get_pthread_attr_stacksize();
Renamed to get_default_pthread_attr(). It is now called "in place" on
pthread_create() instead of passing the value around.
> +
> /*
> * Starting from here, we can create threads. This needs to be after
> * lttng_daemonize due to RCU.
> @@ -498,7 +503,7 @@ int main(int argc, char **argv)
> }
>
> /* Create thread to manage the client socket */
> - ret = pthread_create(&health_thread, NULL,
> + ret = pthread_create(&health_thread, tattr,
> thread_manage_health, (void *) NULL);
> if (ret) {
> errno = ret;
> @@ -517,7 +522,7 @@ int main(int argc, char **argv)
> cmm_smp_mb(); /* Read ready before following operations */
>
> /* Create thread to manage channels */
> - ret = pthread_create(&channel_thread, NULL,
> + ret = pthread_create(&channel_thread, tattr,
> consumer_thread_channel_poll,
> (void *) ctx);
> if (ret) {
> @@ -528,7 +533,7 @@ int main(int argc, char **argv)
> }
>
> /* Create thread to manage the polling/writing of trace metadata */
> - ret = pthread_create(&metadata_thread, NULL,
> + ret = pthread_create(&metadata_thread, tattr,
> consumer_thread_metadata_poll,
> (void *) ctx);
> if (ret) {
> @@ -539,7 +544,7 @@ int main(int argc, char **argv)
> }
>
> /* Create thread to manage the polling/writing of trace data */
> - ret = pthread_create(&data_thread, NULL, consumer_thread_data_poll,
> + ret = pthread_create(&data_thread, tattr, consumer_thread_data_poll,
> (void *) ctx);
> if (ret) {
> errno = ret;
> @@ -549,7 +554,7 @@ int main(int argc, char **argv)
> }
>
> /* Create the thread to manage the receive of fd */
> - ret = pthread_create(&sessiond_thread, NULL,
> + ret = pthread_create(&sessiond_thread, tattr,
> consumer_thread_sessiond_poll,
> (void *) ctx);
> if (ret) {
> @@ -563,7 +568,7 @@ int main(int argc, char **argv)
> * Create the thread to manage the UST metadata periodic timer and
> * live timer.
> */
> - ret = pthread_create(&metadata_timer_thread, NULL,
> + ret = pthread_create(&metadata_timer_thread, tattr,
> consumer_timer_thread, (void *) ctx);
> if (ret) {
> errno = ret;
> diff --git a/src/bin/lttng-relayd/live.c b/src/bin/lttng-relayd/live.c
> index e2096ec..32efab1 100644
> --- a/src/bin/lttng-relayd/live.c
> +++ b/src/bin/lttng-relayd/live.c
> @@ -2147,13 +2147,13 @@ int relayd_live_join(void)
> /*
> * main
> */
> -int relayd_live_create(struct lttng_uri *uri)
> +int relayd_live_create(struct lttng_uri *uri, const pthread_attr_t *tattr)
Removed the tattr parameter since get_default_pthread_attr() is now
called directly.
> {
> int ret = 0, retval = 0;
> void *status;
> int is_root;
>
> - if (!uri) {
> + if (!uri || !tattr) {
> retval = -1;
> goto exit_init_data;
> }
> @@ -2186,7 +2186,7 @@ int relayd_live_create(struct lttng_uri *uri)
> }
>
> /* Setup the dispatcher thread */
> - ret = pthread_create(&live_dispatcher_thread, NULL,
> + ret = pthread_create(&live_dispatcher_thread, tattr,
> thread_dispatcher, (void *) NULL);
> if (ret) {
> errno = ret;
> @@ -2196,7 +2196,7 @@ int relayd_live_create(struct lttng_uri *uri)
> }
>
> /* Setup the worker thread */
> - ret = pthread_create(&live_worker_thread, NULL,
> + ret = pthread_create(&live_worker_thread, tattr,
> thread_worker, NULL);
> if (ret) {
> errno = ret;
> @@ -2206,7 +2206,7 @@ int relayd_live_create(struct lttng_uri *uri)
> }
>
> /* Setup the listener thread */
> - ret = pthread_create(&live_listener_thread, NULL,
> + ret = pthread_create(&live_listener_thread, tattr,
> thread_listener, (void *) NULL);
> if (ret) {
> errno = ret;
> diff --git a/src/bin/lttng-relayd/live.h b/src/bin/lttng-relayd/live.h
> index 2b8a3a0..6cd85e9 100644
> --- a/src/bin/lttng-relayd/live.h
> +++ b/src/bin/lttng-relayd/live.h
> @@ -24,7 +24,7 @@
>
> #include "lttng-relayd.h"
>
> -int relayd_live_create(struct lttng_uri *live_uri);
> +int relayd_live_create(struct lttng_uri *live_uri, const pthread_attr_t *tattr);
Removed tattr.
> int relayd_live_stop(void);
> int relayd_live_join(void);
>
> diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c
> index 6ad6566..505e884 100644
> --- a/src/bin/lttng-relayd/main.c
> +++ b/src/bin/lttng-relayd/main.c
> @@ -119,6 +119,8 @@ static pthread_t dispatcher_thread;
> static pthread_t worker_thread;
> static pthread_t health_thread;
>
> +static pthread_attr_t *tattr;
> +
Same modifications as in the session daemon apply.
> /*
> * last_relay_stream_id_lock protects last_relay_stream_id increment
> * atomicity on 32-bit architectures.
> @@ -2778,6 +2780,8 @@ int main(int argc, char **argv)
> }
> }
>
> + /* Get stack size limit */
> + tattr = get_pthread_attr_stacksize();
>
> /* Initialize thread health monitoring */
> health_relayd = health_app_create(NR_HEALTH_RELAYD_TYPES);
> @@ -2840,7 +2844,7 @@ int main(int argc, char **argv)
> }
>
> /* Create thread to manage the client socket */
> - ret = pthread_create(&health_thread, NULL,
> + ret = pthread_create(&health_thread, tattr,
> thread_manage_health, (void *) NULL);
> if (ret) {
> errno = ret;
> @@ -2850,7 +2854,7 @@ int main(int argc, char **argv)
> }
>
> /* Setup the dispatcher thread */
> - ret = pthread_create(&dispatcher_thread, NULL,
> + ret = pthread_create(&dispatcher_thread, tattr,
> relay_thread_dispatcher, (void *) NULL);
> if (ret) {
> errno = ret;
> @@ -2860,7 +2864,7 @@ int main(int argc, char **argv)
> }
>
> /* Setup the worker thread */
> - ret = pthread_create(&worker_thread, NULL,
> + ret = pthread_create(&worker_thread, tattr,
> relay_thread_worker, NULL);
> if (ret) {
> errno = ret;
> @@ -2870,7 +2874,7 @@ int main(int argc, char **argv)
> }
>
> /* Setup the listener thread */
> - ret = pthread_create(&listener_thread, NULL,
> + ret = pthread_create(&listener_thread, tattr,
> relay_thread_listener, (void *) NULL);
> if (ret) {
> errno = ret;
> @@ -2879,7 +2883,7 @@ int main(int argc, char **argv)
> goto exit_listener_thread;
> }
>
> - ret = relayd_live_create(live_uri);
> + ret = relayd_live_create(live_uri, tattr);
> if (ret) {
> ERR("Starting live viewer threads");
> retval = -1;
> diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
> index bab92a4..359cb83 100644
> --- a/src/bin/lttng-sessiond/main.c
> +++ b/src/bin/lttng-sessiond/main.c
> @@ -212,6 +212,8 @@ static pthread_t ht_cleanup_thread;
> static pthread_t agent_reg_thread;
> static pthread_t load_session_thread;
>
> +static pthread_attr_t *tattr;
> +
> /*
> * UST registration command queue. This queue is tied with a futex and uses a N
> * wakers / 1 waiter implemented and detailed in futex.c/.h
> @@ -2407,7 +2409,7 @@ static int spawn_consumer_thread(struct consumer_data *consumer_data)
> goto error;
> }
>
> - ret = pthread_create(&consumer_data->thread, NULL, thread_manage_consumer,
> + ret = pthread_create(&consumer_data->thread, tattr, thread_manage_consumer,
> consumer_data);
> if (ret) {
> errno = ret;
> @@ -5636,6 +5638,9 @@ int main(int argc, char **argv)
> goto exit_create_run_as_worker_cleanup;
> }
>
> + /* Get stack size limit */
> + tattr = get_pthread_attr_stacksize();
> +
> /*
> * Starting from here, we can create threads. This needs to be after
> * lttng_daemonize due to RCU.
> @@ -5670,7 +5675,7 @@ int main(int argc, char **argv)
> }
>
> /* Create thread to clean up RCU hash tables */
> - ret = pthread_create(&ht_cleanup_thread, NULL,
> + ret = pthread_create(&ht_cleanup_thread, tattr,
> thread_ht_cleanup, (void *) NULL);
> if (ret) {
> errno = ret;
> @@ -6042,7 +6047,7 @@ int main(int argc, char **argv)
> load_info->path = opt_load_session_path;
>
> /* Create health-check thread */
> - ret = pthread_create(&health_thread, NULL,
> + ret = pthread_create(&health_thread, tattr,
> thread_manage_health, (void *) NULL);
> if (ret) {
> errno = ret;
> @@ -6052,7 +6057,7 @@ int main(int argc, char **argv)
> }
>
> /* Create thread to manage the client socket */
> - ret = pthread_create(&client_thread, NULL,
> + ret = pthread_create(&client_thread, tattr,
> thread_manage_clients, (void *) NULL);
> if (ret) {
> errno = ret;
> @@ -6062,7 +6067,7 @@ int main(int argc, char **argv)
> }
>
> /* Create thread to dispatch registration */
> - ret = pthread_create(&dispatch_thread, NULL,
> + ret = pthread_create(&dispatch_thread, tattr,
> thread_dispatch_ust_registration, (void *) NULL);
> if (ret) {
> errno = ret;
> @@ -6072,7 +6077,7 @@ int main(int argc, char **argv)
> }
>
> /* Create thread to manage application registration. */
> - ret = pthread_create(®_apps_thread, NULL,
> + ret = pthread_create(®_apps_thread, tattr,
> thread_registration_apps, (void *) NULL);
> if (ret) {
> errno = ret;
> @@ -6082,7 +6087,7 @@ int main(int argc, char **argv)
> }
>
> /* Create thread to manage application socket */
> - ret = pthread_create(&apps_thread, NULL,
> + ret = pthread_create(&apps_thread, tattr,
> thread_manage_apps, (void *) NULL);
> if (ret) {
> errno = ret;
> @@ -6092,7 +6097,7 @@ int main(int argc, char **argv)
> }
>
> /* Create thread to manage application notify socket */
> - ret = pthread_create(&apps_notify_thread, NULL,
> + ret = pthread_create(&apps_notify_thread, tattr,
> ust_thread_manage_notify, (void *) NULL);
> if (ret) {
> errno = ret;
> @@ -6102,7 +6107,7 @@ int main(int argc, char **argv)
> }
>
> /* Create agent registration thread. */
> - ret = pthread_create(&agent_reg_thread, NULL,
> + ret = pthread_create(&agent_reg_thread, tattr,
> agent_thread_manage_registration, (void *) NULL);
> if (ret) {
> errno = ret;
> @@ -6114,7 +6119,7 @@ int main(int argc, char **argv)
> /* Don't start this thread if kernel tracing is not requested nor root */
> if (is_root && !opt_no_kernel) {
> /* Create kernel thread to manage kernel event */
> - ret = pthread_create(&kernel_thread, NULL,
> + ret = pthread_create(&kernel_thread, tattr,
> thread_manage_kernel, (void *) NULL);
> if (ret) {
> errno = ret;
> @@ -6125,7 +6130,7 @@ int main(int argc, char **argv)
> }
>
> /* Create session loading thread. */
> - ret = pthread_create(&load_session_thread, NULL, thread_load_session,
> + ret = pthread_create(&load_session_thread, tattr, thread_load_session,
> load_info);
> if (ret) {
> errno = ret;
> diff --git a/src/common/utils.c b/src/common/utils.c
> index 1e52ae0..62f970c 100644
> --- a/src/common/utils.c
> +++ b/src/common/utils.c
Moved the function to defaults.c
> @@ -31,6 +31,8 @@
> #include <pwd.h>
> #include <sys/file.h>
> #include <unistd.h>
> +#include <stdbool.h>
> +#include <sys/resource.h>
>
> #include <common/common.h>
> #include <common/runas.h>
> @@ -1383,3 +1385,65 @@ int utils_show_man_page(int section, const char *page_name)
> section_string, page_name, NULL);
> return ret;
> }
> +
> +static bool pthread_ss_done = false;
Renamed to pthread_attr_init_done. There is also no need to explicitly
initialize the value to "false" since it is static.
> +static pthread_attr_t *tattr = NULL;
> +static pthread_attr_t tattr_value;
Modified to only:
static pthread_attr_t tattr;
> +
> +/*
> + * Some libc don't honor the limit set for the stack size and use their own
> + * empirically chosen static value. This function checks if the current stack
> + * size is smaller than the stack size limit and if so returns a
> + * pthread_attr_t struct where the thread stack size is set to the soft stack
> + * size limit.
> + *
> + * If the libc honors stack size limit, return a NULL pointer.
> + */
> +LTTNG_HIDDEN
> +pthread_attr_t *get_pthread_attr_stacksize() {
Moved the '{' to its own line.
Also, make sure you use "void" in the function's signature when no
arguments are expected. An empty parameter list has a different
meaning in C than in C++, see http://stackoverflow.com/a/51080/3334991
> + int ret = 0;
> + size_t ptstacksize;
> + struct rlimit rlim;
> +
> + /* Return cached value */
Missing dot at the end of the sentence.
> + if (pthread_ss_done) {
> + goto end;
> + }
> +
> + /* Get stack size limits */
Missing dot at the end of the sentence.
> + ret = getrlimit(RLIMIT_STACK, &rlim);
> + if (ret < 0) {
> + PERROR("getrlimit");
> + goto end;
> + }
> + DBG("Stack size limits: soft %lld, hard %lld bytes",
> + (long long) rlim.rlim_cur,
> + (long long) rlim.rlim_max);
> +
> + /* Get default thread stack size */
Missing dot at the end of the sentence.
> + ret = pthread_attr_getstacksize(&tattr_value, &ptstacksize);
> + if (ret < 0) {
> + PERROR("pthread_attr_getstacksize");
> + goto end;
> + }
> + DBG("Default pthread stack size is %zu bytes", ptstacksize);
> +
> + /* Check if default thread stack size respects ulimits */
Missing dot at the end of sentence.
> + if (ptstacksize < rlim.rlim_cur) {
> + MSG("Your libc doesn't honor stack size limits, setting thread stack size to soft limit (%lld bytes)",
> + (long long) rlim.rlim_cur);
Changed to DBG since there isn't much the user can do (beyond
switching libc) and we have "fixed" the problem.
> +
> + /* Create pthread_attr_t struct with ulimit stack size */
Missing dot at the end of sentence.
> + ret = pthread_attr_setstacksize(&tattr_value, rlim.rlim_cur);
> + if (ret < 0) {
> + PERROR("pthread_attr_setstacksize");
> + goto end;
> + }
> + tattr = &tattr_value;
> + }
> +
> + /* Enable cached value */
> + pthread_ss_done = true;
> +end:
> + return tattr;
> +}
> diff --git a/src/common/utils.h b/src/common/utils.h
> index 7285f5c..568c123 100644
> --- a/src/common/utils.h
> +++ b/src/common/utils.h
> @@ -60,5 +60,6 @@ int utils_create_lock_file(const char *filepath);
> int utils_recursive_rmdir(const char *path);
> int utils_truncate_stream_file(int fd, off_t length);
> int utils_show_man_page(int section, const char *page_name);
> +pthread_attr_t *get_pthread_attr_stacksize();
(void) here.
>
> #endif /* _COMMON_UTILS_H */
> --
> 2.7.4
>
--
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com
More information about the lttng-dev
mailing list