From 61fa40f869d11fd96b443a4f2e00df70d6804ce5 Mon Sep 17 00:00:00 2001 From: Yibo Cai Date: Thu, 21 Mar 2019 10:48:45 +0800 Subject: [PATCH] Fix rwlock memory order issue on aarch64 Mysql stress test on aarch64 results to rwlock bug assert. Errorlog: --------- 2019-03-20T08:26:35.547378Z 62 [ERROR] [MY-013183] [InnoDB] Assertion failure: sync0rw.ic:499 thread 281471895343344 InnoDB: We intentionally generate a memory trap. InnoDB: Submit a detailed bug report to http://bugs.mysql.com. InnoDB: If you get repeated assertion failures or crashes, even InnoDB: immediately after the mysqld startup, there may be InnoDB: corruption in the InnoDB tablespace. Please refer to InnoDB: http://dev.mysql.com/doc/refman/8.0/en/forcing-innodb-recovery.html InnoDB: about forcing recovery. 08:26:35 UTC - mysqld got signal 6 ; This could be because you hit a bug. It is also possible that this binary or one of the libraries it was linked against is corrupt, improperly built, or misconfigured. This error can also be caused by malfunctioning hardware. Attempting to collect some information that could help diagnose the problem. As this is a crash and something is definitely wrong, the information collection process might fail. key_buffer_size=8388608 read_buffer_size=131072 max_used_connections=64 max_threads=4000 thread_count=65 connection_count=64 It is possible that mysqld could use up to key_buffer_size + (read_buffer_size + sort_buffer_size)*max_threads = 1588879 K bytes of memory Hope that's ok; if not, decrease some variables in the equation. Thread pointer: 0xffe328000db0 Attempting backtrace. You can use the following information to find out where mysqld died. If you see no messages after this, something went terribly wrong... stack_bottom = ffff48560888 thread_stack 0x46000 /home/linux/mysql-8.0/1-install/bin/mysqld(my_print_stacktrace(unsigned char*, unsigned long)+0x44) [0xaaaac5ec412c] /home/linux/mysql-8.0/1-install/bin/mysqld(handle_fatal_signal+0x3bc) [0xaaaac52acb44] linux-vdso.so.1(__kernel_rt_sigreturn+0) [0xffffab8746c0] /lib/aarch64-linux-gnu/libc.so.6(raise+0xb0) [0xffffab1bc4d8] Bug analysis ------------ "writer_thread" and "recursive" are two members of rw_lock_t struct. lock->writer_thread MUST be updated BEFORE lock->recursive, as mentioned in comments of function rw_lock_set_writer_id_and_recursion_flag() in file sync0rw.ic. Writer memory barrier is inserted correctly in producer code by calling os_compare_and_swap_thread_id(). https://github.com/mysql/mysql-server/blob/mysql-8.0.15/storage/innobase/include/sync0rw.ic#L283-L307 local_thread = lock->writer_thread; success = os_compare_and_swap_thread_id( &lock->writer_thread, local_thread, curr_thread); <-- wmb inserted ut_a(success); lock->recursive = recursive; But the consumer code lacks necessary reader memory barrier and may suffer from memory order issue. https://github.com/mysql/mysql-server/blob/mysql-8.0.15/storage/innobase/sync/sync0rw.cc#L518-L525 if (!pass) { os_rmb; // XXX: this rmb doesn't help } /* Decrement failed: An X or SX lock is held by either this thread or another. Try to relock. */ if (!pass && lock->recursive && // XXX: should insert rmb here! os_thread_eq(lock->writer_thread, thread_id)) { In above code, lock->recursive is visited before lock->writer_thread in program order. No problems have been observed on x86 with current code. But this code is unsafe for weak memory order platforms like Arm. Without explicit rmb() between loading the two variables, we may observe latest lock->recursive but obsolete lock->writer_thread, leads to inconsistent data. In this patch we re-order lock->recursive before an existing rmb. This ensures that the read path barrier correlates correctly with the write path barrier and that the read path observes lock->recursive before lock->writer_thread. Test result ----------- We run stress test for one hour to reproduce the bug. Two out of three tests bugcheck in about 30 minutes. After applying this patch, five 1-hour tests and one 24-hours stress test all pass. --- storage/innobase/sync/sync0rw.cc | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/storage/innobase/sync/sync0rw.cc b/storage/innobase/sync/sync0rw.cc index a04cc755186..7f141cb8604 100644 --- a/storage/innobase/sync/sync0rw.cc +++ b/storage/innobase/sync/sync0rw.cc @@ -515,14 +515,18 @@ ibool rw_lock_x_lock_low( } else { os_thread_id_t thread_id = os_thread_get_curr_id(); + bool recursive; + os_thread_id_t writer_thread; + if (!pass) { + recursive = lock->recursive; os_rmb; + writer_thread = lock->writer_thread; } /* Decrement failed: An X or SX lock is held by either this thread or another. Try to relock. */ - if (!pass && lock->recursive && - os_thread_eq(lock->writer_thread, thread_id)) { + if (!pass && recursive && os_thread_eq(writer_thread, thread_id)) { /* Other s-locks can be allowed. If it is request x recursively while holding sx lock, this x lock should be along with the latching-order. */ @@ -585,15 +589,19 @@ ibool rw_lock_sx_lock_low( } else { os_thread_id_t thread_id = os_thread_get_curr_id(); + bool recursive; + os_thread_id_t writer_thread; + if (!pass) { + recursive = lock->recursive; os_rmb; + writer_thread = lock->writer_thread; } /* Decrement failed: It already has an X or SX lock by this thread or another thread. If it is this thread, relock, else fail. */ - if (!pass && lock->recursive && - os_thread_eq(lock->writer_thread, thread_id)) { + if (!pass && recursive && os_thread_eq(writer_thread, thread_id)) { /* This thread owns an X or SX lock */ if (lock->sx_recursive++ == 0) { /* This thread is making first SX-lock request -- 2.17.1