[Commits] Rev 4369: MDEV-7148 - Recurring: InnoDB: Failing assertion: !lock->recursive in lp:maria/5.5

Sergey Vojtovich svoj at mariadb.org
Thu Nov 20 11:55:41 EET 2014


At lp:maria/5.5

------------------------------------------------------------
revno: 4369
revision-id: svoj at mariadb.org-20141120094823-fozdsrm1kzv46kxi
parent: jplindst at mariadb.org-20141119182734-cwbaed0ka90ocj5e
committer: Sergey Vojtovich <svoj at mariadb.org>
branch nick: 5.5
timestamp: Thu 2014-11-20 13:48:23 +0400
message:
  MDEV-7148 - Recurring: InnoDB: Failing assertion: !lock->recursive
  
  On PPC64 high-loaded server may crash due to assertion failure in InnoDB
  rwlocks code.
  
  This happened because load order between "recursive" and "writer_thread"
  wasn't properly enforced.
  
  Fixed so that now there is full memory barrier between these loads.
=== modified file 'storage/innobase/include/sync0rw.ic'
--- a/storage/innobase/include/sync0rw.ic	2014-08-19 16:28:35 +0000
+++ b/storage/innobase/include/sync0rw.ic	2014-11-20 09:48:23 +0000
@@ -438,6 +438,7 @@ rw_lock_x_lock_func_nowait(
 	os_thread_id_t	curr_thread	= os_thread_get_curr_id();
 
 	ibool success;
+	ibool local_recursive= lock->recursive;
 
 #ifdef INNODB_RW_LOCKS_USE_ATOMICS
 	success = os_compare_and_swap_lint(&lock->lock_word, X_LOCK_DECR, 0);
@@ -452,10 +453,14 @@ rw_lock_x_lock_func_nowait(
 	mutex_exit(&(lock->mutex));
 
 #endif
+	/* Note: recursive must be loaded before writer_thread see
+	comment for rw_lock_set_writer_id_and_recursion_flag().
+	To achieve this we load it before os_compare_and_swap_lint(),
+	which implies full memory barrier in current implementation. */
 	if (success) {
 		rw_lock_set_writer_id_and_recursion_flag(lock, TRUE);
 
-	} else if (lock->recursive
+	} else if (local_recursive
 		   && os_thread_eq(lock->writer_thread, curr_thread)) {
 		/* Relock: this lock_word modification is safe since no other
 		threads can modify (lock, unlock, or reserve) lock_word while

=== modified file 'storage/innobase/sync/sync0rw.c'
--- a/storage/innobase/sync/sync0rw.c	2014-08-29 12:14:11 +0000
+++ b/storage/innobase/sync/sync0rw.c	2014-11-20 09:48:23 +0000
@@ -566,6 +566,7 @@ rw_lock_x_lock_low(
 	ulint		line)	/*!< in: line where requested */
 {
 	os_thread_id_t	curr_thread	= os_thread_get_curr_id();
+	ibool local_recursive= lock->recursive;
 
 	if (rw_lock_lock_word_decr(lock, X_LOCK_DECR)) {
 
@@ -586,10 +587,12 @@ rw_lock_x_lock_low(
                                     file_name, line);
 
 	} else {
-               if (!pass)
-                 os_rmb;
-		/* Decrement failed: relock or failed lock */
-		if (!pass && lock->recursive
+		/* Decrement failed: relock or failed lock
+		Note: recursive must be loaded before writer_thread see
+		comment for rw_lock_set_writer_id_and_recursion_flag().
+		To achieve this we load it before rw_lock_lock_word_decr(),
+		which implies full memory barrier in current implementation. */
+		if (!pass && local_recursive
 		    && os_thread_eq(lock->writer_thread, curr_thread)) {
 			/* Relock */
                         lock->lock_word -= X_LOCK_DECR;

=== modified file 'storage/xtradb/include/sync0rw.ic'
--- a/storage/xtradb/include/sync0rw.ic	2014-08-19 16:28:35 +0000
+++ b/storage/xtradb/include/sync0rw.ic	2014-11-20 09:48:23 +0000
@@ -438,6 +438,7 @@ rw_lock_x_lock_func_nowait(
 	os_thread_id_t	curr_thread	= os_thread_get_curr_id();
 
 	ibool success;
+	ibool local_recursive= lock->recursive;
 
 #ifdef INNODB_RW_LOCKS_USE_ATOMICS
 	success = os_compare_and_swap_lint(&lock->lock_word, X_LOCK_DECR, 0);
@@ -452,10 +453,14 @@ rw_lock_x_lock_func_nowait(
 	mutex_exit(&(lock->mutex));
 
 #endif
+	/* Note: recursive must be loaded before writer_thread see
+	comment for rw_lock_set_writer_id_and_recursion_flag().
+	To achieve this we load it before os_compare_and_swap_lint(),
+	which implies full memory barrier in current implementation. */
 	if (success) {
 		rw_lock_set_writer_id_and_recursion_flag(lock, TRUE);
 
-	} else if (lock->recursive
+	} else if (local_recursive
 		   && os_thread_eq(lock->writer_thread, curr_thread)) {
 		/* Relock: this lock_word modification is safe since no other
 		threads can modify (lock, unlock, or reserve) lock_word while

=== modified file 'storage/xtradb/sync/sync0rw.c'
--- a/storage/xtradb/sync/sync0rw.c	2014-08-29 12:14:11 +0000
+++ b/storage/xtradb/sync/sync0rw.c	2014-11-20 09:48:23 +0000
@@ -563,6 +563,7 @@ rw_lock_x_lock_low(
 	ulint		line)	/*!< in: line where requested */
 {
 	os_thread_id_t	curr_thread	= os_thread_get_curr_id();
+	ibool local_recursive= lock->recursive;
 
 	if (rw_lock_lock_word_decr(lock, X_LOCK_DECR)) {
 
@@ -583,10 +584,12 @@ rw_lock_x_lock_low(
                                     file_name, line);
 
 	} else {
-               if (!pass)
-                 os_rmb;
-		/* Decrement failed: relock or failed lock */
-		if (!pass && lock->recursive
+		/* Decrement failed: relock or failed lock
+		Note: recursive must be loaded before writer_thread see
+		comment for rw_lock_set_writer_id_and_recursion_flag().
+		To achieve this we load it before rw_lock_lock_word_decr(),
+		which implies full memory barrier in current implementation. */
+		if (!pass && local_recursive
 		    && os_thread_eq(lock->writer_thread, curr_thread)) {
 			/* Relock */
                         lock->lock_word -= X_LOCK_DECR;



More information about the commits mailing list