From ec9a8ddd85da0598b718d16e5c8aa2b5e435e6a4 Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Wed, 17 Feb 2016 16:07:32 +1100 Subject: [PATCH 1/2] mysys: Don't alloca() in _lf_pinbox_real_free A crash on powerpc64le was traced to the use of alloca() in _lf_pinbox_real_free, where the code was attempting to allocate 252kB on the stack. _lf_pinbox_real_free is not a hotpath, and the lock-free allocator is used only in the performance schema and mysys/waiting_threads.c (which appears to be dead code). So simply replace alloca() with the much safer my_malloc(): allocate on the heap. Signed-off-by: Daniel Axtens --- mysys/lf_alloc-pin.c | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/mysys/lf_alloc-pin.c b/mysys/lf_alloc-pin.c index 65db784..07222c2 100644 --- a/mysys/lf_alloc-pin.c +++ b/mysys/lf_alloc-pin.c @@ -336,36 +336,31 @@ static int match_pins(LF_PINS *el, void *addr) */ static void _lf_pinbox_real_free(LF_PINS *pins) { - int npins; + int npins, sorted_size; void *list; void **addr= NULL; void *first= NULL, *last= NULL; LF_PINBOX *pinbox= pins->pinbox; + struct st_harvester hv; npins= pinbox->pins_in_array+1; + sorted_size= sizeof(void *)*LF_PINBOX_PINS*npins; -#ifdef HAVE_ALLOCA - if (pins->stack_ends_here != NULL) + /* create a sorted list of pinned addresses, to speed up searches */ + addr= (void **) my_malloc(sorted_size, MYF(MY_WME)); + if (addr) { - int alloca_size= sizeof(void *)*LF_PINBOX_PINS*npins; - /* create a sorted list of pinned addresses, to speed up searches */ - if (available_stack_size(&pinbox, *pins->stack_ends_here) > alloca_size) - { - struct st_harvester hv; - addr= (void **) alloca(alloca_size); - hv.granary= addr; - hv.npins= npins; - /* scan the dynarray and accumulate all pinned addresses */ - _lf_dynarray_iterate(&pinbox->pinarray, - (lf_dynarray_func)harvest_pins, &hv); - - npins= hv.granary-addr; - /* and sort them */ - if (npins) - qsort(addr, npins, sizeof(void *), (qsort_cmp)ptr_cmp); - } + hv.granary= addr; + hv.npins= npins; + /* scan the dynarray and accumulate all pinned addresses */ + _lf_dynarray_iterate(&pinbox->pinarray, + (lf_dynarray_func)harvest_pins, &hv); + + npins= hv.granary-addr; + /* and sort them */ + if (npins) + qsort(addr, npins, sizeof(void *), (qsort_cmp)ptr_cmp); } -#endif list= pins->purgatory; pins->purgatory= 0; @@ -389,7 +384,7 @@ static void _lf_pinbox_real_free(LF_PINS *pins) if (cur == *a || cur == *b) goto found; } - else /* no alloca - no cookie. linear search here */ + else /* couldn't created sorted array: linear search here */ { if (_lf_dynarray_iterate(&pinbox->pinarray, (lf_dynarray_func)match_pins, cur)) @@ -408,6 +403,8 @@ static void _lf_pinbox_real_free(LF_PINS *pins) } if (last) pinbox->free_func(first, last, pinbox->free_func_arg); + + my_free(addr); } /* lock-free memory allocator for fixed-size objects */ From 4d92dfdf6e5e6acdd3ddb9cc7165c8dde1953e17 Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Thu, 18 Feb 2016 11:21:33 +1100 Subject: [PATCH 2/2] mysys: Remove 'stack_ends_here' The calculation of stack_ends_here was only used by the lock-free allocator, and only to do aggressive sizing of an alloca() call. The calculation is unclear and the use in alloca() caused a crash. The previous patch removed the alloca() call, so now remove the calculation and propagation. Signed-off-by: Daniel Axtens --- include/lf.h | 5 ++--- include/my_pthread.h | 1 - mysys/lf_alloc-pin.c | 16 +--------------- mysys/my_thr_init.c | 3 --- 4 files changed, 3 insertions(+), 22 deletions(-) diff --git a/include/lf.h b/include/lf.h index d807b21..11de069 100644 --- a/include/lf.h +++ b/include/lf.h @@ -111,13 +111,12 @@ typedef struct { typedef struct { void * volatile pin[LF_PINBOX_PINS]; LF_PINBOX *pinbox; - void **stack_ends_here; void *purgatory; uint32 purgatory_count; uint32 volatile link; /* we want sizeof(LF_PINS) to be 64 to avoid false sharing */ -#if SIZEOF_INT*2+SIZEOF_CHARP*(LF_PINBOX_PINS+3) != 64 - char pad[64-sizeof(uint32)*2-sizeof(void*)*(LF_PINBOX_PINS+3)]; +#if SIZEOF_INT*2+SIZEOF_CHARP*(LF_PINBOX_PINS+2) != 64 + char pad[64-sizeof(uint32)*2-sizeof(void*)*(LF_PINBOX_PINS+2)]; #endif } LF_PINS; diff --git a/include/my_pthread.h b/include/my_pthread.h index 6463cee..ee7dd5e 100644 --- a/include/my_pthread.h +++ b/include/my_pthread.h @@ -853,7 +853,6 @@ struct st_my_thread_var my_bool init; struct st_my_thread_var *next,**prev; void *opt_info; - void *stack_ends_here; #ifndef DBUG_OFF void *dbug; char name[THREAD_NAME_SIZE+1]; diff --git a/mysys/lf_alloc-pin.c b/mysys/lf_alloc-pin.c index 07222c2..96dd860 100644 --- a/mysys/lf_alloc-pin.c +++ b/mysys/lf_alloc-pin.c @@ -96,8 +96,7 @@ upper 16 bits are used for a version. It is assumed that pins belong to a THD and are not transferable - between THD's (LF_PINS::stack_ends_here being a primary reason - for this limitation). + between THD's */ #include #include @@ -146,7 +145,6 @@ void lf_pinbox_destroy(LF_PINBOX *pinbox) */ LF_PINS *_lf_pinbox_get_pins(LF_PINBOX *pinbox) { - struct st_my_thread_var *var; uint32 pins, next, top_ver; LF_PINS *el; /* @@ -189,12 +187,6 @@ LF_PINS *_lf_pinbox_get_pins(LF_PINBOX *pinbox) el->link= pins; el->purgatory_count= 0; el->pinbox= pinbox; - var= my_thread_var; - /* - Threads that do not call my_thread_init() should still be - able to use the LF_HASH. - */ - el->stack_ends_here= (var ? & var->stack_ends_here : NULL); return el; } @@ -322,12 +314,6 @@ static int match_pins(LF_PINS *el, void *addr) return 0; } -#if STACK_DIRECTION < 0 -#define available_stack_size(CUR,END) (long) ((char*)(CUR) - (char*)(END)) -#else -#define available_stack_size(CUR,END) (long) ((char*)(END) - (char*)(CUR)) -#endif - #define next_node(P, X) (*((uchar * volatile *)(((uchar *)(X)) + (P)->free_ptr_offset))) #define anext_node(X) next_node(&allocator->pinbox, (X)) diff --git a/mysys/my_thr_init.c b/mysys/my_thr_init.c index 6d56150..76f5d8e 100644 --- a/mysys/my_thr_init.c +++ b/mysys/my_thr_init.c @@ -297,9 +297,6 @@ my_bool my_thread_init(void) mysql_mutex_init(key_my_thread_var_mutex, &tmp->mutex, MY_MUTEX_INIT_FAST); mysql_cond_init(key_my_thread_var_suspend, &tmp->suspend, NULL); - tmp->stack_ends_here= (char*)&tmp + - STACK_DIRECTION * (long)my_thread_stack_size; - mysql_mutex_lock(&THR_LOCK_threads); tmp->id= ++thread_id; ++THR_thread_count;