From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS58073 46.182.106.0/24 X-Spam-Status: No, score=-0.1 required=3.0 tests=BAYES_00,RCVD_IN_BRBL_LASTEXT, RCVD_IN_XBL shortcircuit=no autolearn=no version=3.3.2 X-Original-To: spew@80x24.org Received: from 80x24.org (tor-exit.critical.cat [46.182.106.190]) by dcvr.yhbt.net (Postfix) with ESMTP id 279D9633823 for ; Sat, 18 Jul 2015 03:39:15 +0000 (UTC) From: Eric Wong To: spew@80x24.org Subject: [PATCH] thread_pthread: eliminate malloc from signal thread list Date: Sat, 18 Jul 2015 03:39:14 +0000 Message-Id: <1437190754-29067-1-git-send-email-e@80x24.org> List-Id: ccan/list may be used and the list nodes are embedded directly into the rb_thread_t structure. Note: double-checked locking from FGLOCK is still possible, but I am not convinced it is necessary with modern pthreads implementations. --- thread_pthread.c | 90 ++++++++++++++++---------------------------------------- thread_pthread.h | 9 +++++- 2 files changed, 34 insertions(+), 65 deletions(-) diff --git a/thread_pthread.c b/thread_pthread.c index 26fe9ff..4eca11d 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -1122,15 +1122,7 @@ native_sleep(rb_thread_t *th, struct timeval *timeout_tv) } #ifdef USE_SIGNAL_THREAD_LIST -struct signal_thread_list { - rb_thread_t *th; - struct signal_thread_list *prev; - struct signal_thread_list *next; -}; - -static struct signal_thread_list signal_thread_list_anchor = { - 0, 0, 0, -}; +static LIST_HEAD(signal_thread_list_head); #define FGLOCK(lock, body) do { \ native_mutex_lock(lock); \ @@ -1144,12 +1136,12 @@ static struct signal_thread_list signal_thread_list_anchor = { static void print_signal_list(char *str) { - struct signal_thread_list *list = - signal_thread_list_anchor.next; + rb_thread_t *th; + thread_debug("list (%s)> ", str); - while (list) { - thread_debug("%p (%p), ", list->th, list->th->thread_id); - list = list->next; + list_for_each(&signal_thread_list_head, th, + native_thread_data.signal_thread.node) { + thread_debug("%p (%p), ", th, th->thread_id); } thread_debug("\n"); } @@ -1158,47 +1150,23 @@ print_signal_list(char *str) static void add_signal_thread_list(rb_thread_t *th) { - if (!th->native_thread_data.signal_thread_list) { - FGLOCK(&signal_thread_list_lock, { - struct signal_thread_list *list = - malloc(sizeof(struct signal_thread_list)); - - if (list == 0) { - fprintf(stderr, "[FATAL] failed to allocate memory\n"); - exit(EXIT_FAILURE); - } - - list->th = th; - - list->prev = &signal_thread_list_anchor; - list->next = signal_thread_list_anchor.next; - if (list->next) { - list->next->prev = list; - } - signal_thread_list_anchor.next = list; - th->native_thread_data.signal_thread_list = list; - }); - } + FGLOCK(&signal_thread_list_lock, { + if (!th->native_thread_data.signal_thread.listed) { + list_add_tail(&signal_thread_list_head, + &th->native_thread_data.signal_thread.node); + } + }); } static void remove_signal_thread_list(rb_thread_t *th) { - if (th->native_thread_data.signal_thread_list) { - FGLOCK(&signal_thread_list_lock, { - struct signal_thread_list *list = - (struct signal_thread_list *) - th->native_thread_data.signal_thread_list; - - list->prev->next = list->next; - if (list->next) { - list->next->prev = list->prev; - } - th->native_thread_data.signal_thread_list = 0; - list->th = 0; - free(list); /* ok */ - }); - } + FGLOCK(&signal_thread_list_lock, { + if (th->native_thread_data.signal_thread.listed) { + list_del(&th->native_thread_data.signal_thread.node); + th->native_thread_data.signal_thread.listed = 0; + } + }); } static void @@ -1231,26 +1199,20 @@ ubf_select(void *ptr) static void ping_signal_thread_list(void) { - if (signal_thread_list_anchor.next) { - FGLOCK(&signal_thread_list_lock, { - struct signal_thread_list *list; + FGLOCK(&signal_thread_list_lock, { + rb_thread_t *th; - list = signal_thread_list_anchor.next; - while (list) { - ubf_select_each(list->th); - list = list->next; - } - }); - } + list_for_each(&signal_thread_list_head, th, + native_thread_data.signal_thread.node) { + ubf_select_each(th); + } + }); } static int check_signal_thread_list(void) { - if (signal_thread_list_anchor.next) - return 1; - else - return 0; + return !list_empty(&signal_thread_list_head); } #else /* USE_SIGNAL_THREAD_LIST */ #define add_signal_thread_list(th) (void)(th) diff --git a/thread_pthread.h b/thread_pthread.h index 24a4af4..dcffd7a 100644 --- a/thread_pthread.h +++ b/thread_pthread.h @@ -11,6 +11,8 @@ #ifndef RUBY_THREAD_PTHREAD_H #define RUBY_THREAD_PTHREAD_H +#include "ccan/list/list.h" + #ifdef HAVE_PTHREAD_NP_H #include #endif @@ -26,7 +28,12 @@ typedef struct rb_thread_cond_struct { } rb_nativethread_cond_t; typedef struct native_thread_data_struct { - void *signal_thread_list; + /* Protected by signal_thread_list_lock: */ + union { + unsigned long listed; + struct list_node node; + } signal_thread; + rb_nativethread_cond_t sleep_cond; } native_thread_data_t; -- EW