Second try at stack-guard patch, also apply fix for bug #435494

This commit is contained in:
Tom Lane 2008-03-03 22:30:11 +00:00 committed by Michal Schorm
parent 0198dfa540
commit e6fb210ebd
3 changed files with 97 additions and 44 deletions

View File

@ -1,26 +1,84 @@
mysql is not accounting for the "guard page" when setting thread stack size
requests. This is fatal on PPC systems, which may use guard pages as large
as 64K. I'll bet a good deal that the hacks it uses for IA64 are a result
of misdiagnosis of a similar problem, so remove them.
as 64K. This patch also documents the IA64 situation a bit better.
It is not at this point entirely clear whether mysql is wrong in ignoring
the guard page, or whether this is a RHEL bug (see our bz#435337). So not
reporting this upstream yet. But we need the patch now, so we can build mysql
in rawhide (the build machines are using RHEL5 kernels).
Note: there are quite a few other setstacksize calls besides the two in
mysqld.cc; is it important to fix any of the others?
diff -Naur mysql-5.0.45.orig/sql/mysqld.cc mysql-5.0.45/sql/mysqld.cc
--- mysql-5.0.45.orig/sql/mysqld.cc 2007-07-04 09:06:03.000000000 -0400
+++ mysql-5.0.45/sql/mysqld.cc 2008-02-28 15:17:20.000000000 -0500
@@ -2286,6 +2286,7 @@
{
int error;
pthread_attr_t thr_attr;
+ size_t guard_size = 0;
DBUG_ENTER("start_signal_handler");
+++ mysql-5.0.45/sql/mysqld.cc 2008-03-03 17:16:25.000000000 -0500
@@ -2281,6 +2281,68 @@
DBUG_VOID_RETURN;
}
(void) pthread_attr_init(&thr_attr);
@@ -2294,15 +2295,9 @@
+/* pthread_attr_setstacksize without so much platform-dependency */
+/* returns the actual stack size if possible */
+static size_t my_setstacksize(pthread_attr_t *attr, size_t stacksize)
+{
+ size_t guard_size = 0;
+
+#if defined(__ia64__) || defined(__ia64)
+ /*
+ On IA64, half of the requested stack size is used for "normal stack"
+ and half for "register stack". The space measured by check_stack_overrun
+ is the "normal stack", so double the request to make sure we have the
+ caller-expected amount of normal stack.
+
+ NOTE: there is no guarantee that the register stack can't grow faster
+ than normal stack, so it's very unclear that we won't dump core due to
+ stack overrun despite check_stack_overrun's efforts. Experimentation
+ shows that in the execution_constants test, the register stack grows
+ less than half as fast as normal stack, but perhaps other scenarios are
+ less forgiving. If it turns out that more space is needed for the
+ register stack, that could be forced (rather inefficiently) by using a
+ multiplier higher than 2 here.
+ */
+ stacksize *= 2;
+#endif
+
+ /*
+ On many machines, the "guard space" is subtracted from the requested
+ stack size, and that space is quite large on some platforms. So add
+ it to our request, if we can find out what it is.
+
+ FIXME: autoconfiscate use of pthread_attr_getguardsize
+ */
+ if (pthread_attr_getguardsize(attr, &guard_size))
+ guard_size = 0; /* if can't find it out, treat as 0 */
+
+ pthread_attr_setstacksize(attr, stacksize + guard_size);
+
+ /* Retrieve actual stack size if possible */
+#ifdef HAVE_PTHREAD_ATTR_GETSTACKSIZE
+ {
+ size_t real_stack_size= 0;
+ /* We must ignore real_stack_size = 0 as Solaris 2.9 can return 0 here */
+ if (pthread_attr_getstacksize(attr, &real_stack_size) == 0 &&
+ real_stack_size > guard_size)
+ {
+ real_stack_size -= guard_size;
+ if (real_stack_size < stacksize)
+ {
+ if (global_system_variables.log_warnings)
+ sql_print_warning("Asked for %ld thread stack, but got %ld",
+ (long) stacksize, (long) real_stack_size);
+ stacksize= real_stack_size;
+ }
+ }
+ }
+#endif
+
+#if defined(__ia64__) || defined(__ia64)
+ stacksize /= 2;
+#endif
+ return stacksize;
+}
static void start_signal_handler(void)
{
@@ -2294,15 +2356,7 @@
(void) pthread_attr_setdetachstate(&thr_attr,PTHREAD_CREATE_DETACHED);
if (!(opt_specialflag & SPECIAL_NO_PRIOR))
my_pthread_attr_setprio(&thr_attr,INTERRUPT_PRIOR);
@ -33,13 +91,11 @@ diff -Naur mysql-5.0.45.orig/sql/mysqld.cc mysql-5.0.45/sql/mysqld.cc
-#else
- pthread_attr_setstacksize(&thr_attr,thread_stack);
-#endif
+
+ pthread_attr_getguardsize(&thr_attr, &guard_size);
+ pthread_attr_setstacksize(&thr_attr, thread_stack + guard_size);
+ (void) my_setstacksize(&thr_attr,thread_stack);
#endif
(void) pthread_mutex_lock(&LOCK_thread_count);
@@ -3499,37 +3494,29 @@
@@ -3499,41 +3553,12 @@
init_signals();
if (!(opt_specialflag & SPECIAL_NO_PRIOR))
my_pthread_setprio(pthread_self(),CONNECT_PRIOR);
@ -53,27 +109,17 @@ diff -Naur mysql-5.0.45.orig/sql/mysqld.cc mysql-5.0.45/sql/mysqld.cc
- pthread_attr_setstacksize(&connection_attrib,thread_stack);
-#endif
-#ifdef HAVE_PTHREAD_ATTR_GETSTACKSIZE
+
{
+ size_t guard_size = 0;
+ size_t stack_size = 0;
+
+ pthread_attr_getguardsize(&connection_attrib, &guard_size);
+
+ pthread_attr_setstacksize(&connection_attrib, thread_stack + guard_size);
+
+#ifdef HAVE_PTHREAD_ATTR_GETSTACKSIZE
/* Retrieve used stack size; Needed for checking stack overflows */
- {
- /* Retrieve used stack size; Needed for checking stack overflows */
- size_t stack_size= 0;
pthread_attr_getstacksize(&connection_attrib, &stack_size);
- pthread_attr_getstacksize(&connection_attrib, &stack_size);
-#if defined(__ia64__) || defined(__ia64)
- stack_size/= 2;
-#endif
/* We must check if stack_size = 0 as Solaris 2.9 can return 0 here */
- /* We must check if stack_size = 0 as Solaris 2.9 can return 0 here */
- if (stack_size && stack_size < thread_stack)
+ if (stack_size && stack_size < thread_stack + guard_size)
{
if (global_system_variables.log_warnings)
- {
- if (global_system_variables.log_warnings)
- sql_print_warning("Asked for %lu thread stack, but got %ld",
- thread_stack, (long) stack_size);
-#if defined(__ia64__) || defined(__ia64)
@ -81,14 +127,16 @@ diff -Naur mysql-5.0.45.orig/sql/mysqld.cc mysql-5.0.45/sql/mysqld.cc
-#else
- thread_stack= stack_size;
-#endif
+ sql_print_warning("Asked for %lu+%lu thread stack, but got %ld",
+ thread_stack, (long) guard_size, (long) stack_size);
+ thread_stack= stack_size - guard_size;
}
- }
- }
#endif
+ }
-#endif
+
#ifdef __NETWARE__
/* Increasing stacksize of threads on NetWare */
-
pthread_attr_setstacksize(&connection_attrib, NW_THD_STACKSIZE);
+#else
+ thread_stack = my_setstacksize(&connection_attrib,thread_stack);
#endif
(void) thr_setconcurrency(concurrency); // 10 by default

View File

@ -68,7 +68,7 @@ start(){
if [ $ret -eq 0 ]; then
STARTTIMEOUT=30
while [ $STARTTIMEOUT -gt 0 ]; do
RESPONSE=`/usr/bin/mysqladmin -uUNKNOWN_MYSQL_USER ping 2>&1` && break
RESPONSE=`/usr/bin/mysqladmin --socket="$socketfile" --user=UNKNOWN_MYSQL_USER ping 2>&1` && break
echo "$RESPONSE" | grep -q "Access denied for user" && break
sleep 1
let STARTTIMEOUT=${STARTTIMEOUT}-1

View File

@ -1,6 +1,6 @@
Name: mysql
Version: 5.0.45
Release: 10%{?dist}
Release: 11%{?dist}
Summary: MySQL client programs and shared libraries
Group: Applications/Databases
URL: http://www.mysql.com
@ -484,8 +484,13 @@ fi
%{_mandir}/man1/mysql_client_test.1*
%changelog
* Mon Mar 3 2008 Tom Lane <tgl@redhat.com> 5.0.45-11
- Fix mysql-stack-guard patch to work correctly on IA64
- Fix mysql.init to wait correctly when socket is not in default place
Related: #435494
* Mon Mar 03 2008 Dennis Gilmore <dennis@ausil.us> 5.0.45-10
- add sparc64 to 64 bit arches for test suite checking
- add sparc64 to 64 bit arches for test suite checking
- add sparc, sparcv9 and sparc64 to multilib handling
* Thu Feb 28 2008 Tom Lane <tgl@redhat.com> 5.0.45-9